Merge pull request #156 from europeana/MET-1053

MET-1053 , RepresentationIterator throws null pointer exception in a …

service/mcs/driver/pom.xml

@@ -89,6 +89,13 @@
             <version>1.0.2</version>
         </dependency>
 
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <scope>test</scope>
+        </dependency>
+
+
     </dependencies>
     <build>
         <testResources>

service/mcs/driver/src/main/java/eu/europeana/cloud/mcs/driver/RepresentationIterator.java

@@ -12,14 +12,13 @@
 
 /**
  * Class for iterating through Representations of given data set.
- *
+ * <p>
  * The best way to initialise iterator is to obtain it by calling
  * {@link DataSetServiceClient#getRepresentationIteratorForProvider(String providerId, String dataSetId)}
  * method.
- *
+ * <p>
  * Iterator obtains {@link Representation} objects in chunks, obtaining new
  * chunk only if needed, inside overridden {@link Iterator} class methods.
- *
  */
 public class RepresentationIterator implements Iterator<Representation> {
 
@@ -35,10 +34,10 @@ public class RepresentationIterator implements Iterator<Representation> {
     /**
      * Creates instance of RepresentationIterator.
      *
-     * @param client properly initialised client for internal communication with
-     * MCS server (required)
+     * @param client     properly initialised client for internal communication with
+     *                   MCS server (required)
      * @param providerId id of the provider (required)
-     * @param dataSetId data set identifier (required)
+     * @param dataSetId  data set identifier (required)
      */
     public RepresentationIterator(DataSetServiceClient client, String providerId, String dataSetId) {
         if (client == null) {
@@ -60,10 +59,10 @@ public RepresentationIterator(DataSetServiceClient client, String providerId, St
 
     /**
      * Returns <code>true</code> if the iteration has more elements.
-     *
-     * The first call to this method might take longer time than the others, as
-     * there might be a need to obtain first chunk of data. If data set does not
-     * exists, first call to this method will throw {@link DriverException} with
+     * <p>
+     * Some calls to this method might take longer time than the others, if the
+     * new chunk of data has to be obtained. If data set does not exists,
+     * first call to this method will throw {@link DriverException} with
      * inner exception: {@link DataSetNotExistsException}.
      *
      * @return {@code true} if the iteration has more elements, false if not.
@@ -73,13 +72,19 @@ public boolean hasNext() {
         if (firstTime.unpack()) {
             obtainNextChunk();
         }
-
-        return (representationListIterator.hasNext() || nextSlice != null);
+        if (representationListIterator.hasNext()) {
+            return representationListIterator.hasNext();
+        }
+        if (nextSlice != null) {
+            obtainNextChunk();
+            return representationListIterator.hasNext();
+        }
+        return false;
     }
 
     /**
      * Returns next element in the iteration.
-     *
+     * <p>
      * Some calls to this method might take longer time than the others, if the
      * new chunk of data has to be obtained. If data set does not exists, first
      * call to this method will throw {@link DriverException} with inner

service/mcs/driver/src/test/java/eu/europeana/cloud/mcs/driver/RepresentationIteratorTest.java

@@ -0,0 +1,126 @@
+package eu.europeana.cloud.mcs.driver;
+
+import eu.europeana.cloud.common.model.Representation;
+import eu.europeana.cloud.common.response.ResultSlice;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Matchers.notNull;
+import static org.mockito.Mockito.*;
+
+
+/**
+ * Created by Tarek on 5/24/2018.
+ */
+public class RepresentationIteratorTest {
+
+    public static final String NEXT_SLICE = "nextSlice";
+    private DataSetServiceClient dataSetServiceClient;
+    private static final String PROVIDER = "PROVIDER";
+    private static final String DATASET = "DATASET";
+
+
+    @Before
+    public void init() {
+        dataSetServiceClient = mock(DataSetServiceClient.class);
+    }
+
+    @Test
+    public void testRepresentationIteratorForTheFirstIteration() throws Exception {
+        RepresentationIterator representationIterator = new RepresentationIterator(dataSetServiceClient, PROVIDER, DATASET);
+        ResultSlice<Representation> representationResultSlice = getFinalRepresentationResultSlice(2);
+
+        when(dataSetServiceClient.getDataSetRepresentationsChunk(PROVIDER, DATASET, null)).thenReturn(representationResultSlice);
+        int count = 0;
+
+        while (representationIterator.hasNext()) {
+            assertTrue(representationIterator.hasNext());
+            representationIterator.next();
+            count++;
+        }
+        assertEquals(count, 2);
+        verify(dataSetServiceClient, times(0)).getDataSetRepresentationsChunk(eq(PROVIDER), eq(DATASET), notNull(String.class));
+        assertFalse(representationIterator.hasNext());
+
+
+    }
+
+    @Test
+    public void testRepresentationIteratorWhenTheSecondIterationIsEmpty() throws Exception {
+        RepresentationIterator representationIterator = new RepresentationIterator(dataSetServiceClient, PROVIDER, DATASET);
+
+        ResultSlice<Representation> representationResultSlice = getRepresentationResultSlice(100);
+
+        when(dataSetServiceClient.getDataSetRepresentationsChunk(PROVIDER, DATASET, null)).thenReturn(representationResultSlice);
+
+
+        ResultSlice<Representation> emptyResultSet = new ResultSlice<>();
+        emptyResultSet.setResults(new ArrayList<Representation>());
+        when(dataSetServiceClient.getDataSetRepresentationsChunk(eq(PROVIDER), eq(DATASET), eq(NEXT_SLICE))).thenReturn(emptyResultSet);
+
+        int count = 0;
+        while (representationIterator.hasNext()) {
+            assertTrue(representationIterator.hasNext());
+            representationIterator.next();
+            count++;
+        }
+        assertEquals(100, count);
+        verify(dataSetServiceClient, times(1)).getDataSetRepresentationsChunk(eq(PROVIDER), eq(DATASET), eq(NEXT_SLICE));
+        assertFalse(representationIterator.hasNext());
+
+
+    }
+
+
+    @Test
+    public void testRepresentationIteratorWhenTheSecondIterationNotEmpty() throws Exception {
+        RepresentationIterator representationIterator = new RepresentationIterator(dataSetServiceClient, PROVIDER, DATASET);
+
+        ResultSlice<Representation> representationResultSlice = getRepresentationResultSlice(100);
+        when(dataSetServiceClient.getDataSetRepresentationsChunk(PROVIDER, DATASET, null)).thenReturn(representationResultSlice);
+
+        ResultSlice<Representation> nextResultSet = getFinalRepresentationResultSlice(5);
+        when(dataSetServiceClient.getDataSetRepresentationsChunk(eq(PROVIDER), eq(DATASET), eq(NEXT_SLICE))).thenReturn(nextResultSet);
+
+        int count = 0;
+
+        while (representationIterator.hasNext()) {
+            assertTrue(representationIterator.hasNext());
+            representationIterator.next();
+            count++;
+        }
+        assertEquals(105, count);
+        verify(dataSetServiceClient, times(1)).getDataSetRepresentationsChunk(eq(PROVIDER), eq(DATASET), eq(NEXT_SLICE));
+        assertFalse(representationIterator.hasNext());
+
+
+    }
+
+
+    private ResultSlice<Representation> getRepresentationResultSlice(int ItemNumber) {
+        List<Representation> representationList = new ArrayList<>(ItemNumber);
+        for (int i = 0; i < ItemNumber; i++) {
+            representationList.add(new Representation());
+        }
+
+        ResultSlice<Representation> representationResultSlice = new ResultSlice<>();
+        representationResultSlice.setResults(representationList);
+        representationResultSlice.setNextSlice(NEXT_SLICE);
+        return representationResultSlice;
+    }
+
+    private ResultSlice<Representation> getFinalRepresentationResultSlice(int ItemNumber) {
+        ResultSlice<Representation> representationResultSlice = getRepresentationResultSlice(ItemNumber);
+        representationResultSlice.setNextSlice(null);
+        return representationResultSlice;
+    }
+
+
+}