Skip to content

NottableString not does not work correctly when there are matches. #1974

@DarioViva42

Description

@DarioViva42

Bug Description

Because it is currently not possible to strictly match only query parameters in the expectation (see: #1879) the next best thing is to use NottableString.not on the parameters that should not be present. But this seems to not work correctly.

What I am trying to do
I want to use NottableString.not to add make sure that the expectation only passes, when certain parameter names are not used in a request. But the rules for when they work are really strange. When no query parameter is added to the request I can add as many not requirements to the expectation as I want. But when adding a parameter to the request, suddenly only one not requirement is allowed (of course with another parameter name notted, than was used in the request).

What also does not work is if I want to explicitly check that one parameter is present but another is not.

It seems to me that the moment multiple withQueryStringParameter are used nottable parameters stop working.

MockServer version
5.15

To Reproduce

Steps to reproduce the issue:

I use MockServer via the junit dependency (https://central.sonatype.com/artifact/org.mock-server/mockserver-junit-jupiter/overview)
The following Code can be used to reproduce the weird behaviour:

Code to test:

import org.springframework.stereotype.Service;
import org.springframework.web.client.RestClient;

@Service
public class RestService {

	public String response(String url) {
		return RestClient.builder()
				.baseUrl(url)
				.build()
				.get()
				.uri(uriBuilder -> uriBuilder.path("/test")
						.queryParam("name", "John")
						.build())
				.retrieve()
				.body(String.class);
	}
}

In the code above only one query parameter is used (name) and no other parameter is used additionally. So the below test should pass. It expects that the name parameter is present and the age parameter is not present. This is of course the case. But the test still fails.

Test that should pass:

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockserver.client.MockServerClient;
import org.mockserver.junit.jupiter.MockServerExtension;
import org.mockserver.model.HttpRequest;
import org.mockserver.model.HttpResponse;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockserver.model.NottableString.not;

@ExtendWith({MockServerExtension.class, MockitoExtension.class})
class RestServiceTest {

	private static MockServerClient msc;

	@InjectMocks
	private RestService restService;

	@BeforeAll
	public static void initMockserver(MockServerClient client) {
		RestServiceTest.msc = client;
	}

	@Test
	void response() {
		msc
				.when(HttpRequest.request()
						.withQueryStringParameter("name", "John")
						.withQueryStringParameter(not("age"))
						.withMethod("GET"))
				.respond(HttpResponse.response()
						.withBody("Hello World!"));

		String response = restService.response("http://localhost:" + msc.getPort());

		assertThat(response).isEqualTo("Hello World!");
	}
}

The test can be “fixed” by either keeping only the positive requirement for name. or only using the negative requirement for age.

Expected behaviour
The test passes.

MockServer Log

18:58:27.066 [MockServer-EventLog0] INFO org.mockserver.log.MockServerEventLog -- 63852 request:

  {
    "method" : "GET",
    "path" : "/test",
    "headers" : {
      "content-length" : [ "0" ],
      "User-Agent" : [ "Java-http-client/25.0.1" ],
      "Upgrade" : [ "h2c" ],
      "Host" : [ "localhost:63852" ],
      "HTTP2-Settings" : [ "AAEAAEAAAAIAAAAAAAMAAAAAAAQBAAAAAAUAAEAAAAYABgAA" ],
      "Connection" : [ "Upgrade, HTTP2-Settings" ],
      "Accept-Encoding" : [ "gzip", "deflate" ]
    },
    "keepAlive" : true,
    "secure" : false,
    "protocol" : "HTTP_1_1",
    "localAddress" : "127.0.0.1:63852",
    "remoteAddress" : "127.0.0.1:63854"
  }

 didn't match expectation:

  {
    "httpRequest" : {
      "method" : "GET",
      "queryStringParameters" : {
        "name" : [ "John" ],
        "!age" : [ ".*" ]
      }
    },
    "httpResponse" : {
      "body" : "Hello World!"
    },
    "id" : "2e092b71-44a1-4dde-8eed-1b4f6a227c00",
    "priority" : 0,
    "timeToLive" : {
      "unlimited" : true
    },
    "times" : {
      "unlimited" : true
    }
  }

 because:

  method matched
  path matched
  body matched
  headers matched
  cookies matched
  pathParameters matched
  queryParameters didn't match

Additional Context

Digging deeper we found out that there is a special case for when there are no parameters at all (matched.isEmpty()). Then the test passes if all the requirements are either notted or optional.

https://github.com/mock-server/mockserver/blob/master/mockserver-core/src/main/java/org/mockserver/matchers/MultiValueMapMatcher.java#L42-L48

But if some parameters are sent in the request another codepath is used. In this code path the notted requirements are not correctly taken into account. But it seems to be that the optionals are taken into account.

https://github.com/mock-server/mockserver/blob/master/mockserver-core/src/main/java/org/mockserver/collections/SubSetMatcher.java#L31-L33

We are pretty sure that there is a bug on line 31. Shouldn’t the following code be used instead?

long subsetNonOptionalSize = subset.stream()
    .filter(ImmutableEntry::isNotOptional)
    .filter(entry -> !entry.isNotted())
    .count();

Notice the additional filter that takes care, that nottable parameters are not added to the total count.

A better name for the variable would probably be subsetPositiveMatchesSize or something similar.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions