diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/storage/BulkFlowNodeStorage.java b/src/main/java/org/jenkinsci/plugins/workflow/support/storage/BulkFlowNodeStorage.java index 40612ca2..348bba6f 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/storage/BulkFlowNodeStorage.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/storage/BulkFlowNodeStorage.java @@ -44,7 +44,10 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.TreeMap; /** * {@link FlowNodeStorage} implementation that stores all the {@link FlowNode}s together in one file for efficient bulk I/O @@ -64,7 +67,7 @@ public class BulkFlowNodeStorage extends FlowNodeStorage { private final FlowExecution exec; /** Lazy-loaded mapping. */ - private transient HashMap nodes = null; + private transient Map nodes = null; /** If true, we've been modified since last flush. */ private boolean isModified = false; @@ -81,20 +84,21 @@ public BulkFlowNodeStorage(FlowExecution exec, File dir) { } /** Loads the nodes listing, lazily - so loading the {@link FlowExecution} doesn't trigger a more complex load. */ - HashMap getOrLoadNodes() throws IOException { + @SuppressWarnings("unchecked") + private Map getOrLoadNodes() throws IOException { if (nodes == null) { if (dir.exists()) { File storeFile = getStoreFile(); if (storeFile.exists()) { - HashMap roughNodes = null; + Map roughNodes = null; try { - roughNodes = (HashMap) (XSTREAM.fromXML(getStoreFile())); + roughNodes = (Map) (XSTREAM.fromXML(getStoreFile())); } catch (Exception ex) { - nodes = new HashMap(); + nodes = new HashMap<>(); throw new IOException("Failed to read nodes", ex); } if (roughNodes == null) { - nodes = new HashMap(); + nodes = new HashMap<>(); throw new IOException("Unable to load nodes, invalid data"); } for (Tag t : roughNodes.values()) { @@ -111,11 +115,11 @@ HashMap getOrLoadNodes() throws IOException { } nodes = roughNodes; } else { - nodes = new HashMap(); + nodes = new HashMap<>(); } } else { IOUtils.mkdirs(dir); - nodes = new HashMap(); + nodes = new HashMap<>(); } } return nodes; @@ -128,6 +132,7 @@ public FlowNode getNode(@NonNull String id) throws IOException { return (t != null) ? t.node : null; } + @Override public void storeNode(@NonNull FlowNode n, boolean delayWritingActions) throws IOException { Tag t = getOrLoadNodes().get(n.getId()); if (t != null) { @@ -165,12 +170,24 @@ public void flush() throws IOException { if (!dir.exists()) { IOUtils.mkdirs(dir); } - PipelineIOUtils.writeByXStream(nodes, getStoreFile(), XSTREAM, !this.isAvoidAtomicWrite()); + Map sorted = new TreeMap<>(BulkFlowNodeStorage::sort); + sorted.putAll(nodes); + // Serialize as LinkedHashMap so that XStream does not try to save the comparator: + PipelineIOUtils.writeByXStream(new LinkedHashMap<>(sorted), getStoreFile(), XSTREAM, !this.isAvoidAtomicWrite()); isModified = false; } } + private static int sort(String k1, String k2) { + try { + return Integer.parseInt(k1) - Integer.parseInt(k2); + } catch (NumberFormatException x) { + return k1.compareTo(k2); + } + } + + @Override public List loadActions(@NonNull FlowNode node) throws IOException { Tag t = getOrLoadNodes().get(node.getId()); return (t != null) ? t.actions() : Collections.emptyList(); @@ -179,8 +196,9 @@ public List loadActions(@NonNull FlowNode node) throws IOException { /** * Just stores this one node */ + @Override public void saveActions(@NonNull FlowNode node, @NonNull List actions) throws IOException { - HashMap map = getOrLoadNodes(); + Map map = getOrLoadNodes(); Tag t = map.get(node.getId()); if (t != null) { t.node = node; @@ -193,6 +211,7 @@ public void saveActions(@NonNull FlowNode node, @NonNull List actions) t } /** Have we written everything to disk that we need to, or is there something waiting to be written */ + @Override public boolean isPersistedFully() { return !isModified; } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/storage/BulkFlowNodeStorageTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/storage/BulkFlowNodeStorageTest.java new file mode 100644 index 00000000..0db64f0a --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/storage/BulkFlowNodeStorageTest.java @@ -0,0 +1,61 @@ +/* + * The MIT License + * + * Copyright 2024 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.jenkinsci.plugins.workflow.support.storage; + +import java.io.File; +import java.util.ArrayList; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import javax.xml.parsers.DocumentBuilderFactory; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.w3c.dom.Element; +import org.w3c.dom.Node; + +public final class BulkFlowNodeStorageTest { + + @Rule public JenkinsRule r = new JenkinsRule(); + + @Test public void orderOfEntries() throws Exception { + var p = r.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition("for (int i = 1; i <= 40; i++) {echo(/step #$i/)}", true)); + var b = r.buildAndAssertSuccess(p); + var entryList = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new File(b.getRootDir(), "workflow-completed/flowNodeStore.xml")).getDocumentElement().getChildNodes(); + var ids = new ArrayList(); + for (int i = 0; i < entryList.getLength(); i++) { + var entry = entryList.item(i); + if (entry.getNodeType() == Node.ELEMENT_NODE) { + ids.add(((Element) entry).getElementsByTagName("*").item(0).getTextContent()); + } + } + assertThat(ids, is(IntStream.rangeClosed(2, 43).mapToObj(Integer::toString).collect(Collectors.toList()))); + } + +}