Merge pull request #61 from kiwix/fix_suggestion_iterator

kiwix · Aug 8, 2023 · 4fed04f · 4fed04f
2 parents f561034 + c94a04b
commit 4fed04f
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 4 deletions.
diff --git a/lib/src/main/cpp/libzim/entry_iterator.cpp b/lib/src/main/cpp/libzim/entry_iterator.cpp
@@ -89,16 +89,34 @@ METHOD0(jboolean, hasNext) {
 METHOD0(jobject, next) {
   switch (get_order(env, thisObj)) {
     case 0: {
+      auto end = getPtr<PATH_NATIVE_TYPE>(env, thisObj, "nativeHandleEnd");
+      if ((*GET_PTR(PATH_NATIVE_TYPE)) == *end) {
+        throwException(env, "java/util/NoSuchElementException", "");
+        return nullptr;
+      }
+
       zim::Entry entry = **GET_PTR(PATH_NATIVE_TYPE);
       (*GET_PTR(PATH_NATIVE_TYPE))++;
       return BUILD_WRAPPER("org/kiwix/libzim/Entry", entry);
     }
     case 1: {
+      auto end = getPtr<TITLE_NATIVE_TYPE>(env, thisObj, "nativeHandleEnd");
+      if ((*GET_PTR(TITLE_NATIVE_TYPE)) == *end) {
+        throwException(env, "java/util/NoSuchElementException", "");
+        return nullptr;
+      }
+
       zim::Entry entry = **GET_PTR(TITLE_NATIVE_TYPE);
       (*GET_PTR(TITLE_NATIVE_TYPE))++;
       return BUILD_WRAPPER("org/kiwix/libzim/Entry", entry);
     }
     case 2: {
+      auto end = getPtr<EFFICIENT_NATIVE_TYPE>(env, thisObj, "nativeHandleEnd");
+      if ((*GET_PTR(EFFICIENT_NATIVE_TYPE)) == *end) {
+        throwException(env, "java/util/NoSuchElementException", "");
+        return nullptr;
+      }
+
       zim::Entry entry = **GET_PTR(EFFICIENT_NATIVE_TYPE);
       (*GET_PTR(EFFICIENT_NATIVE_TYPE))++;
       return BUILD_WRAPPER("org/kiwix/libzim/Entry", entry);

diff --git a/lib/src/main/cpp/libzim/search_iterator.cpp b/lib/src/main/cpp/libzim/search_iterator.cpp
@@ -57,11 +57,19 @@ METHOD0(jstring, getZimId) {
 } CATCH_EXCEPTION(0)
 
 METHOD0(jboolean, hasNext) {
+  // THIS is the next item to return. So we have to check it with end
   auto end = getPtr<NATIVE_TYPE>(env, thisObj, "nativeHandleEnd");
   return *THIS != *end;
 } CATCH_EXCEPTION(false)
 
 METHOD0(jobject, next) {
+  // THIS is the next item to return. So we have to return it and advance for next round
+  auto end = getPtr<NATIVE_TYPE>(env, thisObj, "nativeHandleEnd");
+  if (*THIS == *end) {
+    throwException(env, "java/util/NoSuchElementException", "");
+    return nullptr;
+  }
+
   zim::Entry entry = **THIS;
   (*THIS)++;
   return BUILD_WRAPPER("org/kiwix/libzim/Entry", entry);

diff --git a/lib/src/main/cpp/libzim/suggestion_iterator.cpp b/lib/src/main/cpp/libzim/suggestion_iterator.cpp
@@ -42,13 +42,19 @@ METHOD0(void, dispose)
 } CATCH_EXCEPTION()
 
 METHOD0(jboolean, hasNext) {
-  NATIVE_TYPE next(*THIS);
-  next++;
+  // THIS is the next item to return. So we have to check it with end
   auto end = getPtr<NATIVE_TYPE>(env, thisObj, "nativeHandleEnd");
-  return next == *end;
+  return *THIS != *end;
 } CATCH_EXCEPTION(false)
 
 METHOD0(jobject, next) {
+  // THIS is the next item to return. So we have to return it and advance for next round
+  auto end = getPtr<NATIVE_TYPE>(env, thisObj, "nativeHandleEnd");
+  if (*THIS == *end) {
+    throwException(env, "java/util/NoSuchElementException", "");
+    return nullptr;
+  }
+
   zim::SuggestionItem item = **THIS;
   (*THIS)++;
   return BUILD_WRAPPER("org/kiwix/libzim/SuggestionItem", item);

diff --git a/lib/src/test/test.java b/lib/src/test/test.java
@@ -123,6 +123,14 @@ private void testArchive(TestArchive archive)
             assertEquals("favicon.png", iter.next().getPath());
             assertEquals("main.html", iter.next().getPath());
             assertFalse(iter.hasNext());
+            try {
+                iter.next();
+                fail("ERROR: next() should raise a NoSuchElementException.");
+            } catch (NoSuchElementException e) {
+                // We are good
+            } catch(Exception e) {
+                fail("ERROR: Must be a NoSuchElementException.");
+            }
         }
 
         {
@@ -131,6 +139,14 @@ private void testArchive(TestArchive archive)
             assertEquals("main.html", iter.next().getPath());
             // No favicon, because favicon is not a main article (no title)
             assertFalse(iter.hasNext());
+            try {
+                iter.next();
+                fail("ERROR: next() should raise a NoSuchElementException.");
+            } catch (NoSuchElementException e) {
+                // We are good
+            } catch(Exception e) {
+                fail("ERROR: Must be a NoSuchElementException.");
+            }
         }
 
         {
@@ -139,20 +155,44 @@ private void testArchive(TestArchive archive)
             assertEquals("main.html", iter.next().getPath());
             assertEquals("favicon.png", iter.next().getPath());
             assertFalse(iter.hasNext());
+            try {
+                iter.next();
+                fail("ERROR: next() should raise a NoSuchElementException.");
+            } catch (NoSuchElementException e) {
+                // We are good
+            } catch(Exception e) {
+                fail("ERROR: Must be a NoSuchElementException.");
+            }
         }
 
         {
             TestEntryIterator iter = archive.findByPath("ma");
             assertTrue(iter.hasNext());
             assertEquals("main.html", iter.next().getPath());
             assertFalse(iter.hasNext());
+            try {
+                iter.next();
+                fail("ERROR: next() should raise a NoSuchElementException.");
+            } catch (NoSuchElementException e) {
+                // We are good
+            } catch(Exception e) {
+                fail("ERROR: Must be a NoSuchElementException.");
+            }
         }
 
         {
             TestEntryIterator iter = archive.findByTitle("Test");
             assertTrue(iter.hasNext());
             assertEquals("main.html", iter.next().getPath());
             assertFalse(iter.hasNext());
+            try {
+                iter.next();
+                fail("ERROR: next() should raise a NoSuchElementException.");
+            } catch (NoSuchElementException e) {
+                // We are good
+            } catch(Exception e) {
+                fail("ERROR: Must be a NoSuchElementException.");
+            }
         }
 
         // Test invalid path
@@ -210,7 +250,7 @@ public void testNotValid() {
         } catch (ZimFileFormatException e) {
             assertEquals("Invalid magic number", e.getMessage());
         } catch(Exception e) {
-                fail("ERROR: Must be a ZimFileFormatException.");
+            fail("ERROR: Must be a ZimFileFormatException.");
         }
     }
 
@@ -459,6 +499,14 @@ public void testSearcher() throws Exception, ZimFileFormatException, JNIKiwixExc
             assertTrue(results.hasNext());
             TestSuggestionItem suggestionItem = results.next();
             assertFalse(results.hasNext());
+            try {
+                results.next();
+                fail("ERROR: next() should raise a NoSuchElementException.");
+            } catch (NoSuchElementException e) {
+                // We are good
+            } catch(Exception e) {
+                fail("ERROR: Must be a NoSuchElementException.");
+            }
             assertEquals("Test ZIM file", suggestionItem.getTitle());
             assertEquals("main.html", suggestionItem.getPath());
             assertTrue(suggestionItem.hasSnippet());