close #622: fixed iteration order of the page ranges returned by the …

…ConversionUtils
torakiki · Aug 23, 2024 · 043d9d6 · 043d9d6
1 parent bea5705
commit 043d9d6
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 15 deletions.
diff --git a/pdfsam-core/src/main/java/org/pdfsam/core/support/params/ConversionUtils.java b/pdfsam-core/src/main/java/org/pdfsam/core/support/params/ConversionUtils.java
@@ -25,10 +25,7 @@
 import org.sejda.model.pdf.page.PagesSelection;
 
 import java.util.Arrays;
-import java.util.Collections;
-import java.util.Set;
 
-import static java.util.stream.Collectors.toSet;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
 import static org.apache.commons.lang3.StringUtils.split;
 import static org.pdfsam.i18n.I18nContext.i18n;
@@ -46,28 +43,27 @@ private ConversionUtils() {
     /**
      * @return the {@link PageRange} set for the given string, an empty set otherwise.
      */
-    public static Set<PageRange> toPageRangeSet(String selection) throws ConversionException {
+    public static NullSafeSet<PageRange> toPageRangeSet(String selection) throws ConversionException {
         if (isNotBlank(selection)) {
             var pageRangeSet = new NullSafeSet<PageRange>();
-            var tokens = Arrays.stream(split(selection, ",")).map(StringUtils::strip).collect(toSet());
-            for (var token : tokens) {
-                pageRangeSet.add(toPageRange(token));
-            }
+            Arrays.stream(split(selection, ",")).map(StringUtils::strip).map(ConversionUtils::toPageRange)
+                    .forEachOrdered(pageRangeSet::add);
             return pageRangeSet;
         }
-        return Collections.emptySet();
+        return new NullSafeSet<>();
     }
 
-    public static Set<PagesSelection> toPagesSelectionSet(String selection) throws ConversionException {
+    /**
+     * @return the {@link PagesSelection} set for the given string, an empty set otherwise.
+     */
+    public static NullSafeSet<PagesSelection> toPagesSelectionSet(String selection) throws ConversionException {
         if (isNotBlank(selection)) {
             var pageRangeSet = new NullSafeSet<PagesSelection>();
-            var tokens = Arrays.stream(split(selection, ",")).map(StringUtils::strip).collect(toSet());
-            for (var token : tokens) {
-                pageRangeSet.add(toPageSelection(token));
-            }
+            Arrays.stream(split(selection, ",")).map(StringUtils::strip).map(ConversionUtils::toPageSelection)
+                    .forEachOrdered(pageRangeSet::add);
             return pageRangeSet;
         }
-        return Collections.emptySet();
+        return new NullSafeSet<>();
     }
 
     private static PageRange toPageRange(String value) throws ConversionException {

diff --git a/pdfsam-core/src/test/java/org/pdfsam/core/support/params/ConversionUtilsTest.java b/pdfsam-core/src/test/java/org/pdfsam/core/support/params/ConversionUtilsTest.java
@@ -21,12 +21,15 @@
 import org.junit.jupiter.api.Test;
 import org.sejda.conversion.exception.ConversionException;
 import org.sejda.model.pdf.page.PageRange;
+import org.sejda.model.pdf.page.PagesSelection;
 
 import java.util.Set;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.sejda.model.pdf.page.PageRange.one;
 
 /**
  * @author Andrea Vacondio
@@ -89,4 +92,18 @@ public void multiple() {
         assertEquals(4, pageSet.stream().findFirst().get().getEnd());
         assertTrue(pageSet.stream().anyMatch(PageRange::isUnbounded));
     }
+
+    @Test
+    public void testOrderToPageRangeSet() {
+        Set<PageRange> pageSet = ConversionUtils.toPageRangeSet("1-7,9,17");
+        assertEquals(3, pageSet.size());
+        assertThat(pageSet).containsExactly(new PageRange(1, 7), one(9), one(17));
+    }
+
+    @Test
+    public void testOrderToPagesSelectionSet() {
+        Set<PagesSelection> pageSet = ConversionUtils.toPagesSelectionSet("1-7,9,17");
+        assertEquals(3, pageSet.size());
+        assertThat(pageSet).containsExactly(new PageRange(1, 7), one(9), one(17));
+    }
 }