From e212e7453fd612dbd018fe27fd612e2ba4aea749 Mon Sep 17 00:00:00 2001 From: Robert Kolmos Date: Wed, 25 Apr 2018 15:08:52 -0700 Subject: [PATCH 1/3] Implement TreeNavigator and add unit tests. --- .../research/domain/result/TaskResult.java | 19 + .../domain/task/navigation/StepNavigator.kt | 2 +- .../domain/task/navigation/TreeNavigator.java | 336 ++++++++++++++++++ .../research/domain/TreeNavigatorTests.java | 334 +++++++++++++++++ 4 files changed, 690 insertions(+), 1 deletion(-) create mode 100644 domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/TreeNavigator.java create mode 100644 domain/src/test/java/org/sagebionetworks/research/domain/TreeNavigatorTests.java diff --git a/domain/src/main/java/org/sagebionetworks/research/domain/result/TaskResult.java b/domain/src/main/java/org/sagebionetworks/research/domain/result/TaskResult.java index 8d7009383..af631c9f1 100644 --- a/domain/src/main/java/org/sagebionetworks/research/domain/result/TaskResult.java +++ b/domain/src/main/java/org/sagebionetworks/research/domain/result/TaskResult.java @@ -33,6 +33,8 @@ package org.sagebionetworks.research.domain.result; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import java.util.ArrayList; @@ -63,6 +65,23 @@ public TaskResult(@NonNull final String identifier, @NonNull UUID taskRunUUID, @ this.asyncResults = ImmutableSet.copyOf(asyncResults); } + /** + * Returns the first result in the step history with the given identifier. + * @param identifier The identifier to search for in the step history. + * @return the first result in the step history with the given identifier, or null if the + * identifier isn't found in the step history. + */ + @Nullable + public Result getResult(String identifier) { + for (Result result : this.stepHistory) { + if (result.getIdentifier().equals(identifier)) { + return result; + } + } + + return null; + } + @NonNull public UUID getTaskRunUUID() { return taskRunUUID; diff --git a/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/StepNavigator.kt b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/StepNavigator.kt index 2476fb4de..3a7d76880 100644 --- a/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/StepNavigator.kt +++ b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/StepNavigator.kt @@ -79,5 +79,5 @@ interface StepNavigator { * @param taskResult current step result * @return progress within the task */ - fun getProgress(step: Step, taskResult: TaskResult): Progress + fun getProgress(step: Step, taskResult: TaskResult): Progress? } diff --git a/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/TreeNavigator.java b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/TreeNavigator.java new file mode 100644 index 000000000..23ee50308 --- /dev/null +++ b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/TreeNavigator.java @@ -0,0 +1,336 @@ +package org.sagebionetworks.research.domain.task.navigation; + +import android.support.annotation.NonNull; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.sagebionetworks.research.domain.result.Result; +import org.sagebionetworks.research.domain.result.TaskResult; +import org.sagebionetworks.research.domain.step.SectionStep; +import org.sagebionetworks.research.domain.step.Step; +import org.sagebionetworks.research.domain.task.Task; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +public class TreeNavigator implements StepNavigator { + + // Stores the list of progressMarkers which are the identifiers of the steps which count + // towards computing the progress. An empty list represents the absence of progress markers, + // while null represents that the navigator should attempt to estimate the progress without the + // use of any progress markers. + @Nullable + private final ImmutableList progressMarkers; + + // The root of the tree navigator. + @NonNull + private final TreeNavigator.Node root; + + // A map from step identifier to step. + @NonNull + private final ImmutableMap stepsById; + + /** + * Constructs a TreeNavigator from the given list of steps, and the given progress markers + * @param steps The list of steps to construct this TreeNavigator from. + * @param progressMarkers The list of progressMarkers to construct this TreeNavigator from. + */ + public TreeNavigator(@NotNull List steps, @Nullable List progressMarkers) { + this.root = new Node(steps); + if (progressMarkers == null) { + this.progressMarkers = null; + } else { + ImmutableList.Builder builder = new ImmutableList.Builder<>(); + builder.addAll(progressMarkers); + this.progressMarkers = builder.build(); + } + this.stepsById = buildStepsByID(steps); + } + + /** + * Constructs and returns an ImmutableMap that maps step identifiers to steps for every step + * and every substep of every step recursively, in the given list of steps. + * @param steps The list of steps to construct the map from. + * @return An ImmutableMap that maps step identifiers to steps for every step and every substep + * of every step recursively, in the given list of steps. + */ + private static ImmutableMap buildStepsByID(List steps) { + ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); + for (Step step : steps) { + addStepToBuilderRecursively(step, builder); + } + + return builder.build(); + } + + /** + * Adds the given step and all of its substeps to the given builder. + * @param step The step to add (along with all of its substeps) to the given builder. + * @param builder The builder to add the given step to. + */ + private static void addStepToBuilderRecursively(Step step, ImmutableMap.Builder builder) { + if (step instanceof SectionStep) { + SectionStep sectionStep = (SectionStep)step; + builder.put(sectionStep.getIdentifier(), sectionStep); + for (Step childStep : sectionStep.getSteps()) { + addStepToBuilderRecursively(childStep, builder); + } + } else { + builder.put(step.getIdentifier(), step); + } + } + + @Nullable + @Override + public Step getStep(@NotNull String identifier) { + return this.stepsById.get(identifier); + } + + @Nullable + @Override + public Step getNextStep(@Nullable Step step, @NotNull TaskResult taskResult) { + return nextStepHelper(step, this.root, new AtomicBoolean(false)); + } + + /** + * Returns the first leaf step that appears after the given initialStep, in this TreeNavigator. + * After is defined as the next leaf in a pre-order traversal of the tree. + * @param initialStep The step to find the step after. + * @param current The step that is being evaluated by this call to nextStepHelper(). + * @param hasFoundInitial True if the initialStep has already been encountered by a parent + * recursive call of this call to nextStepHelper(), false otherwise. + * @return The first leaf step that appears after the given initialStep, or null if no such + * step exists. + */ + @Nullable + private static Step nextStepHelper(@Nullable Step initialStep, @Nullable Node current, + AtomicBoolean hasFoundInitial) { + if (current == null) { + return null; + } + + if (current.step != null) { + if (hasFoundInitial.get() && current.isLeaf()) { + return current.step; + } + + if (current.step.equals(initialStep)) { + hasFoundInitial.set(true); + } + } + + if (current.children != null) { + for (Node child : current.children) { + Step found = nextStepHelper(initialStep, child, hasFoundInitial); + if (found != null) { + return found; + } + } + } + + return null; + } + + @Nullable + @Override + public Step getPreviousStep(@NotNull Step step, @NotNull TaskResult taskResult) { + return previousStepHelper(step, this.root, new AtomicBoolean(false)); + } + + /** + * Returns the first leaf step that appears before the given initialStep, in this TreeNavigator. + * Before is defined as the next leaf in a reverse pre-order traversal of the tree. + * @param initialStep The step to find the step before. + * @param current The step that is being evaluated by this call to previousStepHelper(). + * @param hasFoundInitial True if the initialStep has already been encountered by a parent + * recursive call of this call to previousStepHelper(), false otherwise. + * @return The first leaf step that appears before the given initialStep, or null if no such + * step exists. + */ + @Nullable + private static Step previousStepHelper(@Nullable Step initialStep, @Nullable Node current, + AtomicBoolean hasFoundInitial) { + if (current == null) { + return null; + } + + if (current.step != null) { + if (hasFoundInitial.get() && current.isLeaf()) { + return current.step; + } + + if (current.step.equals(initialStep)) { + hasFoundInitial.set(true); + } + } + + if (current.children != null) { + for (Node child : current.children.reverse()) { + Step found = previousStepHelper(initialStep, child, hasFoundInitial); + if (found != null) { + return found; + } + } + } + + return null; + } + + @Nullable + @Override + public Task.Progress getProgress(@NonNull final Step step, @NonNull TaskResult taskResult) { + if (this.progressMarkers != null) { + // Get the list of steps that have executed, and add the current step in case it hasn't + // been added to the step history yet. + List stepIdentifiers = new ArrayList<>(); + for (Result result : taskResult.getStepHistory()) { + stepIdentifiers.add(result.getIdentifier()); + } + stepIdentifiers.add(step.getIdentifier()); + + int idx = this.getLastIndexInMarkers(stepIdentifiers); + if (idx == -1) { + return null; + } + + int current = idx + 1; + if (current == this.progressMarkers.size() && + !this.progressMarkers.contains(step.getIdentifier())) { + // If this is the last step in the progress markers and we are beyond the step we + // return null. + return null; + } + + // The progress is the current step index, out of the total number of markers, and it + // isn't estimated. + return new Task.Progress(current, this.progressMarkers.size(), false); + } + + // If there are no progress markers, default to using the total number of steps, and the + // result to figure out how many have gone. + ImmutableSet stepIDs = this.stepsById.keySet(); + Set finishedStepIDs = new HashSet<>(); + for (Result result : taskResult.getStepHistory()) { + finishedStepIDs.add(result.getIdentifier()); + } + + // The union total will store the number of elements in both sets. + int unionTotal = 0; + for (String stepID: finishedStepIDs) { + if (stepIDs.contains(stepID)) { + unionTotal++; + } + } + + // The number of unique elements across both sets is the total. + int total = stepIDs.size() + finishedStepIDs.size() - unionTotal; + // The current step hasn't been finished so we remove it. + finishedStepIDs.remove(step.getIdentifier()); + // We add one here because the progress should be 1 indexed. + int current = finishedStepIDs.size() + 1; + return new Task.Progress(current, total, true); + + } + + /** + * Returns the last index in the progress markers such that the given list of identifiers + * contains the progress marker at this index. + * @param identifiers The list of identifiers to check against the progress markers. + * @return The last index in the progress markers such that the given list of identifiers + * contains the progress marker at this index. + */ + private int getLastIndexInMarkers(List identifiers) { + if (this.progressMarkers != null) { + for (int i = this.progressMarkers.size() - 1; i >= 0; i--) { + if (identifiers.contains(progressMarkers.get(i))) { + return i; + } + } + } + + return -1; + } + + /* + * A class to represent a single node in the TreeNavigator + */ + private static final class Node { + // The step that this node represents + @Nullable + private final Step step; + + // The children if any that this node has. children.isEmpty() == false always, when + // children == null the node is a leaf. + @Nullable + private final ImmutableList children; + + /** + * Constructs a new node from the given list of steps. This node will represent the root + * of a Tree in which the steps are the children. + * @param steps The list of steps to construct the node from. + */ + private Node(@Nullable List steps) { + this.step = null; + this.children = constructChildNodes(steps); + } + + /** + * Constructs a new node corresponding to the given step. + * @param step the step to construct the node from. + */ + private Node(@NonNull Step step) { + this.step = step; + if (step instanceof SectionStep) { + this.children = constructChildNodes(((SectionStep)step).getSteps()); + } else { + this.children = null; + } + } + + /** + * Constructs nodes from all the steps in the given list and returns them + * in an ImmutableList. + * @param childSteps the list of steps to construct nodes from. + * @return An ImmutableList of nodes constructed from the given steps. + */ + private ImmutableList constructChildNodes(@Nullable List childSteps) { + if (childSteps != null && !childSteps.isEmpty()) { + List children = new ArrayList<>(); + for (Step childStep : childSteps) { + children.add(new Node(childStep)); + } + + ImmutableList.Builder builder = new ImmutableList.Builder<>(); + builder.addAll(children); + return builder.build(); + } + + return null; + } + + + /** + * Returns true if this node is a leaf, false otherwise. + * @return true if this node is a leaf, false otherwise. + */ + private boolean isLeaf() { + return this.children == null; + } + + @Override + public String toString() { + if (this.step != null) { + return this.step.toString() + ": " + "NODE"; + } else { + return "ROOT"; + } + } + } +} diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/TreeNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/TreeNavigatorTests.java new file mode 100644 index 000000000..a8f9957c5 --- /dev/null +++ b/domain/src/test/java/org/sagebionetworks/research/domain/TreeNavigatorTests.java @@ -0,0 +1,334 @@ +package org.sagebionetworks.research.domain; + +import com.google.common.collect.ImmutableList; + +import org.junit.Test; +import org.sagebionetworks.research.domain.result.Result; +import org.sagebionetworks.research.domain.result.TaskResult; +import org.sagebionetworks.research.domain.step.SectionStep; +import org.sagebionetworks.research.domain.step.Step; +import org.sagebionetworks.research.domain.step.StepType; +import org.sagebionetworks.research.domain.task.Task; +import org.sagebionetworks.research.domain.task.navigation.TreeNavigator; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertNull; +import static junit.framework.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class TreeNavigatorTests { + private static final String TEST_STEP_TYPE = "TEST_STEP"; + + private static Step mockStep(String identifier, String type) { + Step step = mock(Step.class); + when(step.getIdentifier()).thenReturn(identifier); + when(step.getType()).thenReturn(type); + when(step.toString()).thenReturn(identifier + ": " + type); + return step; + } + + private static SectionStep mockSectionStep(String identifier ,List substeps) { + SectionStep sectionStep = mock(SectionStep.class); + when(sectionStep.getSteps()).thenReturn(substeps); + when(sectionStep.getIdentifier()).thenReturn(identifier); + String type = StepType.SECTION; + when(sectionStep.getType()).thenReturn(type); + when(sectionStep.toString()).thenReturn(identifier + ": " + type); + return sectionStep; + } + + private static List createSteps(String[] identifiers) { + List steps = new ArrayList<>(); + for (String identifier : identifiers) { + steps.add(mockStep(identifier, TEST_STEP_TYPE)); + } + return steps; + } + + private static Result mockResult(Step step) { + Result result = mock(Result.class); + String identifier = step.getIdentifier(); + when(result.getIdentifier()).thenReturn(identifier); + String type = step.getType(); + when(result.getType()).thenReturn(type); + return result; + } + + private static TaskResult mockTaskResult(String identifier, List substeps) { + List subResults = new ArrayList<>(); + for (Step step : substeps) { + subResults.add(mockResult(step)); + } + return mockTaskResultFromResults(identifier, subResults); + } + + private static TaskResult mockTaskResultFromResults(String identifier, List subResults) { + TaskResult taskResult = mock(TaskResult.class); + for (Result stepResult : subResults) { + when(taskResult.getResult(stepResult.getIdentifier())).thenReturn(stepResult); + } + + ImmutableList.Builder builder = new ImmutableList.Builder<>(); + builder.addAll(subResults); + ImmutableList stepHistory = builder.build(); + when(taskResult.getStepHistory()).thenReturn(stepHistory); + when(taskResult.getIdentifier()).thenReturn(identifier); + return taskResult; + } + + private static final List TEST_STEPS; + private static final List TEST_PROGRESS_MARKERS; + private static final TreeNavigator TEST_NAVIGATOR; + static { + List steps = new ArrayList<>(createSteps(new String[]{"introduction", "step1", "step2", "step3"})); + steps.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.C"}))); + steps.add(mockSectionStep("step5", createSteps(new String[]{"step5.X", "step5.Y", "step5.Z"}))); + steps.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.C"}))); + steps.addAll(createSteps(new String[]{"step7", "completion"})); + TEST_STEPS = steps; + String[] progressMarkers = {"step1", "step2", "step3", "step4", "step5", "step6", "step7"}; + TEST_PROGRESS_MARKERS = Arrays.asList(progressMarkers); + TEST_NAVIGATOR = new TreeNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS); + } + + // NOTE: For all tests except for the progress ones, creating a TaskResult shouldn't change the behavior of + // the method being called. However, to ensure the test reflects the state the method will actually be called + // in a TaskResult is created anyway. + + // MARK: Test getProgress(). + + @Test + public void testProgess_NoMarkers_FlatHierarchy() { + List steps = createSteps(new String[]{"1", "2", "3", "4"}); + TreeNavigator navigator = new TreeNavigator(steps, null); + TaskResult taskResult = mockTaskResult("task", steps.subList(0, 1)); + + Task.Progress progress = navigator.getProgress(steps.get(1), taskResult); + assertNotNull(progress); + assertEquals(2, progress.getProgress()); + assertEquals(4, progress.getTotal()); + assertTrue(progress.isEstimated()); + } + + @Test + public void testProgress_MarkersAndSections_Introduction() { + TaskResult taskResult = mockTaskResult("task", new ArrayList()); + Task.Progress progress = TEST_NAVIGATOR.getProgress(TEST_STEPS.get(0), taskResult); + assertNull(progress); + } + + @Test + public void testProgress_MarkersAndSections_Step2() { + Step step = TEST_STEPS.get(2); + TaskResult taskResult = mockTaskResult("task", TEST_STEPS.subList(0, 2)); + Task.Progress progress = TEST_NAVIGATOR.getProgress(step, taskResult); + assertNotNull(progress); + assertEquals(2, progress.getProgress()); + assertEquals(7, progress.getTotal()); + assertFalse(progress.isEstimated()); + } + + @Test + public void testProgress_MarkersAndSections_Step7() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps())); + SectionStep step6 = (SectionStep) TEST_STEPS.get(6); + stepHistory.add(mockTaskResult(step6.getIdentifier(), step6.getSteps())); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Task.Progress progress = TEST_NAVIGATOR.getProgress(TEST_STEPS.get(7), taskResult); + assertNotNull(progress); + assertEquals(7, progress.getProgress()); + assertEquals(7, progress.getTotal()); + assertFalse(progress.isEstimated()); + } + + @Test + public void testProgress_MarkersAndSections_Step4B() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + List step4Substeps = step4.getSteps(); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4Substeps.subList(0, 1))); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Task.Progress progress = TEST_NAVIGATOR.getProgress(step4.getSteps().get(1), taskResult); + assertNotNull(progress); + assertEquals(4, progress.getProgress()); + assertEquals(7, progress.getTotal()); + assertFalse(progress.isEstimated()); + } + + @Test + public void testProgress_MarkersAndSections_Step5X() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), new ArrayList())); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Task.Progress progress = TEST_NAVIGATOR.getProgress(step5.getSteps().get(0), taskResult); + assertNotNull(progress); + assertEquals(5, progress.getProgress()); + assertEquals(7, progress.getTotal()); + assertFalse(progress.isEstimated()); + } + + @Test + public void testProgress_MarkersAndSections_Step6C() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps())); + SectionStep step6 = (SectionStep) TEST_STEPS.get(6); + stepHistory.add(mockTaskResult(step6.getIdentifier(), step6.getSteps().subList(0, 2))); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Task.Progress progress = TEST_NAVIGATOR.getProgress(step6.getSteps().get(2), taskResult); + assertNotNull(progress); + assertEquals(6, progress.getProgress()); + assertEquals(7, progress.getTotal()); + assertFalse(progress.isEstimated()); + } + + @Test + public void testProgress_MarkersAndSections_Completion() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps())); + SectionStep step6 = (SectionStep) TEST_STEPS.get(6); + stepHistory.add(mockTaskResult(step6.getIdentifier(), step6.getSteps())); + stepHistory.add(mockResult(TEST_STEPS.get(7))); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Task.Progress progress = TEST_NAVIGATOR.getProgress(TEST_STEPS.get(8), taskResult); + assertNull(progress); + } + + // MARK: test getPreviousStep() + + @Test + public void testBack_From5X() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), new ArrayList())); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Step previousStep = TEST_NAVIGATOR.getPreviousStep(step5.getSteps().get(0), taskResult); + assertNotNull(previousStep); + assertEquals("step4.C", previousStep.getIdentifier()); + } + + @Test + public void testBack_From5Z() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps().subList(0, 2))); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Step previousStep = TEST_NAVIGATOR.getPreviousStep(step5.getSteps().get(2), taskResult); + assertNotNull(previousStep); + assertEquals("step5.Y", previousStep.getIdentifier()); + } + + @Test + public void testBack_From3() { + TaskResult taskResult = mockTaskResult("task", TEST_STEPS.subList(1, 3)); + Step previousStep = TEST_NAVIGATOR.getPreviousStep(TEST_STEPS.get(3), taskResult); + assertNotNull(previousStep); + assertEquals("step2", previousStep.getIdentifier()); + } + + // MARK: test getNextStep() + + @Test + public void testForward_From5X() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), new ArrayList())); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Step nextStep = TEST_NAVIGATOR.getNextStep(step5.getSteps().get(0), taskResult); + assertNotNull(nextStep); + assertEquals("step5.Y", nextStep.getIdentifier()); + } + + @Test + public void testForward_From5Z() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(TEST_STEPS.get(i))); + } + + SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); + SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps().subList(0, 2))); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + Step nextStep = TEST_NAVIGATOR.getNextStep(step5.getSteps().get(2), taskResult); + assertNotNull(nextStep); + assertEquals("step6.A", nextStep.getIdentifier()); + } + + @Test + public void testForward_From2() { + TaskResult taskResult = mockTaskResult("task", TEST_STEPS.subList(0, 2)); + Step nextStep = TEST_NAVIGATOR.getNextStep(TEST_STEPS.get(2), taskResult); + assertNotNull(nextStep); + assertEquals("step3", nextStep.getIdentifier()); + } + + + // MARK: test getStep() + + @Test + public void testGetStep() { + assertEquals(TEST_STEPS.get(0), TEST_NAVIGATOR.getStep("introduction")); + assertEquals(((SectionStep)TEST_STEPS.get(4)).getSteps().get(2), TEST_NAVIGATOR.getStep("step4.C")); + assertEquals(TEST_STEPS.get(7), TEST_NAVIGATOR.getStep("step7")); + assertEquals(TEST_STEPS.get(5), TEST_NAVIGATOR.getStep("step5")); + } +} From 08317218296aa6895b48c9dd31191344223f1dcb Mon Sep 17 00:00:00 2001 From: Robert Kolmos Date: Wed, 25 Apr 2018 18:21:47 -0700 Subject: [PATCH 2/3] Implement StrategyBasedNavigator and add tests. --- .../strategy/StrategyBasedNavigator.java | 132 ++++---- .../domain/navigation/AllNavigatorTests.java | 12 + .../IndividualNavigatorTests.java} | 190 ++++++------ .../StrategyBasedNavigatorTests.java | 291 ++++++++++++++++++ .../research/domain/navigation/TestStep.java | 8 + .../domain/navigation/TreeNavigatorTests.java | 37 +++ 6 files changed, 502 insertions(+), 168 deletions(-) create mode 100644 domain/src/test/java/org/sagebionetworks/research/domain/navigation/AllNavigatorTests.java rename domain/src/test/java/org/sagebionetworks/research/domain/{TreeNavigatorTests.java => navigation/IndividualNavigatorTests.java} (63%) create mode 100644 domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java create mode 100644 domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java create mode 100644 domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java diff --git a/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java index c853ad8db..af7a0e81f 100644 --- a/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java +++ b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java @@ -38,111 +38,103 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; + +import org.sagebionetworks.research.domain.result.Result; import org.sagebionetworks.research.domain.result.TaskResult; import org.sagebionetworks.research.domain.step.Step; +import org.sagebionetworks.research.domain.step.ui.action.ActionType.Navigation; import org.sagebionetworks.research.domain.task.Task.Progress; import org.sagebionetworks.research.domain.task.navigation.StepNavigator; +import org.sagebionetworks.research.domain.task.navigation.TreeNavigator; import org.sagebionetworks.research.domain.task.navigation.strategy.ConditionalStepNavigationStrategy.ReplacementStepNavigationStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.BackStepStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy; import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy.Identifiers; import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.SkipStepStrategy; -public class StrategyBasedNavigator implements StepNavigator { - @NonNull - private final ImmutableList steps; - - @Nullable - private final ConditionalStepNavigationStrategy conditionalRule; +import java.util.List; +public class StrategyBasedNavigator implements StepNavigator { + // The tree navigator that backs up this StrategyBasedNavigator whenever the various navigation rules aren't + // applicable. @NonNull - private final ImmutableMap stepsById; - - public StrategyBasedNavigator(@NonNull final ImmutableList steps, - @Nullable final ConditionalStepNavigationStrategy conditionalRule) { - this.steps = steps; - this.conditionalRule = conditionalRule; - - Builder mapBuilder = ImmutableMap.builderWithExpectedSize(this.steps.size()); - for (Step step : this.steps) { - mapBuilder.put(step.getIdentifier(), step); - } - this.stepsById = mapBuilder.build(); + private final TreeNavigator treeNavigator; + + /** + * Constructs a new StrategyBasedNavigator from the given list of steps, and the given list of progress markers. + * @param steps The list of steps to create this StepBasedNavigator from. + * @param progressMarkers The list of progress markers to create this StepBasedNavigator from. + */ + public StrategyBasedNavigator(@NonNull final List steps, @Nullable List progressMarkers) { + this.treeNavigator = new TreeNavigator(steps, progressMarkers); } @Override public Step getNextStep(final Step step, @NonNull TaskResult taskResult) { Step nextStep = null; - Step previousStep = step; - boolean shouldSkip; - do { - shouldSkip = false; - if (previousStep == null) { - nextStep = steps.get(0); - } else { - String nextStepIdentifier = getNextStepIdentifier(previousStep, taskResult); - if (nextStepIdentifier != null) { - if (Identifiers.Exit.getKey().equals(nextStepIdentifier)) { - return null; - } else { - nextStep = getStep(nextStepIdentifier); - } - } - if (nextStep == null) { - int indexOfNextStep = steps.indexOf(previousStep) + 1; - if (indexOfNextStep < steps.size()) { - return steps.get(indexOfNextStep); - } - } - } - - String nextStepIdentifier = conditionalRule.skipToStep(previousStep, taskResult); - if (Identifiers.NextStep.getKey().equals(nextStepIdentifier)) { - shouldSkip = true; - } else { - nextStep = getStep(nextStepIdentifier); + // First we try to get the next step from the step by casting it to a NextStepStrategy. + if (step instanceof NextStepStrategy) { + String nextStepId = ((NextStepStrategy)step).getNextStepIdentifier(taskResult); + if (nextStepId != null) { + nextStep = this.getStep(nextStepId); } + } - if (!shouldSkip && nextStep != null && step instanceof SkipStepStrategy) { - shouldSkip = ((SkipStepStrategy) step).shouldSkip(taskResult); - } + // If we don't get a valid step from casting to a NextStepStrategy we default to using the tree navigator to + // get the next step. + if (nextStep == null) { + nextStep = treeNavigator.getNextStep(step, taskResult); + } - if (shouldSkip) { - previousStep = nextStep; + if (nextStep != null) { + // As long as the next step we have found shouldn't be skipped we return it. + if (!(nextStep instanceof SkipStepStrategy) || + !((SkipStepStrategy)nextStep).shouldSkip(taskResult)) { + return nextStep; } - } while (shouldSkip); - if (conditionalRule instanceof ReplacementStepNavigationStrategy) { - nextStep = ((ReplacementStepNavigationStrategy) conditionalRule).getReplacementStep(nextStep, taskResult); + // If we should skip the next step we found, we recurse on the next step to get the one after that. + return getNextStep(nextStep, taskResult); } - return nextStep; - } - @Nullable - @VisibleForTesting - String getNextStepIdentifier(@NonNull Step step, @NonNull TaskResult taskResult) { - String nextStepIdentifier; - if (step instanceof StepNavigationStrategy.NextStepStrategy) { - StepNavigationStrategy.NextStepStrategy nextRule = (StepNavigationStrategy.NextStepStrategy) step; - nextStepIdentifier = nextRule.getNextStepIdentifier(taskResult); - } else { - nextStepIdentifier = conditionalRule.nextStepIdentifier(step, taskResult); - } - return nextStepIdentifier; + // If the tree navigator returns null we also return null. + return null; } @Override public Step getPreviousStep(@NonNull final Step step, @NonNull TaskResult taskResult) { - return null; + // First we make sure that the given step allows backward navigation. + if (step instanceof BackStepStrategy && !((BackStepStrategy)step).isBackAllowed(taskResult)) { + return null; + } + + // If backward navigation is allowed we check the result. + Step previousStep = null; + List stepHistory = taskResult.getStepHistory(); + int idx = stepHistory.indexOf(step); + if (idx > 0) { + String previousStepId = stepHistory.get(idx - 1).getIdentifier(); + previousStep = this.getStep(previousStepId); + } + + // If the task result doesn't give us a previous step to go back to we default to using the tree navigator + // to get a previous step. + if (previousStep == null) { + previousStep = this.treeNavigator.getPreviousStep(step, taskResult); + } + + return previousStep; } @NonNull @Override public Progress getProgress(@NonNull final Step step, @NonNull TaskResult taskResult) { - return null; + return this.treeNavigator.getProgress(step, taskResult); } @Override @Nullable public Step getStep(@NonNull final String identifier) { - return stepsById.get(identifier); + return this.treeNavigator.getStep(identifier); } } diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/AllNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/AllNavigatorTests.java new file mode 100644 index 000000000..55666fe3c --- /dev/null +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/AllNavigatorTests.java @@ -0,0 +1,12 @@ +package org.sagebionetworks.research.domain.navigation; + +import org.junit.runner.*; +import org.junit.runners.*; +import org.junit.runners.Suite.*; +import org.sagebionetworks.research.domain.task.navigation.strategy.StrategyBasedNavigator; + +@RunWith(Suite.class) +@SuiteClasses({ TreeNavigatorTests.class, StrategyBasedNavigatorTests.class }) +public class AllNavigatorTests { + +} diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/TreeNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java similarity index 63% rename from domain/src/test/java/org/sagebionetworks/research/domain/TreeNavigatorTests.java rename to domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java index a8f9957c5..5c2035414 100644 --- a/domain/src/test/java/org/sagebionetworks/research/domain/TreeNavigatorTests.java +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java @@ -1,15 +1,16 @@ -package org.sagebionetworks.research.domain; +package org.sagebionetworks.research.domain.navigation; import com.google.common.collect.ImmutableList; -import org.junit.Test; +import org.junit.*; import org.sagebionetworks.research.domain.result.Result; import org.sagebionetworks.research.domain.result.TaskResult; import org.sagebionetworks.research.domain.step.SectionStep; import org.sagebionetworks.research.domain.step.Step; import org.sagebionetworks.research.domain.step.StepType; import org.sagebionetworks.research.domain.task.Task; -import org.sagebionetworks.research.domain.task.navigation.TreeNavigator; +import org.sagebionetworks.research.domain.task.Task.Progress; +import org.sagebionetworks.research.domain.task.navigation.StepNavigator; import java.util.ArrayList; import java.util.Arrays; @@ -19,22 +20,41 @@ import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.assertNull; -import static junit.framework.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; -public class TreeNavigatorTests { - private static final String TEST_STEP_TYPE = "TEST_STEP"; +public abstract class IndividualNavigatorTests { - private static Step mockStep(String identifier, String type) { + private static final String STEP_TYPE = "step"; + public static final List TEST_STEPS; + public static final List TEST_PROGRESS_MARKERS; + static { + List steps = new ArrayList<>(createSteps(new String[]{"introduction", "step1", "step2", "step3"})); + steps.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.C"}))); + steps.add(mockSectionStep("step5", createSteps(new String[]{"step5.X", "step5.Y", "step5.Z"}))); + steps.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.C"}))); + steps.addAll(createSteps(new String[]{"step7", "completion"})); + TEST_STEPS = steps; + String[] progressMarkers = {"step1", "step2", "step3", "step4", "step5", "step6", "step7"}; + TEST_PROGRESS_MARKERS = Arrays.asList(progressMarkers); + } + + private final StepNavigator navigator; + private final List steps; + + public IndividualNavigatorTests(StepNavigator navigator) { + this.navigator = navigator; + this.steps = TEST_STEPS; + } + + public static Step mockStep(String identifier) { Step step = mock(Step.class); when(step.getIdentifier()).thenReturn(identifier); - when(step.getType()).thenReturn(type); - when(step.toString()).thenReturn(identifier + ": " + type); + when(step.getType()).thenReturn(STEP_TYPE); + when(step.toString()).thenReturn(identifier + ": " + STEP_TYPE); return step; } - private static SectionStep mockSectionStep(String identifier ,List substeps) { + public static SectionStep mockSectionStep(String identifier ,List substeps) { SectionStep sectionStep = mock(SectionStep.class); when(sectionStep.getSteps()).thenReturn(substeps); when(sectionStep.getIdentifier()).thenReturn(identifier); @@ -44,15 +64,15 @@ private static SectionStep mockSectionStep(String identifier ,List substep return sectionStep; } - private static List createSteps(String[] identifiers) { + public static List createSteps(String[] identifiers) { List steps = new ArrayList<>(); for (String identifier : identifiers) { - steps.add(mockStep(identifier, TEST_STEP_TYPE)); + steps.add(mockStep(identifier)); } return steps; } - private static Result mockResult(Step step) { + public static Result mockResult(Step step) { Result result = mock(Result.class); String identifier = step.getIdentifier(); when(result.getIdentifier()).thenReturn(identifier); @@ -61,15 +81,17 @@ private static Result mockResult(Step step) { return result; } - private static TaskResult mockTaskResult(String identifier, List substeps) { + public static TaskResult mockTaskResult(String identifier, List substeps) { List subResults = new ArrayList<>(); - for (Step step : substeps) { - subResults.add(mockResult(step)); + if (substeps != null) { + for (Step step : substeps) { + subResults.add(mockResult(step)); + } } return mockTaskResultFromResults(identifier, subResults); } - private static TaskResult mockTaskResultFromResults(String identifier, List subResults) { + public static TaskResult mockTaskResultFromResults(String identifier, List subResults) { TaskResult taskResult = mock(TaskResult.class); for (Result stepResult : subResults) { when(taskResult.getResult(stepResult.getIdentifier())).thenReturn(stepResult); @@ -83,52 +105,24 @@ private static TaskResult mockTaskResultFromResults(String identifier, List TEST_STEPS; - private static final List TEST_PROGRESS_MARKERS; - private static final TreeNavigator TEST_NAVIGATOR; - static { - List steps = new ArrayList<>(createSteps(new String[]{"introduction", "step1", "step2", "step3"})); - steps.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.C"}))); - steps.add(mockSectionStep("step5", createSteps(new String[]{"step5.X", "step5.Y", "step5.Z"}))); - steps.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.C"}))); - steps.addAll(createSteps(new String[]{"step7", "completion"})); - TEST_STEPS = steps; - String[] progressMarkers = {"step1", "step2", "step3", "step4", "step5", "step6", "step7"}; - TEST_PROGRESS_MARKERS = Arrays.asList(progressMarkers); - TEST_NAVIGATOR = new TreeNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS); - } - // NOTE: For all tests except for the progress ones, creating a TaskResult shouldn't change the behavior of // the method being called. However, to ensure the test reflects the state the method will actually be called // in a TaskResult is created anyway. // MARK: Test getProgress(). - @Test - public void testProgess_NoMarkers_FlatHierarchy() { - List steps = createSteps(new String[]{"1", "2", "3", "4"}); - TreeNavigator navigator = new TreeNavigator(steps, null); - TaskResult taskResult = mockTaskResult("task", steps.subList(0, 1)); - - Task.Progress progress = navigator.getProgress(steps.get(1), taskResult); - assertNotNull(progress); - assertEquals(2, progress.getProgress()); - assertEquals(4, progress.getTotal()); - assertTrue(progress.isEstimated()); - } - @Test public void testProgress_MarkersAndSections_Introduction() { TaskResult taskResult = mockTaskResult("task", new ArrayList()); - Task.Progress progress = TEST_NAVIGATOR.getProgress(TEST_STEPS.get(0), taskResult); + Progress progress = navigator.getProgress(steps.get(0), taskResult); assertNull(progress); } @Test public void testProgress_MarkersAndSections_Step2() { - Step step = TEST_STEPS.get(2); - TaskResult taskResult = mockTaskResult("task", TEST_STEPS.subList(0, 2)); - Task.Progress progress = TEST_NAVIGATOR.getProgress(step, taskResult); + Step step = steps.get(2); + TaskResult taskResult = mockTaskResult("task", steps.subList(0, 2)); + Task.Progress progress = navigator.getProgress(step, taskResult); assertNotNull(progress); assertEquals(2, progress.getProgress()); assertEquals(7, progress.getTotal()); @@ -139,17 +133,17 @@ public void testProgress_MarkersAndSections_Step2() { public void testProgress_MarkersAndSections_Step7() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps())); - SectionStep step6 = (SectionStep) TEST_STEPS.get(6); + SectionStep step6 = (SectionStep) steps.get(6); stepHistory.add(mockTaskResult(step6.getIdentifier(), step6.getSteps())); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Task.Progress progress = TEST_NAVIGATOR.getProgress(TEST_STEPS.get(7), taskResult); + Task.Progress progress = navigator.getProgress(steps.get(7), taskResult); assertNotNull(progress); assertEquals(7, progress.getProgress()); assertEquals(7, progress.getTotal()); @@ -160,14 +154,14 @@ public void testProgress_MarkersAndSections_Step7() { public void testProgress_MarkersAndSections_Step4B() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); List step4Substeps = step4.getSteps(); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4Substeps.subList(0, 1))); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Task.Progress progress = TEST_NAVIGATOR.getProgress(step4.getSteps().get(1), taskResult); + Task.Progress progress = navigator.getProgress(step4.getSteps().get(1), taskResult); assertNotNull(progress); assertEquals(4, progress.getProgress()); assertEquals(7, progress.getTotal()); @@ -178,15 +172,15 @@ public void testProgress_MarkersAndSections_Step4B() { public void testProgress_MarkersAndSections_Step5X() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), new ArrayList())); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Task.Progress progress = TEST_NAVIGATOR.getProgress(step5.getSteps().get(0), taskResult); + Task.Progress progress = navigator.getProgress(step5.getSteps().get(0), taskResult); assertNotNull(progress); assertEquals(5, progress.getProgress()); assertEquals(7, progress.getTotal()); @@ -197,17 +191,17 @@ public void testProgress_MarkersAndSections_Step5X() { public void testProgress_MarkersAndSections_Step6C() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps())); - SectionStep step6 = (SectionStep) TEST_STEPS.get(6); + SectionStep step6 = (SectionStep) steps.get(6); stepHistory.add(mockTaskResult(step6.getIdentifier(), step6.getSteps().subList(0, 2))); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Task.Progress progress = TEST_NAVIGATOR.getProgress(step6.getSteps().get(2), taskResult); + Task.Progress progress = navigator.getProgress(step6.getSteps().get(2), taskResult); assertNotNull(progress); assertEquals(6, progress.getProgress()); assertEquals(7, progress.getTotal()); @@ -218,18 +212,18 @@ public void testProgress_MarkersAndSections_Step6C() { public void testProgress_MarkersAndSections_Completion() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps())); - SectionStep step6 = (SectionStep) TEST_STEPS.get(6); + SectionStep step6 = (SectionStep) steps.get(6); stepHistory.add(mockTaskResult(step6.getIdentifier(), step6.getSteps())); - stepHistory.add(mockResult(TEST_STEPS.get(7))); + stepHistory.add(mockResult(steps.get(7))); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Task.Progress progress = TEST_NAVIGATOR.getProgress(TEST_STEPS.get(8), taskResult); + Task.Progress progress = navigator.getProgress(steps.get(8), taskResult); assertNull(progress); } @@ -239,15 +233,15 @@ public void testProgress_MarkersAndSections_Completion() { public void testBack_From5X() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), new ArrayList())); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Step previousStep = TEST_NAVIGATOR.getPreviousStep(step5.getSteps().get(0), taskResult); + Step previousStep = navigator.getPreviousStep(step5.getSteps().get(0), taskResult); assertNotNull(previousStep); assertEquals("step4.C", previousStep.getIdentifier()); } @@ -256,23 +250,23 @@ public void testBack_From5X() { public void testBack_From5Z() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps().subList(0, 2))); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Step previousStep = TEST_NAVIGATOR.getPreviousStep(step5.getSteps().get(2), taskResult); + Step previousStep = navigator.getPreviousStep(step5.getSteps().get(2), taskResult); assertNotNull(previousStep); assertEquals("step5.Y", previousStep.getIdentifier()); } @Test public void testBack_From3() { - TaskResult taskResult = mockTaskResult("task", TEST_STEPS.subList(1, 3)); - Step previousStep = TEST_NAVIGATOR.getPreviousStep(TEST_STEPS.get(3), taskResult); + TaskResult taskResult = mockTaskResult("task", steps.subList(1, 3)); + Step previousStep = navigator.getPreviousStep(steps.get(3), taskResult); assertNotNull(previousStep); assertEquals("step2", previousStep.getIdentifier()); } @@ -283,15 +277,15 @@ public void testBack_From3() { public void testForward_From5X() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), new ArrayList())); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Step nextStep = TEST_NAVIGATOR.getNextStep(step5.getSteps().get(0), taskResult); + Step nextStep = navigator.getNextStep(step5.getSteps().get(0), taskResult); assertNotNull(nextStep); assertEquals("step5.Y", nextStep.getIdentifier()); } @@ -300,23 +294,23 @@ public void testForward_From5X() { public void testForward_From5Z() { List stepHistory = new ArrayList<>(); for (int i = 0; i < 3; i++) { - stepHistory.add(mockResult(TEST_STEPS.get(i))); + stepHistory.add(mockResult(steps.get(i))); } - SectionStep step4 = (SectionStep) TEST_STEPS.get(4); + SectionStep step4 = (SectionStep) steps.get(4); stepHistory.add(mockTaskResult(step4.getIdentifier(), step4.getSteps())); - SectionStep step5 = (SectionStep) TEST_STEPS.get(5); + SectionStep step5 = (SectionStep) steps.get(5); stepHistory.add(mockTaskResult(step5.getIdentifier(), step5.getSteps().subList(0, 2))); TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); - Step nextStep = TEST_NAVIGATOR.getNextStep(step5.getSteps().get(2), taskResult); + Step nextStep = navigator.getNextStep(step5.getSteps().get(2), taskResult); assertNotNull(nextStep); assertEquals("step6.A", nextStep.getIdentifier()); } @Test public void testForward_From2() { - TaskResult taskResult = mockTaskResult("task", TEST_STEPS.subList(0, 2)); - Step nextStep = TEST_NAVIGATOR.getNextStep(TEST_STEPS.get(2), taskResult); + TaskResult taskResult = mockTaskResult("task", steps.subList(0, 2)); + Step nextStep = navigator.getNextStep(steps.get(2), taskResult); assertNotNull(nextStep); assertEquals("step3", nextStep.getIdentifier()); } @@ -326,9 +320,9 @@ public void testForward_From2() { @Test public void testGetStep() { - assertEquals(TEST_STEPS.get(0), TEST_NAVIGATOR.getStep("introduction")); - assertEquals(((SectionStep)TEST_STEPS.get(4)).getSteps().get(2), TEST_NAVIGATOR.getStep("step4.C")); - assertEquals(TEST_STEPS.get(7), TEST_NAVIGATOR.getStep("step7")); - assertEquals(TEST_STEPS.get(5), TEST_NAVIGATOR.getStep("step5")); + assertEquals(steps.get(0), navigator.getStep("introduction")); + assertEquals(((SectionStep) steps.get(4)).getSteps().get(2), navigator.getStep("step4.C")); + assertEquals(steps.get(7), navigator.getStep("step7")); + assertEquals(steps.get(5), navigator.getStep("step5")); } } diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java new file mode 100644 index 000000000..f19ebaddf --- /dev/null +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java @@ -0,0 +1,291 @@ +package org.sagebionetworks.research.domain.navigation; + +import org.junit.*; +import org.mockito.*; +import org.sagebionetworks.research.domain.result.Result; +import org.sagebionetworks.research.domain.result.TaskResult; +import org.sagebionetworks.research.domain.step.SectionStep; +import org.sagebionetworks.research.domain.step.Step; +import org.sagebionetworks.research.domain.task.Task; +import org.sagebionetworks.research.domain.task.navigation.strategy.StrategyBasedNavigator; + +import java.util.ArrayList; +import java.util.List; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertNull; +import static junit.framework.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.*; + +public class StrategyBasedNavigatorTests extends IndividualNavigatorTests { + private static final String SKIP_RESULT_IDENTIFIER = "skip"; + private static final String TEST_STEP_TYPE = "test step"; + + private static Step mockTestStep(String identifier, boolean isBackAllowed, String nextStepId, + boolean shouldAddSkipRule) { + TestStep step = mock(TestStep.class); + when(step.getIdentifier()).thenReturn(identifier); + when(step.getType()).thenReturn(TEST_STEP_TYPE); + when(step.toString()).thenReturn(identifier + ": " + TEST_STEP_TYPE); + when(step.isBackAllowed(any(TaskResult.class))).thenReturn(isBackAllowed); + when(step.getNextStepIdentifier(any(TaskResult.class))).thenReturn(nextStepId); + if (shouldAddSkipRule) { + // If we are adding a skip rule, return true for any argument in which the given TaskResult contains a + // result with the SKIP_RESULT_IDENTIFIER, false otherwise. + TaskResult match = argThat(new ArgumentMatcher() { + @Override + public boolean matches(final TaskResult argument) { + return argument != null && argument.getResult(SKIP_RESULT_IDENTIFIER) != null; + } + }); + when(step.shouldSkip(match)).thenReturn(true); + TaskResult notMatch = argThat(new ArgumentMatcher() { + @Override + public boolean matches(final TaskResult argument) { + return argument != null && argument.getResult(SKIP_RESULT_IDENTIFIER) == null; + } + }); + when(step.shouldSkip(notMatch)).thenReturn(false); + } else { + // If we aren't adding a skip rule we always return false. + when(step.shouldSkip(any(TaskResult.class))).thenReturn(false); + } + + return step; + } + + public StrategyBasedNavigatorTests() { + super(new StrategyBasedNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); + } + + // MARK: Test getProgress(). + + @Test + public void testProgess_NoMarkers_FlatHierarchy() { + List steps = createSteps(new String[]{"1", "2", "3", "4"}); + StrategyBasedNavigator navigator = new StrategyBasedNavigator(steps,null); + TaskResult taskResult = mockTaskResult("task", steps.subList(0, 1)); + + Task.Progress progress = navigator.getProgress(steps.get(1), taskResult); + assertNotNull(progress); + assertEquals(2, progress.getProgress()); + assertEquals(4, progress.getTotal()); + assertTrue(progress.isEstimated()); + } + + private TaskResult mockSectionResult(Step step, int from, int to) { + SectionStep sectionStep = (SectionStep) step; + return mockTaskResult(sectionStep.getIdentifier(), sectionStep.getSteps().subList(from, to)); + } + + private static final List STRATEGY_TEST_STEPS; + private static final StrategyBasedNavigator STRATEGY_TEST_NAVIGATOR; + static { + STRATEGY_TEST_STEPS = new ArrayList<>(); + STRATEGY_TEST_STEPS.add(mockTestStep("introduction", true, "step2", false)); + STRATEGY_TEST_STEPS.add(mockStep("step1")); + STRATEGY_TEST_STEPS.add(mockTestStep("step2", true, null, true)); + STRATEGY_TEST_STEPS.add(mockTestStep("step3", true, "step1", false)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.c"}))); + List step5Substeps = new ArrayList<>(); + step5Substeps.add(mockTestStep("step5.X", true, null, true)); + step5Substeps.add(mockTestStep("step5.Y", true, "step7", false)); + step5Substeps.add(mockTestStep("step5.Z", true, "step4.B", false)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step5", step5Substeps)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.c"}))); + STRATEGY_TEST_STEPS.add(mockTestStep("step7", false, "step6.A", false)); + STRATEGY_TEST_STEPS.add(mockStep("conclusion")); + STRATEGY_TEST_NAVIGATOR = new StrategyBasedNavigator(STRATEGY_TEST_STEPS, TEST_PROGRESS_MARKERS); + } + + // MARK: test skip rule + + @Test + public void testSkip_resultPresent_From2() { + List stepHistory = new ArrayList<>(); + stepHistory.add(mockTaskResult(SKIP_RESULT_IDENTIFIER, null)); + for (int i = 0; i < 2; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // From step1 we should go directly to step 3 since step 2 should be skipped. + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(STRATEGY_TEST_STEPS.get(1), taskResult); + assertNotNull(nextStep); + assertEquals("step3", nextStep.getIdentifier()); + } + + @Test + public void testSkip_resultNotPresent_From2() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 2; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // From step1, step2 shouldn't be skipped so we should end up there. + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(STRATEGY_TEST_STEPS.get(1), taskResult); + assertNotNull(nextStep); + assertEquals("step2", nextStep.getIdentifier()); + } + + @Test + public void testSkip_resultPresent_From5X() { + List stepHistory = new ArrayList<>(); + stepHistory.add(mockTaskResult(SKIP_RESULT_IDENTIFIER, new ArrayList())); + for (int i = 0; i < 4; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(4), 0, 2)); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // From step4.C, we should go directly to step5.Y since step5.X should be skipped. + SectionStep step4 = (SectionStep)STRATEGY_TEST_STEPS.get(4); + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(step4.getSteps().get(2), taskResult); + assertNotNull(nextStep); + assertEquals("step5.Y", nextStep.getIdentifier()); + } + + @Test + public void testSkip_resultNotPresent_From5X() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 4; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(4), 0, 2)); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // From step4.C, we should go directly to step5.X since this step shouldn't be skipped. + SectionStep step4 = (SectionStep)STRATEGY_TEST_STEPS.get(4); + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(step4.getSteps().get(2), taskResult); + assertNotNull(nextStep); + assertEquals("step5.X", nextStep.getIdentifier()); + } + + // MARK: test back rule + + @Test + public void testBackAllowed_From2() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 2; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // Step 2 should be able to skip backward and should go to step 1. + Step previousStep = STRATEGY_TEST_NAVIGATOR.getPreviousStep(STRATEGY_TEST_STEPS.get(2), taskResult); + assertNotNull(previousStep); + assertEquals("step1", previousStep.getIdentifier()); + } + + @Test + public void testBackAllowed_From3() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // Step 3 allows skipping back so we should go to step2. + Step previousStep = STRATEGY_TEST_NAVIGATOR.getPreviousStep(STRATEGY_TEST_STEPS.get(3), taskResult); + assertNotNull(previousStep); + assertEquals("step2", previousStep.getIdentifier()); + } + + @Test + public void testBackNotAllowed_From7() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 4; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + for (int i = 4; i < 7; i++) { + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(i), 0, 3)); + } + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // Step 7 doesn't allow going backward so we should get null + Step previousStep = STRATEGY_TEST_NAVIGATOR.getPreviousStep(STRATEGY_TEST_STEPS.get(7), taskResult); + assertNull(previousStep); + } + + // MARK: test next step rule + + @Test + public void testNext_fromStep2() { + TaskResult taskResult = mockTaskResult("task", STRATEGY_TEST_STEPS.subList(0, 2)); + // Step 2 returns null for it's next step identifier so the default behaviour should kick in and we should + // move on to step3. + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(STRATEGY_TEST_STEPS.get(2), taskResult); + assertNotNull(nextStep); + assertEquals("step3", nextStep.getIdentifier()); + } + + @Test + public void testNext_FromIntroduction() { + TaskResult taskResult = mockTaskResult("task", new ArrayList()); + // The introduction step has a next rule which returns "step2" so we should move on to step2. + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(STRATEGY_TEST_STEPS.get(0), taskResult); + assertNotNull(nextStep); + assertEquals("step2", nextStep.getIdentifier()); + } + + @Test + public void testNext_From3() { + TaskResult taskResult = mockTaskResult("task", STRATEGY_TEST_STEPS.subList(0, 3)); + // Step 3 has a next rule which returns "step1" so we should move on to step1. + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(STRATEGY_TEST_STEPS.get(3), taskResult); + assertNotNull(nextStep); + assertEquals("step1", nextStep.getIdentifier()); + } + + @Test + public void testNext_from5Y() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 4; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(4), 0, 3)); + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(5), 0, 1)); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // step5.Y has a next rule returns "step7" so we should move on to step7. + SectionStep step5 = (SectionStep) STRATEGY_TEST_STEPS.get(5); + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(step5.getSteps().get(1), taskResult); + assertNotNull(nextStep); + assertEquals("step7", nextStep.getIdentifier()); + } + + @Test + public void testNext_from5Z() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 4; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(4), 0, 3)); + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(5), 0, 2)); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // step5.Z has a next rule returns "step4.B" so we should move on to step4.B. + SectionStep step5 = (SectionStep) STRATEGY_TEST_STEPS.get(5); + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(step5.getSteps().get(2), taskResult); + assertNotNull(nextStep); + assertEquals("step4.B", nextStep.getIdentifier()); + } + + @Test + public void testNext_from7() { + List stepHistory = new ArrayList<>(); + for (int i = 0; i < 4; i++) { + stepHistory.add(mockResult(STRATEGY_TEST_STEPS.get(i))); + } + + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(4), 0, 3)); + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(5), 0, 3)); + stepHistory.add(mockSectionResult(STRATEGY_TEST_STEPS.get(6), 0, 3)); + TaskResult taskResult = mockTaskResultFromResults("task", stepHistory); + // step7 has a next rule returns "step6.A" so we should move on to step6.A. + Step nextStep = STRATEGY_TEST_NAVIGATOR.getNextStep(STRATEGY_TEST_STEPS.get(7), taskResult); + assertNotNull(nextStep); + assertEquals("step6.A", nextStep.getIdentifier()); + } +} \ No newline at end of file diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java new file mode 100644 index 000000000..3aec13b15 --- /dev/null +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java @@ -0,0 +1,8 @@ +package org.sagebionetworks.research.domain.navigation; + +import org.sagebionetworks.research.domain.step.Step; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.BackStepStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.SkipStepStrategy; + +public interface TestStep extends Step, NextStepStrategy, BackStepStrategy, SkipStepStrategy { } diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java new file mode 100644 index 000000000..e4394be19 --- /dev/null +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java @@ -0,0 +1,37 @@ +package org.sagebionetworks.research.domain.navigation; + +import org.junit.*; +import org.sagebionetworks.research.domain.navigation.IndividualNavigatorTests; +import org.sagebionetworks.research.domain.result.TaskResult; +import org.sagebionetworks.research.domain.step.Step; +import org.sagebionetworks.research.domain.task.Task; +import org.sagebionetworks.research.domain.task.navigation.TreeNavigator; + +import java.util.List; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertNotNull; +import static junit.framework.Assert.assertTrue; + +public class TreeNavigatorTests extends IndividualNavigatorTests { + + public TreeNavigatorTests() { + super(new TreeNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); + } + + // MARK: Test getProgress(). + + @Test + public void testProgess_NoMarkers_FlatHierarchy() { + List steps = createSteps(new String[]{"1", "2", "3", "4"}); + TreeNavigator navigator = new TreeNavigator(steps, null); + TaskResult taskResult = mockTaskResult("task", steps.subList(0, 1)); + + Task.Progress progress = navigator.getProgress(steps.get(1), taskResult); + assertNotNull(progress); + assertEquals(2, progress.getProgress()); + assertEquals(4, progress.getTotal()); + assertTrue(progress.isEstimated()); + } +} + From bd0c02cd56af42df262ca897e9c1c396624b4b24 Mon Sep 17 00:00:00 2001 From: Robert Kolmos Date: Thu, 26 Apr 2018 12:53:52 -0700 Subject: [PATCH 3/3] temp --- .idea/caches/build_file_checksums.ser | Bin 736 -> 736 bytes .../strategy/StrategyBasedNavigator.java | 11 +-- .../navigation/IndividualNavigatorTests.java | 24 ++--- .../StrategyBasedNavigatorTests.java | 87 +++++++++--------- .../research/domain/navigation/TestStep.java | 8 -- .../domain/navigation/TreeNavigatorTests.java | 4 +- 6 files changed, 62 insertions(+), 72 deletions(-) delete mode 100644 domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java diff --git a/.idea/caches/build_file_checksums.ser b/.idea/caches/build_file_checksums.ser index b902e1e49c2f0fb078e1ec11557992187745a10e..35297e4b416b3ca9c25baec940a5be3753d87c3e 100644 GIT binary patch delta 205 zcmV;;05bpJ1>gmcmjz=3fjzI0oOdA*;q(9FecVlaj*XJoQGk^C0HJ)ulWRqC|N*LFRTbU_<(6-H5@Ie`PIdF6U z6J&2~VQG^w0T>gW$bS&TyNiR0hVHy{Zq>GQ01T5}0v40}0TPo_0wFyOWpZ?BWpj0L0Db@l0000GFJ<=VsZ7UcUTE%*x{b-x Hld%FXZnsog delta 219 zcmV<103`q51>gmcmjzoOcjNM+X3hB*(sI6(5E0MG&QvXaOb=@pLQQurY?T zBILWPNR6Eklb!)e86Oa$bsUoOCz0T&bCo{4zH1t&SC z=Vr4LfT*u@01T5}0u~{|pA%#4aM*QYN&bYKR}*Y>04PH`Tvc;ra&ug2d2DHQlQaP~ zlT!jAJrJNQj;P@H!Z34^bH>K^@pJ$XY;R*>Y%XweZ*XODbZKRCb#VZG00sa601yPq V!t)R#9~ stepHistory = taskResult.getStepHistory(); - int idx = stepHistory.indexOf(step); + int idx = stepHistory.indexOf(taskResult.getResult(step.getIdentifier())); if (idx > 0) { String previousStepId = stepHistory.get(idx - 1).getIdentifier(); previousStep = this.getStep(previousStepId); @@ -126,7 +119,7 @@ public Step getPreviousStep(@NonNull final Step step, @NonNull TaskResult taskRe return previousStep; } - @NonNull + @Nullable @Override public Progress getProgress(@NonNull final Step step, @NonNull TaskResult taskResult) { return this.treeNavigator.getProgress(step, taskResult); diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java index 5c2035414..b4b24fe5b 100644 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java @@ -23,10 +23,9 @@ import static org.mockito.Mockito.*; public abstract class IndividualNavigatorTests { - private static final String STEP_TYPE = "step"; - public static final List TEST_STEPS; - public static final List TEST_PROGRESS_MARKERS; + protected static final List TEST_STEPS; + protected static final List TEST_PROGRESS_MARKERS; static { List steps = new ArrayList<>(createSteps(new String[]{"introduction", "step1", "step2", "step3"})); steps.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.C"}))); @@ -46,6 +45,7 @@ public IndividualNavigatorTests(StepNavigator navigator) { this.steps = TEST_STEPS; } + // region Mocking public static Step mockStep(String identifier) { Step step = mock(Step.class); when(step.getIdentifier()).thenReturn(identifier); @@ -104,13 +104,13 @@ public static TaskResult mockTaskResultFromResults(String identifier, List()); @@ -226,9 +226,9 @@ public void testProgress_MarkersAndSections_Completion() { Task.Progress progress = navigator.getProgress(steps.get(8), taskResult); assertNull(progress); } + // endregion - // MARK: test getPreviousStep() - + // region Test getPreviousStep() @Test public void testBack_From5X() { List stepHistory = new ArrayList<>(); @@ -270,9 +270,9 @@ public void testBack_From3() { assertNotNull(previousStep); assertEquals("step2", previousStep.getIdentifier()); } + // endregion - // MARK: test getNextStep() - + // region Test getNextStep() @Test public void testForward_From5X() { List stepHistory = new ArrayList<>(); @@ -314,10 +314,9 @@ public void testForward_From2() { assertNotNull(nextStep); assertEquals("step3", nextStep.getIdentifier()); } + // endregion - - // MARK: test getStep() - + // region Test getStep() @Test public void testGetStep() { assertEquals(steps.get(0), navigator.getStep("introduction")); @@ -325,4 +324,5 @@ public void testGetStep() { assertEquals(steps.get(7), navigator.getStep("step7")); assertEquals(steps.get(5), navigator.getStep("step5")); } + // endregion } diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java index f19ebaddf..0cba002a4 100644 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java @@ -7,6 +7,9 @@ import org.sagebionetworks.research.domain.step.SectionStep; import org.sagebionetworks.research.domain.step.Step; import org.sagebionetworks.research.domain.task.Task; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.BackStepStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.SkipStepStrategy; import org.sagebionetworks.research.domain.task.navigation.strategy.StrategyBasedNavigator; import java.util.ArrayList; @@ -23,15 +26,40 @@ public class StrategyBasedNavigatorTests extends IndividualNavigatorTests { private static final String SKIP_RESULT_IDENTIFIER = "skip"; private static final String TEST_STEP_TYPE = "test step"; + private static final List STRATEGY_TEST_STEPS; + private static final StrategyBasedNavigator STRATEGY_TEST_NAVIGATOR; + static { + STRATEGY_TEST_STEPS = new ArrayList<>(); + STRATEGY_TEST_STEPS.add(mockTestStep("introduction", true, "step2", false)); + STRATEGY_TEST_STEPS.add(mockStep("step1")); + STRATEGY_TEST_STEPS.add(mockTestStep("step2", true, null, true)); + STRATEGY_TEST_STEPS.add(mockTestStep("step3", true, "step1", false)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.c"}))); + List step5Substeps = new ArrayList<>(); + step5Substeps.add(mockTestStep("step5.X", true, null, true)); + step5Substeps.add(mockTestStep("step5.Y", true, "step7", false)); + step5Substeps.add(mockTestStep("step5.Z", true, "step4.B", false)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step5", step5Substeps)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.c"}))); + STRATEGY_TEST_STEPS.add(mockTestStep("step7", false, "step6.A", false)); + STRATEGY_TEST_STEPS.add(mockStep("conclusion")); + STRATEGY_TEST_NAVIGATOR = new StrategyBasedNavigator(STRATEGY_TEST_STEPS, TEST_PROGRESS_MARKERS); + } + + public StrategyBasedNavigatorTests() { + super(new StrategyBasedNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); + } + // region Mocking private static Step mockTestStep(String identifier, boolean isBackAllowed, String nextStepId, boolean shouldAddSkipRule) { - TestStep step = mock(TestStep.class); + Step step = mock(Step.class, withSettings().extraInterfaces(NextStepStrategy.class, + BackStepStrategy.class, SkipStepStrategy.class)); when(step.getIdentifier()).thenReturn(identifier); when(step.getType()).thenReturn(TEST_STEP_TYPE); when(step.toString()).thenReturn(identifier + ": " + TEST_STEP_TYPE); - when(step.isBackAllowed(any(TaskResult.class))).thenReturn(isBackAllowed); - when(step.getNextStepIdentifier(any(TaskResult.class))).thenReturn(nextStepId); + when(((BackStepStrategy)step).isBackAllowed(any(TaskResult.class))).thenReturn(isBackAllowed); + when(((NextStepStrategy)step).getNextStepIdentifier(any(TaskResult.class))).thenReturn(nextStepId); if (shouldAddSkipRule) { // If we are adding a skip rule, return true for any argument in which the given TaskResult contains a // result with the SKIP_RESULT_IDENTIFIER, false otherwise. @@ -41,28 +69,29 @@ public boolean matches(final TaskResult argument) { return argument != null && argument.getResult(SKIP_RESULT_IDENTIFIER) != null; } }); - when(step.shouldSkip(match)).thenReturn(true); + when(((SkipStepStrategy)step).shouldSkip(match)).thenReturn(true); TaskResult notMatch = argThat(new ArgumentMatcher() { @Override public boolean matches(final TaskResult argument) { return argument != null && argument.getResult(SKIP_RESULT_IDENTIFIER) == null; } }); - when(step.shouldSkip(notMatch)).thenReturn(false); + when(((SkipStepStrategy)step).shouldSkip(notMatch)).thenReturn(false); } else { // If we aren't adding a skip rule we always return false. - when(step.shouldSkip(any(TaskResult.class))).thenReturn(false); + when(((SkipStepStrategy)step).shouldSkip(any(TaskResult.class))).thenReturn(false); } return step; } - public StrategyBasedNavigatorTests() { - super(new StrategyBasedNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); + private TaskResult mockSectionResult(Step step, int from, int to) { + SectionStep sectionStep = (SectionStep) step; + return mockTaskResult(sectionStep.getIdentifier(), sectionStep.getSteps().subList(from, to)); } + // endregion - // MARK: Test getProgress(). - + // region Test getProgress() @Test public void testProgess_NoMarkers_FlatHierarchy() { List steps = createSteps(new String[]{"1", "2", "3", "4"}); @@ -75,34 +104,9 @@ public void testProgess_NoMarkers_FlatHierarchy() { assertEquals(4, progress.getTotal()); assertTrue(progress.isEstimated()); } + // endregion - private TaskResult mockSectionResult(Step step, int from, int to) { - SectionStep sectionStep = (SectionStep) step; - return mockTaskResult(sectionStep.getIdentifier(), sectionStep.getSteps().subList(from, to)); - } - - private static final List STRATEGY_TEST_STEPS; - private static final StrategyBasedNavigator STRATEGY_TEST_NAVIGATOR; - static { - STRATEGY_TEST_STEPS = new ArrayList<>(); - STRATEGY_TEST_STEPS.add(mockTestStep("introduction", true, "step2", false)); - STRATEGY_TEST_STEPS.add(mockStep("step1")); - STRATEGY_TEST_STEPS.add(mockTestStep("step2", true, null, true)); - STRATEGY_TEST_STEPS.add(mockTestStep("step3", true, "step1", false)); - STRATEGY_TEST_STEPS.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.c"}))); - List step5Substeps = new ArrayList<>(); - step5Substeps.add(mockTestStep("step5.X", true, null, true)); - step5Substeps.add(mockTestStep("step5.Y", true, "step7", false)); - step5Substeps.add(mockTestStep("step5.Z", true, "step4.B", false)); - STRATEGY_TEST_STEPS.add(mockSectionStep("step5", step5Substeps)); - STRATEGY_TEST_STEPS.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.c"}))); - STRATEGY_TEST_STEPS.add(mockTestStep("step7", false, "step6.A", false)); - STRATEGY_TEST_STEPS.add(mockStep("conclusion")); - STRATEGY_TEST_NAVIGATOR = new StrategyBasedNavigator(STRATEGY_TEST_STEPS, TEST_PROGRESS_MARKERS); - } - - // MARK: test skip rule - + // region Test With SkipStepStrategy @Test public void testSkip_resultPresent_From2() { List stepHistory = new ArrayList<>(); @@ -162,9 +166,9 @@ public void testSkip_resultNotPresent_From5X() { assertNotNull(nextStep); assertEquals("step5.X", nextStep.getIdentifier()); } + // endregion - // MARK: test back rule - + // region Test With BackStepStrategy @Test public void testBackAllowed_From2() { List stepHistory = new ArrayList<>(); @@ -207,9 +211,9 @@ public void testBackNotAllowed_From7() { Step previousStep = STRATEGY_TEST_NAVIGATOR.getPreviousStep(STRATEGY_TEST_STEPS.get(7), taskResult); assertNull(previousStep); } + // endregion - // MARK: test next step rule - + // region Test With NextStepStrategy @Test public void testNext_fromStep2() { TaskResult taskResult = mockTaskResult("task", STRATEGY_TEST_STEPS.subList(0, 2)); @@ -288,4 +292,5 @@ public void testNext_from7() { assertNotNull(nextStep); assertEquals("step6.A", nextStep.getIdentifier()); } + // endregion } \ No newline at end of file diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java deleted file mode 100644 index 3aec13b15..000000000 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java +++ /dev/null @@ -1,8 +0,0 @@ -package org.sagebionetworks.research.domain.navigation; - -import org.sagebionetworks.research.domain.step.Step; -import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.BackStepStrategy; -import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy; -import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.SkipStepStrategy; - -public interface TestStep extends Step, NextStepStrategy, BackStepStrategy, SkipStepStrategy { } diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java index e4394be19..4e2ca29ec 100644 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java @@ -19,8 +19,7 @@ public TreeNavigatorTests() { super(new TreeNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); } - // MARK: Test getProgress(). - + // region Test getProgress() @Test public void testProgess_NoMarkers_FlatHierarchy() { List steps = createSteps(new String[]{"1", "2", "3", "4"}); @@ -33,5 +32,6 @@ public void testProgess_NoMarkers_FlatHierarchy() { assertEquals(4, progress.getTotal()); assertTrue(progress.isEstimated()); } + // endregion }