Merge pull request #99 from ebx/dev

Release 1.0.5
ebx · Jun 19, 2020 · 0d6a8e6 · 0d6a8e6
2 parents fe8b861 + 79e481a
commit 0d6a8e6
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 56 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -63,4 +63,14 @@ is incorrectly returning the number of elements where the number of elements is
 request amount even though there are in fact more results. This fix should continue to 
 the next page until there are no more results.
 
-## 1.0.5 (Work in progress)
+## 1.0.5 (June 18, 2020)
+* After receiving a response from LinkedIn on the issues surrounding V2 pagination where the 
+`count` value in the response object does not match the number of elements returned or in fact in
+some cases where not all elements are returned, it's best to not provide pagination parameters in
+the request to the LinkedIn API until they have fixed it in the API. Pagination parameters will not
+longer be provided by default in `DefaultLinkedInClient.fetchConnection()`. Instead, if no `count`
+parameter is provided in the initial request, `V2PaginationImpl` will continue to iterate through
+all pages until the number of elements in the response no longer equals the expected count. 
+This will avoid infinitely looping over pages until an empty page is discovered.
+
+## 1.0.6 (Work in progress)
diff --git a/README.md b/README.md
@@ -22,7 +22,7 @@ use:
 <dependency>
   <groupId>com.echobox</groupId>
   <artifactId>ebx-linkedin-sdk</artifactId>
-  <version>1.0.4</version>
+  <version>1.0.6</version>
 </dependency>
 ```
 

diff --git a/pom.xml b/pom.xml
@@ -8,7 +8,7 @@
 
   <groupId>com.echobox</groupId>
   <artifactId>ebx-linkedin-sdk</artifactId>
-  <version>1.0.4</version>
+  <version>1.0.6</version>
   <packaging>jar</packaging>
 
   <properties>

diff --git a/src/main/java/com/echobox/api/linkedin/client/DefaultLinkedInClient.java b/src/main/java/com/echobox/api/linkedin/client/DefaultLinkedInClient.java
@@ -254,12 +254,6 @@ public <T> Connection<T> fetchConnection(String connection, Class<T> connectionT
     final String fullEndpoint = createEndpointForApiCall(connection, false);
 
     List<Parameter> parametersToAdd = new ArrayList<>(Arrays.asList(parameters));
-    if (parametersToAdd.stream().noneMatch(parameter -> parameter.name.equals("start"))) {
-      parametersToAdd.add(Parameter.with("start", 0));
-    }
-    if (parametersToAdd.stream().noneMatch(parameter -> parameter.name.equals("count"))) {
-      parametersToAdd.add(Parameter.with("count", 10));
-    }
     Parameter[] queryParams = parametersToAdd.toArray(new Parameter[parametersToAdd.size()]);
     String parameterString = toParameterString(queryParams);
     final String finalParameterString =

diff --git a/src/main/java/com/echobox/api/linkedin/client/paging/V2PagingImpl.java b/src/main/java/com/echobox/api/linkedin/client/paging/V2PagingImpl.java
@@ -17,9 +17,13 @@
 
 package com.echobox.api.linkedin.client.paging;
 
+import com.echobox.api.linkedin.util.URLUtils;
 import com.eclipsesource.json.JsonArray;
 import com.eclipsesource.json.JsonObject;
 
+import java.util.List;
+import java.util.Map;
+
 /**
  * The paging strategy for V1 JSON responses
  * @author Joanna
@@ -49,28 +53,25 @@ protected void discoverPages(JsonObject jsonObject, String fullEndpoint) {
           // You will know that you have reached the end of the dataset when your response
           // contains less elements in the entities block of the response than your count
           // parameter requested.
-//          Map<String, List<String>> extractParametersFromUrl =
-//              URLUtils.extractParametersFromUrl(fullEndpoint);
-//          if (extractParametersFromUrl.containsKey("count")) {
+          Map<String, List<String>> extractParametersFromUrl =
+              URLUtils.extractParametersFromUrl(fullEndpoint);
+          if (extractParametersFromUrl.containsKey("count")) {
             // Check if the count is less than the elements returned - if so we're at the last page
-//            int requestedCount = Integer.parseInt(extractParametersFromUrl.get("count").get(0));
-//            if (elements.size() <= requestedCount) {
-//              nextPageUrl = null;
-//              setPreviousPageURL(fullEndpoint, start, count);
-//              return;
-//            }
-//          }
-
-          // LinkedIn paging seems to be currently broken and it might return return elements
-          // where the count is less than the number of items requested but in fact there are
-          // more elements and LinkedIn API has not returned them. Instead, continue to explore
-          // the next page until there are no more pages.
-          if (elements.size() <= 0) {
-            nextPageUrl = null;
-            setPreviousPageURL(fullEndpoint, start, count);
-            return;
+            int requestedCount = Integer.parseInt(extractParametersFromUrl.get("count").get(0));
+            if (elements.size() < requestedCount) {
+              nextPageUrl = null;
+              setPreviousPageURL(fullEndpoint, start, count);
+              return;
+            }
+          } else {
+            // No explicit count was requested, check if there are no more datapoints
+            if (elements.size() != count) {
+              nextPageUrl = null;
+              setPreviousPageURL(fullEndpoint, start, count);
+              return;
+            }
           }
-
+  
           // Paging is available
           setNextPageURL(fullEndpoint, start, count);
           setPreviousPageURL(fullEndpoint, start, count);

diff --git a/src/test/java/com/echobox/api/linkedin/client/paging/V2PagingImplTest.java b/src/test/java/com/echobox/api/linkedin/client/paging/V2PagingImplTest.java
@@ -64,20 +64,61 @@ public void testNoElementsShouldReturnNullNextPrevURLs() {
     assertNull(strategy.getPreviousPageUrl());
   }
 
-//  /**
-//   * Test if the provided count in the URL is greater than the number of elements returned then we
-//   * have reached the last page
-//   * There is not previous page URL as the start is at 0
-//   */
-//  @Test
-//  public void testNextAndPrevURLsAreBothNull() {
-//    PagingStrategy strategy = new V2PagingImpl();
-//    strategy.populatePages(
-//        Json.parse("{\"paging\":{\"count\":5,\"start\":0},\"elements\":[1]}").asObject(),
-//        "https://test.com/test?start=10&count=5");
-//    assertNull(strategy.getNextPageUrl());
-//    assertNull(strategy.getPreviousPageUrl());
-//  }
+  /**
+   * Test pagination should stop if the number of elements do not match the number of elements
+   * returned in the response with no explicit URL count and start params
+   */
+  @Test
+  public void testMoreElementsReturnedThanIntended() {
+    PagingStrategy strategy = new V2PagingImpl();
+    strategy.populatePages(Json.parse("{\"paging\":{\"count\":3,\"start\":0},\"elements\":[1, 2, "
+            + "3, 4, 5]}").asObject(),
+        "https://test.com/test");
+    assertNull(strategy.getNextPageUrl());
+    assertNull(strategy.getPreviousPageUrl());
+  }
+
+  /**
+   * Test pagination should stop if the number of elements do not match the number of elements
+   * returned in the response with no explicit URL count and start params
+   */
+  @Test
+  public void testLessElementsReturnedThanIntended() {
+    PagingStrategy strategy = new V2PagingImpl();
+    strategy.populatePages(Json.parse("{\"paging\":{\"count\":5,\"start\":0},\"elements\":[1, 2]}")
+            .asObject(),
+        "https://test.com/test");
+    assertNull(strategy.getNextPageUrl());
+    assertNull(strategy.getPreviousPageUrl());
+  }
+
+  /**
+   * Test pagination should not stop as there might be more elements on the next page
+   */
+  @Test
+  public void testNumElementsReturnedMatchAndContinue() {
+    PagingStrategy strategy = new V2PagingImpl();
+    strategy.populatePages(Json.parse("{\"paging\":{\"count\":2,\"start\":0},\"elements\":[1, 2]}")
+            .asObject(),
+        "https://test.com/test");
+    assertEquals("https://test.com/test?start=2&count=2", strategy.getNextPageUrl());
+    assertNull(strategy.getPreviousPageUrl());
+  }
+
+  /**
+   * Test if the provided count in the URL is greater than the number of elements returned then we
+   * have reached the last page
+   * There is not previous page URL as the start is at 0
+   */
+  @Test
+  public void testNextAndPrevURLsAreBothNull() {
+    PagingStrategy strategy = new V2PagingImpl();
+    strategy.populatePages(
+        Json.parse("{\"paging\":{\"count\":5,\"start\":0},\"elements\":[1]}").asObject(),
+        "https://test.com/test?start=10&count=5");
+    assertNull(strategy.getNextPageUrl());
+    assertNull(strategy.getPreviousPageUrl());
+  }
 
   /**
    * If the start = 0, there should be no previous page URL available
@@ -106,18 +147,18 @@ public void testNextAndPrevURLsAvailableWithProvidedCount() {
     assertEquals("https://test.com/test?start=0&count=5", strategy.getPreviousPageUrl());
   }
 
-//  /**
-//   * If the requested count > the number of elements, there next page URL should be null
-//   * If the start != 0, there should be a previous page URL with no overlap
-//   */
-//  @Test
-//  public void testNextURLIsNullAndPrevURLsAvailable() {
-//    PagingStrategy strategy = new V2PagingImpl();
-//    strategy.populatePages(
-//        Json.parse("{\"paging\":{\"count\":5,\"start\":15},\"elements\":[1]}").asObject(),
-//        "https://test.com/test?start=5&count=5");
-//    assertNull(strategy.getNextPageUrl());
-//    assertEquals("https://test.com/test?start=10&count=5", strategy.getPreviousPageUrl());
-//  }
+  /**
+   * If the requested count > the number of elements, there next page URL should be null
+   * If the start != 0, there should be a previous page URL with no overlap
+   */
+  @Test
+  public void testNextURLIsNullAndPrevURLsAvailable() {
+    PagingStrategy strategy = new V2PagingImpl();
+    strategy.populatePages(
+        Json.parse("{\"paging\":{\"count\":5,\"start\":15},\"elements\":[1]}").asObject(),
+        "https://test.com/test?start=5&count=5");
+    assertNull(strategy.getNextPageUrl());
+    assertEquals("https://test.com/test?start=10&count=5", strategy.getPreviousPageUrl());
+  }
 
 }