Take the session id and the sequence from the Sequence Element's data

pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/SequenceElement.java

@@ -146,9 +146,16 @@ private static Integer toSequence(Object sequence) {
             Integer.valueOf((int) value) : null;
     }
 
+    /**
+     * The key the Element Data is stored under in the Flow Data, unless a
+     * subclass gives another from {@link #getElementDataKey()}.
+     * @see <a href="https://github.com/51Degrees/specifications/blob/67ead6986f3118c68f0714325800b92d48ddc71f/pipeline-specification/pipeline-elements/sequence-element.md#element-data">Specification</a>
+     */
+    public static final String DEFAULT_ELEMENT_DATA_KEY = "sequence";
+
     @Override
     public String getElementDataKey() {
-        return "sequence";
+        return DEFAULT_ELEMENT_DATA_KEY;
     }
 
     @Override

pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/JavaScriptBuilderElement.java

@@ -32,6 +32,8 @@
 import fiftyone.pipeline.core.flowelements.FlowElementBase;
 import fiftyone.pipeline.engines.data.AspectPropertyValue;
 import fiftyone.pipeline.engines.exceptions.PropertyMissingException;
+import fiftyone.pipeline.engines.fiftyone.data.SequenceData;
+import fiftyone.pipeline.engines.fiftyone.flowelements.SequenceElement;
 import fiftyone.pipeline.javascriptbuilder.Constants;
 import fiftyone.pipeline.javascriptbuilder.data.JavaScriptBuilderData;
 import fiftyone.pipeline.javascriptbuilder.templates.JavaScriptResource;
@@ -243,8 +245,9 @@ protected void processInternal(FlowData data) throws Exception {
 
         String url = getUrl(reqProtocol, reqHost);
 
-        String sessionId = getSessionId(data);
-        int sequence = getSequence(data);
+        ElementData sequenceData = getSequenceData(data);
+        String sessionId = getSessionId(sequenceData);
+        int sequence = getSequence(sequenceData);
         String serializedParameters = serializeParameters(parameters);
 
         // With the gathered resources, build a new JavaScriptResource.
@@ -473,52 +476,77 @@ private String getPath() {
 
     /**
      * Gets the session id to write into the script. This is the session id
-     * evidence, set by the Sequence Element or by the request, when it
-     * matches SESSION_ID_PATTERN, and an empty string otherwise.
-     * @param data the flow data
+     * the Sequence Element wrote, when it matches SESSION_ID_PATTERN, and an
+     * empty string otherwise, including when the pipeline has no Sequence
+     * Element.
+     * @param sequenceData the Sequence Element's data, or null
      * @return the session id, or an empty string
      */
-    private String getSessionId(FlowData data) {
-        TryGetResult<Object> trySessionId =
-            data.tryGetEvidence(EVIDENCE_SESSIONID, Object.class);
-        if (trySessionId.hasValue() &&
-            trySessionId.getValue() instanceof String) {
-            String sessionId = (String) trySessionId.getValue();
-            if (SESSION_ID_PATTERN.matcher(sessionId).matches()) {
-                return sessionId;
-            }
+    private String getSessionId(ElementData sequenceData) {
+        Object value = getSequenceValue(
+            sequenceData, SequenceData.SESSION_ID_PROPERTY_NAME);
+        if (value instanceof String &&
+            SESSION_ID_PATTERN.matcher((String) value).matches()) {
+            return (String) value;
         }
         return "";
     }
 
     /**
-     * Gets the sequence to write into the script. This is the sequence
-     * evidence, as a number or as text, when it is a positive 32 bit
-     * integer, and 1 otherwise, including when there is no Sequence Element.
-     * The value is written into the script as code, so text that is not a
-     * number could break the script, and a number below 1 is not a sequence
-     * the Sequence Element would ever give.
-     * @param data the flow data
+     * Gets the sequence to write into the script. This is the sequence the
+     * Sequence Element wrote, as a number or as text, when it is a positive
+     * 32 bit integer, and 1 otherwise, including when the pipeline has no
+     * Sequence Element. The value is written into the script as code, so
+     * text that is not a number could break the script, and a number below 1
+     * is not a sequence the Sequence Element would ever give.
+     * @param sequenceData the Sequence Element's data, or null
      * @return the sequence, which is always at least 1
      */
-    private int getSequence(FlowData data) {
-        TryGetResult<Object> trySequence =
-            data.tryGetEvidence(EVIDENCE_SEQUENCE, Object.class);
+    private int getSequence(ElementData sequenceData) {
+        Object value = getSequenceValue(
+            sequenceData, SequenceData.SEQUENCE_PROPERTY_NAME);
         long sequence = 0;
-        if (trySequence.hasValue()) {
-            Object value = trySequence.getValue();
-            if (value instanceof Integer) {
-                sequence = (Integer) value;
-            } else if (value instanceof String &&
-                SEQUENCE_PATTERN.matcher((String) value).matches()) {
-                sequence = Long.parseLong((String) value);
-            }
+        if (value instanceof Integer) {
+            sequence = (Integer) value;
+        } else if (value instanceof String &&
+            SEQUENCE_PATTERN.matcher((String) value).matches()) {
+            sequence = Long.parseLong((String) value);
         }
         return sequence >= 1 && sequence <= Integer.MAX_VALUE
             ? (int) sequence
             : 1;
     }
 
+    /**
+     * The Sequence Element's data, or null when the pipeline has no Sequence
+     * Element. Step 5 of the specification takes the session id and the
+     * sequence from that data and not from Evidence, so a pipeline with no
+     * Sequence Element renders the defaults even when the request carried a
+     * session id or a sequence of its own.
+     * @param data the flow data
+     * @return the Sequence Element's data, or null
+     * @see <a href="https://github.com/51Degrees/specifications/blob/67ead6986f3118c68f0714325800b92d48ddc71f/pipeline-specification/pipeline-elements/javascript-builder.md#process">Specification</a>
+     */
+    private static ElementData getSequenceData(FlowData data) {
+        // get(String) throws when the Flow Data holds nothing under the key,
+        // so the keys it does hold are checked first.
+        return data.getDataKeys().contains(SequenceElement.DEFAULT_ELEMENT_DATA_KEY)
+            ? data.get(SequenceElement.DEFAULT_ELEMENT_DATA_KEY)
+            : null;
+    }
+
+    /**
+     * One value from the Sequence Element's data.
+     * @param sequenceData the Sequence Element's data, or null
+     * @param name the name of the value within that data
+     * @return the value, or null
+     */
+    private static Object getSequenceValue(
+        ElementData sequenceData,
+        String name) {
+        return sequenceData == null ? null : sequenceData.get(name);
+    }
+
     private void buildJavaScript(
         FlowData data,
         String jsonObject,

pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/JavaScriptBuilderTests.java

@@ -30,6 +30,8 @@
 import fiftyone.pipeline.engines.data.AspectPropertyValue;
 import fiftyone.pipeline.engines.data.AspectPropertyValueDefault;
 import fiftyone.pipeline.engines.exceptions.PropertyMissingException;
+import fiftyone.pipeline.engines.fiftyone.data.SequenceData;
+import fiftyone.pipeline.engines.fiftyone.flowelements.SequenceElement;
 import fiftyone.pipeline.javascriptbuilder.data.JavaScriptBuilderData;
 import fiftyone.pipeline.javascriptbuilder.flowelements.JavaScriptBuilderElement;
 import fiftyone.pipeline.javascriptbuilder.flowelements.JavaScriptBuilderElementBuilder;
@@ -57,6 +59,7 @@
 import java.io.IOException;
 import java.time.Duration;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -527,6 +530,19 @@ private void configureMocks() {
 
         when(flowData.get(anyString())).thenReturn(elementDataMock);
 
+        // The Sequence Element's data, which is where the builder reads the
+        // session id and the sequence from, as a web integration's
+        // pipeline holds it.
+        ElementData sequenceData = mock(ElementData.class);
+        when(sequenceData.get(SequenceData.SESSION_ID_PROPERTY_NAME))
+            .thenReturn(sessionId);
+        when(sequenceData.get(SequenceData.SEQUENCE_PROPERTY_NAME))
+            .thenReturn(sequence);
+        when(flowData.getDataKeys()).thenReturn(Collections.singletonList(
+            SequenceElement.DEFAULT_ELEMENT_DATA_KEY));
+        when(flowData.get(SequenceElement.DEFAULT_ELEMENT_DATA_KEY))
+            .thenReturn(sequenceData);
+
         when(flowData.getAs(anyString(), any(Class.class))).thenReturn(new AspectPropertyValueDefault<>("None"));
 
         doAnswer((Answer<Object>) invocation -> {

pipeline/pipeline.javascriptbuilder/src/test/java/fiftyone/pipeline/javascriptbuilder/SessionIdAndSequenceTests.java

@@ -30,6 +30,8 @@
 import fiftyone.pipeline.core.flowelements.FlowElementBase;
 import fiftyone.pipeline.core.flowelements.PipelineBuilder;
 import fiftyone.pipeline.core.flowelements.Pipeline;
+import fiftyone.pipeline.engines.fiftyone.data.SequenceData;
+import fiftyone.pipeline.engines.fiftyone.flowelements.SequenceElement;
 import fiftyone.pipeline.engines.fiftyone.flowelements.SequenceElementBuilder;
 import fiftyone.pipeline.javascriptbuilder.flowelements.JavaScriptBuilderElement;
 import fiftyone.pipeline.javascriptbuilder.flowelements.JavaScriptBuilderElementBuilder;
@@ -66,6 +68,11 @@
  * the builder's own handling can be tested, the tests "without the Sequence
  * Element" use a stand in for the JSON builder that supplies the payload and
  * leaves the evidence as it was given.
+ * <p>
+ * The builder takes both values from the Sequence Element's data, so the
+ * tests of the rule above put the value straight into that data with a
+ * stand in for the Sequence Element, which lets a value the real element
+ * would never write be tried as well.
  */
 public class SessionIdAndSequenceTests {
 
@@ -122,17 +129,73 @@ protected void unmanagedResourcesCleanup() {
         }
     }
 
+    /**
+     * Writes the two values given straight into the Sequence Element's data,
+     * in place of that element, so that a value the real element would never
+     * write can be tried as well. A null value is not written.
+     */
+    private static class SequenceStandIn extends
+        FlowElementBase<SequenceData, ElementPropertyMetaData> {
+
+        private final Object sessionId;
+        private final Object sequence;
+
+        SequenceStandIn(Logger logger, Object sessionId, Object sequence) {
+            super(logger, null);
+            this.sessionId = sessionId;
+            this.sequence = sequence;
+        }
+
+        @Override
+        protected void processInternal(FlowData data) {
+            SequenceData sequenceData = new SequenceData(logger, data);
+            if (sessionId != null) {
+                sequenceData.put(
+                    SequenceData.SESSION_ID_PROPERTY_NAME, sessionId);
+            }
+            if (sequence != null) {
+                sequenceData.put(
+                    SequenceData.SEQUENCE_PROPERTY_NAME, sequence);
+            }
+            data.getOrAdd(getElementDataKey(), f -> sequenceData);
+        }
+
+        @Override
+        public String getElementDataKey() {
+            return SequenceElement.DEFAULT_ELEMENT_DATA_KEY;
+        }
+
+        @Override
+        public EvidenceKeyFilter getEvidenceKeyFilter() {
+            return new EvidenceKeyFilterWhitelist(
+                Collections.<String>emptyList());
+        }
+
+        @Override
+        public List<ElementPropertyMetaData> getProperties() {
+            return Collections.emptyList();
+        }
+
+        @Override
+        protected void managedResourcesCleanup() {
+        }
+
+        @Override
+        protected void unmanagedResourcesCleanup() {
+        }
+    }
+
     @Test
     public void SessionId_Valid_Rendered() throws Exception {
-        String script = render(false, "abc-123", null);
+        String script = renderWithSequenceData("abc-123", null);
         assertEquals("abc-123", sessionId(script));
         assertEquals("1", sequence(script));
     }
 
     @Test
     public void SessionId_LongestValid_Rendered() throws Exception {
         String id = repeat('a', 64);
-        String script = render(false, id, null);
+        String script = renderWithSequenceData(id, null);
         assertEquals(id, sessionId(script));
     }
 
@@ -149,7 +212,7 @@ static Stream<String> invalidSessionIds() {
     @ParameterizedTest
     @MethodSource("invalidSessionIds")
     public void SessionId_Invalid_RenderedEmpty(String id) throws Exception {
-        String script = render(false, id, null);
+        String script = renderWithSequenceData(id, null);
         assertEquals("", sessionId(script));
     }
 
@@ -190,14 +253,31 @@ public void NoSequenceElement_NoEvidence_DefaultsRendered()
     }
 
     /**
-     * A sequence from a web request is text. With no Sequence Element it
-     * reaches the builder as it is.
+     * With no Sequence Element the defaults are rendered even when the
+     * request carried a session id and a sequence of its own, because step 5
+     * of the
+     * <a href="https://github.com/51Degrees/specifications/blob/67ead6986f3118c68f0714325800b92d48ddc71f/pipeline-specification/pipeline-elements/javascript-builder.md#process">specification</a>
+     * takes both from the Sequence Element's data and says "Where there is
+     * no Sequence Element, the sequence MUST be rendered as 1 and the
+     * session id as an empty string, so the script always parses."
+     */
+    @Test
+    public void NoSequenceElement_WithEvidence_DefaultsRendered()
+        throws Exception {
+        String script = render(false, "abc-123", 7);
+        assertEquals("", sessionId(script));
+        assertEquals("1", sequence(script));
+    }
+
+    /**
+     * A sequence another Sequence Element wrote as text is read as a number
+     * when it is one, because the Element Data holds values of any type.
      */
     @ParameterizedTest
     @ValueSource(strings = {"abc", "-1", "0", "99999999999", "", "2147483648"})
     public void Sequence_InvalidText_RenderedOne(String value)
         throws Exception {
-        String script = render(false, "abc-123", value);
+        String script = renderWithSequenceData("abc-123", value);
         assertEquals("1", sequence(script));
     }
 
@@ -209,7 +289,7 @@ static Stream<Object> invalidSequenceNumbers() {
     @MethodSource("invalidSequenceNumbers")
     public void Sequence_InvalidNumber_RenderedOne(Object value)
         throws Exception {
-        String script = render(false, "abc-123", value);
+        String script = renderWithSequenceData("abc-123", value);
         assertEquals("1", sequence(script));
     }
 
@@ -225,7 +305,7 @@ static Stream<Object[]> validSequences() {
     @MethodSource("validSequences")
     public void Sequence_Valid_Rendered(Object value, String expected)
         throws Exception {
-        String script = render(false, "abc-123", value);
+        String script = renderWithSequenceData("abc-123", value);
         assertEquals(expected, sequence(script));
     }
 
@@ -245,6 +325,31 @@ public void Sequence_InvalidWithSequenceElement_RenderedOne(int value)
         assertEquals("1", sequence(script));
     }
 
+    /**
+     * Builds a pipeline whose Sequence Element data holds the two values
+     * given, processes it, checks the script parses and returns it. A null
+     * value is not written into that data.
+     */
+    private String renderWithSequenceData(Object sessionId, Object sequence)
+        throws Exception {
+        JavaScriptBuilderElement jsElement =
+            new JavaScriptBuilderElementBuilder(loggerFactory).build();
+        PipelineBuilder builder = new PipelineBuilder(loggerFactory);
+        builder.addFlowElement(new SequenceStandIn(
+            loggerFactory.getLogger("sequence"), sessionId, sequence));
+        builder.addFlowElement(
+            new JsonStandIn(loggerFactory.getLogger("json")));
+        builder.addFlowElement(jsElement);
+        String script;
+        try (Pipeline pipeline = builder.build();
+            FlowData flowData = pipeline.createFlowData()) {
+            flowData.process();
+            script = flowData.getFromElement(jsElement).getJavaScript();
+        }
+        assertParses(script);
+        return script;
+    }
+
     /**
      * Builds a pipeline with the Sequence Element and the JSON builder, or
      * with the stand in for them, processes it