close torakiki#125: alternate mix now validates against max pages of …

…the two input documents
Ablesius · Nov 30, 2015 · 7be1c9b · 7be1c9b
1 parent eeebe2c
commit 7be1c9b
Show file tree

Hide file tree

Showing 6 changed files with 92 additions and 39 deletions.
diff --git a/pdfsam-alternate-mix/src/main/java/org/pdfsam/alternatemix/AlternateMixModule.java b/pdfsam-alternate-mix/src/main/java/org/pdfsam/alternatemix/AlternateMixModule.java
@@ -23,12 +23,6 @@
 import java.util.Map;
 import java.util.function.Consumer;
 
-import javafx.geometry.Pos;
-import javafx.scene.Node;
-import javafx.scene.image.ImageView;
-import javafx.scene.layout.Pane;
-import javafx.scene.layout.VBox;
-
 import javax.inject.Inject;
 import javax.inject.Named;
 
@@ -47,6 +41,12 @@
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
+import javafx.geometry.Pos;
+import javafx.scene.Node;
+import javafx.scene.image.ImageView;
+import javafx.scene.layout.Pane;
+import javafx.scene.layout.VBox;
+
 /**
  * Alternate mix module to let the user merge two pdf documents taking pages alternately in straight or reverse order.
  * 
@@ -85,6 +85,7 @@ void onValidSource(AlternateMixParametersBuilder builder, PdfFileSource source)
         this.firstDocument.setId("firstDocumentMix");
         this.firstDocument.setPromptText(DefaultI18nContext.getInstance().i18n(
                 "Select or drag and drop the first PDF you want to mix"));
+        this.firstDocument.addOnLoaded(d -> optionsPane.setFirstDocumentMaxPages(d.pages().getValue()));
         this.secondDocument = new AlternateMixSingleSelectionPane(id()) {
             @Override
             void onValidSource(AlternateMixParametersBuilder builder, PdfFileSource source) {
@@ -94,7 +95,7 @@ void onValidSource(AlternateMixParametersBuilder builder, PdfFileSource source)
         this.secondDocument.setId("secondDocumentMix");
         this.secondDocument.setPromptText(DefaultI18nContext.getInstance().i18n(
                 "Select or drag and drop the second PDF you want to mix"));
-
+        this.secondDocument.addOnLoaded(d -> optionsPane.setSecondDocumentMaxPages(d.pages().getValue()));
     }
 
     @Override

diff --git a/pdfsam-alternate-mix/src/main/java/org/pdfsam/alternatemix/AlternateMixOptionsPane.java b/pdfsam-alternate-mix/src/main/java/org/pdfsam/alternatemix/AlternateMixOptionsPane.java
@@ -25,11 +25,6 @@
 import java.util.Optional;
 import java.util.function.Consumer;
 
-import javafx.scene.control.CheckBox;
-import javafx.scene.control.Label;
-import javafx.scene.layout.HBox;
-import javafx.scene.layout.VBox;
-
 import org.pdfsam.i18n.DefaultI18nContext;
 import org.pdfsam.support.params.TaskParametersBuildStep;
 import org.pdfsam.support.validation.Validators;
@@ -38,14 +33,19 @@
 import org.pdfsam.ui.support.Style;
 import org.pdfsam.ui.workspace.RestorableView;
 
+import javafx.scene.control.CheckBox;
+import javafx.scene.control.Label;
+import javafx.scene.layout.HBox;
+import javafx.scene.layout.VBox;
+
 /**
  * Panel for the Alternate Mix options
  * 
  * @author Andrea Vacondio
  *
  */
-class AlternateMixOptionsPane extends VBox implements TaskParametersBuildStep<AlternateMixParametersBuilder>,
-        RestorableView {
+class AlternateMixOptionsPane extends VBox
+        implements TaskParametersBuildStep<AlternateMixParametersBuilder>, RestorableView {
 
     private CheckBox reverseFirst = new CheckBox(DefaultI18nContext.getInstance().i18n("Reverse the first document"));
     private CheckBox reverseSecond = new CheckBox(DefaultI18nContext.getInstance().i18n("Reverse the second document"));
@@ -61,15 +61,27 @@ class AlternateMixOptionsPane extends VBox implements TaskParametersBuildStep<Al
         this.secondStep.setId("alternateMixSecondStep");
         initialState();
         getStyleClass().addAll(Style.CONTAINER.css());
-        HBox firstStepContainer = new HBox(new Label(DefaultI18nContext.getInstance().i18n(
-                "Switch from the first document to the second one after the following pages")), firstStep);
+        HBox firstStepContainer = new HBox(
+                new Label(DefaultI18nContext.getInstance()
+                        .i18n("Switch from the first document to the second one after the following pages")),
+                firstStep);
         firstStepContainer.getStyleClass().addAll(Style.HCONTAINER.css());
-        HBox secondStepContainer = new HBox(new Label(DefaultI18nContext.getInstance().i18n(
-                "Switch from the second document to the first one after the following pages")), secondStep);
+        HBox secondStepContainer = new HBox(
+                new Label(DefaultI18nContext.getInstance()
+                        .i18n("Switch from the second document to the first one after the following pages")),
+                secondStep);
         secondStepContainer.getStyleClass().addAll(Style.HCONTAINER.css());
         getChildren().addAll(this.reverseFirst, this.reverseSecond, firstStepContainer, secondStepContainer);
     }
 
+    void setFirstDocumentMaxPages(Integer value) {
+        this.firstStep.setValidator(Validators.newPositiveIntRangeString(1, value));
+    }
+
+    void setSecondDocumentMaxPages(Integer value) {
+        this.secondStep.setValidator(Validators.newPositiveIntRangeString(1, value));
+    }
+
     public void apply(AlternateMixParametersBuilder builder, Consumer<String> onError) {
         builder.reverseFirst(reverseFirst.isSelected());
         builder.reverseSecond(reverseSecond.isSelected());
@@ -80,15 +92,15 @@ public void apply(AlternateMixParametersBuilder builder, Consumer<String> onErro
             builder.stepFirst(Integer.parseInt(firstStep.getText()));
             builder.stepSecond(Integer.parseInt(secondStep.getText()));
         } else {
-            onError.accept(DefaultI18nContext.getInstance().i18n(
-                    "Invalid parameter 'first and second step' must be numbers"));
+            onError.accept(
+                    DefaultI18nContext.getInstance().i18n("Invalid parameter 'first and second step' must be numbers"));
         }
     }
 
     private static ValidableTextField createValidableField() {
         ValidableTextField field = new ValidableTextField();
         field.setEnableInvalidStyle(true);
-        field.setErrorMessage(DefaultI18nContext.getInstance().i18n("Select the number of pages"));
+        field.setErrorMessage(DefaultI18nContext.getInstance().i18n("Select a valid number of pages"));
         field.setValidator(Validators.newPositiveIntegerString());
         field.setOnEnterValidation(true);
         field.setPrefWidth(50);
@@ -115,4 +127,5 @@ private void initialState() {
         this.firstStep.setText("1");
         this.secondStep.setText("1");
     }
+
 }
diff --git a/pdfsam-alternate-mix/src/test/java/org/pdfsam/alternatemix/AlternateMixOptionsPaneTest.java b/pdfsam-alternate-mix/src/test/java/org/pdfsam/alternatemix/AlternateMixOptionsPaneTest.java
@@ -30,9 +30,6 @@
 import java.util.Map;
 import java.util.function.Consumer;
 
-import javafx.scene.Parent;
-import javafx.scene.control.CheckBox;
-
 import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
@@ -43,6 +40,9 @@
 import org.pdfsam.test.ClearEventStudioRule;
 import org.pdfsam.ui.commons.ValidableTextField;
 
+import javafx.scene.Parent;
+import javafx.scene.control.CheckBox;
+
 /**
  * @author Andrea Vacondio
  *
@@ -82,6 +82,30 @@ public void invalidSecondStep() throws Exception {
         verify(onError).accept(anyString());
     }
 
+    @Test
+    public void invalidMaxFirstStep() throws Exception {
+        AlternateMixOptionsPane victim = find(".pdfsam-container");
+        victim.setFirstDocumentMaxPages(10);
+        doubleClick("#alternateMixFirstStep").type("30");
+        doubleClick("#alternateMixSecondStep").type('2');
+        FXTestUtils.invokeAndWait(() -> victim.apply(builder, onError), 2);
+        verify(onError).accept(anyString());
+        // tooltip to disapper
+        Thread.sleep(5500);
+    }
+
+    @Test
+    public void invalidMaxSecondStep() throws Exception {
+        AlternateMixOptionsPane victim = find(".pdfsam-container");
+        victim.setSecondDocumentMaxPages(10);
+        doubleClick("#alternateMixFirstStep").type('2');
+        doubleClick("#alternateMixSecondStep").type("30");
+        FXTestUtils.invokeAndWait(() -> victim.apply(builder, onError), 2);
+        verify(onError).accept(anyString());
+        // tooltip to disapper
+        Thread.sleep(5500);
+    }
+
     @Test
     public void validSteps() throws Exception {
         doubleClick("#alternateMixFirstStep").type('3');

diff --git a/pdfsam-core/src/main/java/org/pdfsam/support/validation/PositiveIntRangeStringValidator.java b/pdfsam-core/src/main/java/org/pdfsam/support/validation/PositiveIntRangeStringValidator.java
@@ -32,7 +32,8 @@ class PositiveIntRangeStringValidator extends PositiveIntegerStringValidator {
     private int lower = Integer.MIN_VALUE;
 
     public PositiveIntRangeStringValidator(int lower, int upper) {
-        require(lower > 0 && upper > 0, "Lower and upper limit cannot be negative numbers");
+        require(lower > 0 && upper > 0,
+                "Lower and upper limit cannot be negative numbers, lower=" + lower + " upper=" + upper);
         this.upper = upper;
         this.lower = lower;
     }

diff --git a/pdfsam-fx/src/main/java/org/pdfsam/ui/selection/single/SingleSelectionPane.java b/pdfsam-fx/src/main/java/org/pdfsam/ui/selection/single/SingleSelectionPane.java
@@ -99,12 +99,8 @@ public class SingleSelectionPane extends VBox implements ModuleOwned, PdfDocumen
     private MenuItem removeSelected;
 
     private Consumer<PdfDocumentDescriptor> onLoaded = d -> {
-        PdfDescriptorLoadingStatus status = d.loadingStatus().getValue();
-        if (status == PdfDescriptorLoadingStatus.LOADED
-                || status == PdfDescriptorLoadingStatus.LOADED_WITH_USER_PWD_DECRYPTION) {
-            eventStudio().broadcast(requestFallbackDestination(d.getFile(), getOwnerModule()), getOwnerModule());
-            eventStudio().broadcast(new ChangedSelectedPdfVersionEvent(d.getVersion()), getOwnerModule());
-        }
+        eventStudio().broadcast(requestFallbackDestination(d.getFile(), getOwnerModule()), getOwnerModule());
+        eventStudio().broadcast(new ChangedSelectedPdfVersionEvent(d.getVersion()), getOwnerModule());
     };
 
     private Consumer<PdfDocumentDescriptor> detailsUpdate = d -> {
@@ -130,7 +126,13 @@ public void accept(PdfDocumentDescriptor t) {
 
     private ChangeListener<PdfDescriptorLoadingStatus> onLoadingStatusChange = (o, oldVal, newVal) -> {
         if (descriptor != null & descriptor.hasReferences()) {
-            encryptionIndicatorUpdate.andThen(detailsUpdate).andThen(onLoaded).accept(descriptor);
+            encryptionIndicatorUpdate.andThen(detailsUpdate).andThen(d -> {
+                PdfDescriptorLoadingStatus status = d.loadingStatus().getValue();
+                if (status == PdfDescriptorLoadingStatus.LOADED
+                        || status == PdfDescriptorLoadingStatus.LOADED_WITH_USER_PWD_DECRYPTION) {
+                    onLoaded.accept(d);
+                }
+            }).accept(descriptor);
         }
     };
 

diff --git a/pdfsam-fx/src/test/java/org/pdfsam/ui/selection/single/SingleSelectionPaneTest.java b/pdfsam-fx/src/test/java/org/pdfsam/ui/selection/single/SingleSelectionPaneTest.java
@@ -179,6 +179,15 @@ public void additionalOnLoadedConsumer() throws Exception {
         assertTrue(consumer.isHit());
     }
 
+    @Test
+    public void additionalOnLoadedConsumerNotHit() throws Exception {
+        HitConsumer<PdfDocumentDescriptor> consumer = new HitConsumer<>();
+        SingleSelectionPane victim = find("#victim-selection-pane");
+        victim.addOnLoaded(consumer);
+        moveToEncrytedState(victim);
+        assertFalse(consumer.isHit());
+    }
+
     @Test
     public void onLoadedChangedSelectedPdfVersionEvent() throws Exception {
         Listener<ChangedSelectedPdfVersionEvent> listener = mock(Listener.class);
@@ -350,16 +359,10 @@ public void clickWithErrorsShowsLogStage() throws Exception {
 
     @Test
     public void clickEncryptedThrowsRequest() throws Exception {
-        typePathAndValidate();
         SingleSelectionPane victim = find("#victim-selection-pane");
         Listener<PdfLoadRequestEvent> listener = mock(Listener.class);
         eventStudio().add(PdfLoadRequestEvent.class, listener);
-        FXTestUtils.invokeAndWait(() -> {
-            victim.getPdfDocumentDescriptor().moveStatusTo(PdfDescriptorLoadingStatus.REQUESTED);
-            victim.getPdfDocumentDescriptor().moveStatusTo(PdfDescriptorLoadingStatus.LOADING);
-            victim.getPdfDocumentDescriptor().moveStatusTo(PdfDescriptorLoadingStatus.ENCRYPTED);
-        } , 2);
-
+        moveToEncrytedState(victim);
         click(".glyph-icon");
         type("pwd").click(DefaultI18nContext.getInstance().i18n("Unlock"));
         verify(listener, times(2)).onEvent(any());
@@ -403,6 +406,15 @@ private void moveToLoadedState(SingleSelectionPane victim) throws Exception {
         } , 2);
     }
 
+    private void moveToEncrytedState(SingleSelectionPane victim) throws Exception {
+        typePathAndValidate();
+        FXTestUtils.invokeAndWait(() -> {
+            victim.getPdfDocumentDescriptor().moveStatusTo(PdfDescriptorLoadingStatus.REQUESTED);
+            victim.getPdfDocumentDescriptor().moveStatusTo(PdfDescriptorLoadingStatus.LOADING);
+            victim.getPdfDocumentDescriptor().moveStatusTo(PdfDescriptorLoadingStatus.ENCRYPTED);
+        } , 2);
+    }
+
     private void moveToLoadedWithDecryption(SingleSelectionPane victim) throws Exception {
         typePathAndValidate();
         FXTestUtils.invokeAndWait(() -> {