Uploaded image for project: 'Nuxeo Platform'
  1. Nuxeo Platform
  2. NXP-22798

Use Jersey's Apache HTTP client handler instead of default one for the REST API tests

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 9.3
    • Component/s: Rest API, Tests
    • Release Notes Description:
      Hide

      REST API Client handler now uses Apache's Jersey implementation while it was using the JDK one previously. It provides stricter behaviours leading to better resilience.

      Show
      REST API Client handler now uses Apache's Jersey implementation while it was using the JDK one previously. It provides stricter behaviours leading to better resilience.
    • Impact type:
      API change
    • Upgrade notes:
      Hide

      Added:

      • CloseableClientResponse
      • JerseyClientHelper
      • BaseTest#getServiceFor(String username, String password)
      • BaseTest#getServiceFor(String resource, String username, String password)

      The Apache implementation is stricter than the JDK implementation previously used: any time you obtain a ClientResponse from Jersey it needs to be closed otherwise connections might leak.
      Therefore all the BaseTest#getResponse methods now return a CloseableClientResponse that implements AutoCloseable, and the right pattern to use in the tests is:

          try (CloseableClientResponse response = getResponse(RequestType.GET, "id/" + note.getId())) {
              // Do something with the response
              assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
              assertEntityEqualsDoc(response.getEntityInputStream(), note);
          }
      

      This is true no matter what signature of getResponse you are using and whatever HTTP method you are invoking: POST, GET, PUT, DELETE.

      If you are not relying on BaseTest#getResponse but are using directly WebResource.get or WebResource.post to make a call then you must do something like:

          WebResource wr = client.resource(NUXEO_URL).path(path);
          try (CloseableClientResponse cr = CloseableClientResponse.of(
              wr.queryParam("access_token", accessToken).get(ClientResponse.class))) {
              assertEquals(401, cr.getStatus());
          }
      

      If you are not extending BaseTest and are instantiating a Client yourself, you must make sure to rely on the Apache HTTP client by calling:

          Client client = JerseyClientHelper.DEFAULT_CLIENT;
      

      or:

          Client client = JerseyClientHelper.clientBuilder()
                                            .setXXX()
                                            .setYYY()
                                            .build();
      
      Show
      Added: CloseableClientResponse JerseyClientHelper BaseTest#getServiceFor(String username, String password) BaseTest#getServiceFor(String resource, String username, String password) The Apache implementation is stricter than the JDK implementation previously used: any time you obtain a ClientResponse from Jersey it needs to be closed otherwise connections might leak. Therefore all the BaseTest#getResponse methods now return a CloseableClientResponse that implements AutoCloseable, and the right pattern to use in the tests is: try (CloseableClientResponse response = getResponse(RequestType.GET, "id/" + note.getId())) { // Do something with the response assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); assertEntityEqualsDoc(response.getEntityInputStream(), note); } This is true no matter what signature of getResponse you are using and whatever HTTP method you are invoking: POST, GET, PUT, DELETE. If you are not relying on BaseTest#getResponse but are using directly WebResource.get or WebResource.post to make a call then you must do something like: WebResource wr = client.resource(NUXEO_URL).path(path); try (CloseableClientResponse cr = CloseableClientResponse.of( wr.queryParam( "access_token" , accessToken).get(ClientResponse.class))) { assertEquals(401, cr.getStatus()); } If you are not extending BaseTest and are instantiating a Client yourself, you must make sure to rely on the Apache HTTP client by calling: Client client = JerseyClientHelper.DEFAULT_CLIENT; or: Client client = JerseyClientHelper.clientBuilder() .setXXX() .setYYY() .build();
    • Sprint:
      nxfit 9.3.2
    • Story Points:
      8

      Description

      The BaseTest class initializes a Jersey client with Client#create(ClientConfig cc) that uses the default URLConnectionClientHandler that relies on java.net.HttpURLConnection, the JDK's HTTP client.

      The JDK's Keep-Alive strategy (see http://docs.oracle.com/javase/8/docs/technotes/guides/net/http-keepalive.html) isn't suitable for our unit tests relying on the JettyFeature: a TCP connection opened within a test class (A) can be reused to send HTTP requests in the next executed test class (B), yet when the JettyComponent stops between the 2 test classes it closes the connection's underlying socket.
      In this case, the TCP FIN flag is sent to the client. When the first request of B's first test is sent, the server responds with the TCP RST flag (reset) because the connection has been closed, leading to an empty HTTP response thus a failure when trying to get parsed.

      This was detected when investigating NXP-22781: the BatchUploadFixture suite class is run twice in a row by BatchUploadTest and RedisBatchUploadTest. The second run would fail because trying to reuse a connection kept alive from the first run.

      Notes:

      • This could possibly happen with 2 consecutive test classes not running the same suite class. It's not clear why it was only detected in this case, maybe because the Keep-Alive timeout is 5 seconds (arbitrary and non-configurable value used by the JDK) and is usually is short enough to expire between 2 test class runs.
      • This is happening since NXP-18317. Before that, the first POST request of B's first test would be retried in case of failure, opening a new connection thus succeeding. Now that we don't retry a failing POST request, a java.net.SocketException: Unexpected end of file from server is thrown.

      The ApacheHttpClient4Handler is also much more configurable than the default one through the org.apache.http.impl.client.HttpClientBuilder API.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 3 days, 4 hours
                  3d 4h

                    PagerDuty

                    Error rendering 'com.pagerduty.jira-server-plugin:PagerDuty'. Please contact your Jira administrators.