From 1bf8b797db7ca8c27057ac20d84ff63fdc748efc Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Sun, 20 Sep 2026 21:24:45 -0500 Subject: [PATCH 1/5] Add atomic set configuration map entry endpoint Signed-off-by: Mitch Gaffigan --- .../java/com/mirth/connect/client/core/Client.java | 10 ++++++++++ .../api/servlets/ConfigurationServletInterface.java | 8 ++++++++ .../server/api/servlets/ConfigurationServlet.java | 9 +++++++++ .../server/controllers/ConfigurationController.java | 13 +++++++++++++ .../controllers/DefaultConfigurationController.java | 7 +++++++ 5 files changed, 47 insertions(+) diff --git a/server/src/main/java/com/mirth/connect/client/core/Client.java b/server/src/main/java/com/mirth/connect/client/core/Client.java index eaa261fc7e..f3ebd6b301 100644 --- a/server/src/main/java/com/mirth/connect/client/core/Client.java +++ b/server/src/main/java/com/mirth/connect/client/core/Client.java @@ -809,6 +809,16 @@ public void setConfigurationMap(Map map) throws C getServlet(ConfigurationServletInterface.class).setConfigurationMap(map); } + /** + * Updates a single entry in the configuration map, leaving the rest alone. + * + * @see ConfigurationServletInterface#setConfigurationProperty + */ + @Override + public void setConfigurationProperty(String key, ConfigurationProperty property) throws ClientException { + getServlet(ConfigurationServletInterface.class).setConfigurationProperty(key, property); + } + /** * Returns the database driver list. * diff --git a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java index 9f8d391bfe..4c9ca4907e 100644 --- a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java +++ b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java @@ -306,6 +306,14 @@ public void setConfigurationMap(@Param("map") @RequestBody(description = "The ne @Content(mediaType = MediaType.APPLICATION_JSON, examples = { @ExampleObject(name = "configurationMap", ref = "../apiexamples/configuration_map_json") }) }) Map map) throws ClientException; + @PUT + @Path("/configurationMap/{key}") + @Operation(summary = "Updates a single entry in the configuration map, leaving the rest alone.") + @MirthOperation(name = "setConfigurationProperty", display = "Set configuration map entry", permission = Permissions.CONFIGURATION_MAP_EDIT) + public void setConfigurationProperty( + @Param("key") @Parameter(description = "The key of the entry to set.", required = true) @PathParam("key") String key, + @Param("property") @RequestBody(description = "The value and comment to store under the key.", required = true) ConfigurationProperty property) throws ClientException; + @GET @Path("/databaseDrivers") @Operation(summary = "Returns the database driver list.") diff --git a/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java b/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java index 67dd3419fc..e58740be3a 100644 --- a/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java +++ b/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java @@ -306,6 +306,15 @@ public void setConfigurationMap(Map map) { } } + @Override + public void setConfigurationProperty(String key, ConfigurationProperty property) { + try { + configurationController.setConfigurationProperty(key, property); + } catch (ControllerException e) { + throw new MirthApiException(e); + } + } + @Override public List getDatabaseDrivers() { try { diff --git a/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java b/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java index e4a9ce88c3..7aa1d49dd1 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java @@ -328,6 +328,19 @@ public static ConfigurationController getInstance() { */ public abstract void setConfigurationProperties(Map map, boolean persist) throws ControllerException; + /** + * Sets a single configuration map entry, leaving every other entry alone. Doing the + * read-modify-write here rather than in the caller is what makes it atomic: a client can only + * replace the whole map, so two clients updating different keys at once would otherwise drop + * each other's writes. + * + * @param key + * The key of the entry to set. + * @param property + * The value and comment to store under the key. + */ + public abstract void setConfigurationProperty(String key, ConfigurationProperty property) throws ControllerException; + // properties public Properties getPropertiesForGroup(String group) { return getPropertiesForGroup(group, null); diff --git a/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java b/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java index bfee2eddbf..bc01bd9e94 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java @@ -938,6 +938,13 @@ public synchronized void setConfigurationProperties(Map properties = getConfigurationProperties(); + properties.put(key, property); + setConfigurationProperties(properties, true); + } + @Override public void setStatus(int status) { this.status = status; From f445931eed83037988a8773a932add6a92fc3fc5 Mon Sep 17 00:00:00 2001 From: Tony Germano Date: Tue, 27 Oct 2026 02:53:07 -0400 Subject: [PATCH 2/5] Reject blank keys in the configuration map entry update PUT /configurationMap/{key} now responds with 400 Bad Request for a blank key, as the user servlet does for a blank username. In file mode, saving the map silently drops blank keys while they stay in memory, so a blank key would work until the next restart and then disappear. The key parameter description also says that a / in a key has to be encoded as %2F. Add a test that setting one key keeps the other entries, both in memory and in what is saved, and a smoke test for the blank key, since the check is in the servlet. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Tony Germano --- .../ConfigurationServletInterface.java | 2 +- .../api/servlets/ConfigurationServlet.java | 6 ++ .../DefaultConfigurationControllerTest.java | 57 +++++++++++++++++++ .../smoketest/ConfigurationMapEntryTest.java | 32 +++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java diff --git a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java index 4c9ca4907e..2d2cf399c7 100644 --- a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java +++ b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java @@ -311,7 +311,7 @@ public void setConfigurationMap(@Param("map") @RequestBody(description = "The ne @Operation(summary = "Updates a single entry in the configuration map, leaving the rest alone.") @MirthOperation(name = "setConfigurationProperty", display = "Set configuration map entry", permission = Permissions.CONFIGURATION_MAP_EDIT) public void setConfigurationProperty( - @Param("key") @Parameter(description = "The key of the entry to set.", required = true) @PathParam("key") String key, + @Param("key") @Parameter(description = "The key of the entry to set. It can't be blank, and a / in the key has to be encoded as %2F.", required = true) @PathParam("key") String key, @Param("property") @RequestBody(description = "The value and comment to store under the key.", required = true) ConfigurationProperty property) throws ClientException; @GET diff --git a/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java b/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java index e58740be3a..6afd337564 100644 --- a/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java +++ b/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java @@ -21,9 +21,11 @@ import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Context; +import javax.ws.rs.core.Response; import javax.ws.rs.core.SecurityContext; import org.apache.commons.lang3.ArrayUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -308,6 +310,10 @@ public void setConfigurationMap(Map map) { @Override public void setConfigurationProperty(String key, ConfigurationProperty property) { + if (StringUtils.isBlank(key)) { + throw new MirthApiException(Response.status(Response.Status.BAD_REQUEST).entity("The configuration map key cannot be blank.").build()); + } + try { configurationController.setConfigurationProperty(key, property); } catch (ControllerException e) { diff --git a/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java b/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java index 93072d30b0..571c4f3dad 100644 --- a/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java +++ b/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java @@ -13,6 +13,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -26,8 +28,11 @@ import java.io.Writer; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Properties; +import java.util.concurrent.atomic.AtomicReference; import javax.xml.XMLConstants; import javax.xml.transform.Source; @@ -36,6 +41,7 @@ import javax.xml.transform.stream.StreamSource; import org.apache.commons.io.FileUtils; +import org.junit.After; import org.junit.BeforeClass; import org.junit.Test; import org.mockito.invocation.Invocation; @@ -46,6 +52,8 @@ import com.google.inject.Injector; import com.mirth.connect.client.core.ControllerException; import com.mirth.connect.model.DriverInfo; +import com.mirth.connect.model.converters.ObjectXMLSerializer; +import com.mirth.connect.util.ConfigurationProperty; public class DefaultConfigurationControllerTest { @@ -66,6 +74,11 @@ protected void configure() { injector.getInstance(ControllerFactory.class); } + @After + public void resetConfigurationMapLocation() { + DefaultConfigurationController.mirthConfig.clearProperty(DefaultConfigurationController.CONFIGURATION_MAP_LOCATION); + } + @Test public void getDatabaseDrivers_NotBlank() throws Exception { DefaultConfigurationController configurationController = spy(new DefaultConfigurationController()); @@ -275,6 +288,38 @@ public void testParseDbdriversXmlWithExternalDtd() { assertTrue(exceptionCaught); } + @Test + public void setConfigurationProperty_KeepsOtherEntries() throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + + configurationController.setConfigurationProperty("B", new ConfigurationProperty("2", "second")); + + Map expected = new HashMap(); + expected.put("A", "1"); + expected.put("B", "2"); + assertEquals(expected, configurationController.getConfigurationMap()); + + @SuppressWarnings("unchecked") + Map saved = ObjectXMLSerializer.getInstance().deserialize(database.get(), HashMap.class); + assertEquals("1", saved.get("A").getValue()); + assertEquals("comment", saved.get("A").getComment()); + assertEquals("2", saved.get("B").getValue()); + assertEquals("second", saved.get("B").getComment()); + } + + private DefaultConfigurationController databaseBackedConfigurationController(AtomicReference database) { + DefaultConfigurationController.mirthConfig.setProperty(DefaultConfigurationController.CONFIGURATION_MAP_LOCATION, "database"); + + DefaultConfigurationController configurationController = spy(new DefaultConfigurationController()); + doAnswer(invocation -> database.get()).when(configurationController).getProperty(anyString(), anyString()); + doAnswer(invocation -> { + database.set(invocation.getArgument(2)); + return null; + }).when(configurationController).saveProperty(anyString(), anyString(), any()); + return configurationController; + } + private void assertDefaultDrivers(List drivers, boolean includeODBC) { assertEquals(includeODBC ? 7 : 6, drivers.size()); int i = 0; @@ -341,6 +386,18 @@ private String normalizeXml(String xml) throws Exception { return writer.toString(); } + private static final String INITIAL_CONFIGURATION_MAP_XML = """ + + + A + + 1 + comment + + + + """; + // @formatter:off private String DEFAULT_DRIVERS_XML = "\n" + " \n" + diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java new file mode 100644 index 0000000000..7ce65ac79b --- /dev/null +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2026 Tony Germano + +package org.openintegrationengine.smoketest; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import com.mirth.connect.client.core.ClientException; +import com.mirth.connect.client.core.api.servlets.ConfigurationServletInterface; +import com.mirth.connect.util.ConfigurationProperty; + +/** Exercises the single-entry configuration map update through the Java client and a live server. */ +@DisplayName("Configuration map entry update") +class ConfigurationMapEntryTest { + + /** The key check runs in the servlet, so only a request to a live server reaches it. */ + @Test + @DisplayName("rejects a blank key") + void rejectsBlankKey() throws Exception { + assertThrows(ClientException.class, () -> configuration().setConfigurationProperty(" ", new ConfigurationProperty("1", null))); + + assertFalse(configuration().getConfigurationMap().containsKey(" ")); + } + + private static ConfigurationServletInterface configuration() { + return SharedServer.get().servlet(ConfigurationServletInterface.class); + } +} From d966f8cca234b7fb90acff4e1db4ce19fa692b12 Mon Sep 17 00:00:00 2001 From: Tony Germano Date: Wed, 28 Oct 2026 04:17:24 -0400 Subject: [PATCH 3/5] Add request body examples for the configuration map entry update PUT /configurationMap/{key} takes a single ConfigurationProperty as the request body, so give it XML and JSON examples like the neighbouring configuration map endpoints. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Tony Germano --- .../core/api/servlets/ConfigurationServletInterface.java | 6 +++++- .../connect/server/servlets/SwaggerExamplesServlet.java | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java index 2d2cf399c7..db57984e0b 100644 --- a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java +++ b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java @@ -312,7 +312,11 @@ public void setConfigurationMap(@Param("map") @RequestBody(description = "The ne @MirthOperation(name = "setConfigurationProperty", display = "Set configuration map entry", permission = Permissions.CONFIGURATION_MAP_EDIT) public void setConfigurationProperty( @Param("key") @Parameter(description = "The key of the entry to set. It can't be blank, and a / in the key has to be encoded as %2F.", required = true) @PathParam("key") String key, - @Param("property") @RequestBody(description = "The value and comment to store under the key.", required = true) ConfigurationProperty property) throws ClientException; + @Param("property") @RequestBody(description = "The value and comment to store under the key.", required = true, content = { + @Content(mediaType = MediaType.APPLICATION_XML, examples = { + @ExampleObject(name = "configurationProperty", ref = "../apiexamples/configuration_property_xml") }), + @Content(mediaType = MediaType.APPLICATION_JSON, examples = { + @ExampleObject(name = "configurationProperty", ref = "../apiexamples/configuration_property_json") }) }) ConfigurationProperty property) throws ClientException; @GET @Path("/databaseDrivers") diff --git a/server/src/main/java/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java b/server/src/main/java/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java index 476775c82b..566f65c566 100644 --- a/server/src/main/java/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java +++ b/server/src/main/java/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java @@ -241,6 +241,8 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se requestedObject = getChannelSummaryListExample(); } else if (exampleRequested.equals("configuration_map")) { requestedObject = getConfigurationMapExample(); + } else if (exampleRequested.equals("configuration_property")) { + requestedObject = getConfigurationPropertyExample(); } else if (exampleRequested.equals("connection_test_response")) { requestedObject = getConnectionTestResponseExample(); } else if (exampleRequested.equals("dashboard_channel_info")) { @@ -728,6 +730,10 @@ private Map getConfigurationMapExample() { return configurationMap; } + private ConfigurationProperty getConfigurationPropertyExample() { + return new ConfigurationProperty("exampleValue", "Example comment"); + } + private Map> getConnectorMapExample(boolean includeNull) { Map> connectorMap = new HashMap<>(); List connectorList = new ArrayList<>(); From a3f09e451bf23c56ed98da83cdae26a1fa4d66ee Mon Sep 17 00:00:00 2001 From: Tony Germano Date: Tue, 27 Oct 2026 05:26:12 -0400 Subject: [PATCH 4/5] Add a content version for configuration map entries ConfigurationProperty.versionOf returns the lowercase hex SHA-256 of an entry's value, a unit separator (U+001F), and its comment, with a null value and a null or blank comment treated as empty, and the empty string for a missing entry. It depends only on the entry's contents, so a client can calculate it from what the server returned and send it back as a precondition without the API storing or returning a revision. The unit separator is used instead of NUL because some clients can't easily hold a NUL in a string, such as C strings and shell variables. It is a static method rather than a getter so that it doesn't show up as a property of ConfigurationProperty in the OpenAPI schema. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Tony Germano --- .../connect/util/ConfigurationProperty.java | 37 ++++++++++++++ .../util/ConfigurationPropertyTest.java | 49 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 server/src/test/java/com/mirth/connect/util/ConfigurationPropertyTest.java diff --git a/server/src/main/java/com/mirth/connect/util/ConfigurationProperty.java b/server/src/main/java/com/mirth/connect/util/ConfigurationProperty.java index 83223a6b62..2d2e81147a 100644 --- a/server/src/main/java/com/mirth/connect/util/ConfigurationProperty.java +++ b/server/src/main/java/com/mirth/connect/util/ConfigurationProperty.java @@ -10,6 +10,10 @@ package com.mirth.connect.util; import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.HexFormat; public class ConfigurationProperty implements Serializable { @@ -37,6 +41,39 @@ public void setComment(String comment) { this.comment = comment; } + /** + * Returns the version of a configuration map entry, for use as the expected version when + * updating that entry. The version depends only on the entry's contents: it is the lowercase + * hex SHA-256 of the UTF-8 bytes of the value, a unit separator (U+001F), and the comment. A null value + * and a null or blank comment are treated as empty strings. A missing entry has the empty + * string as its version. + * + * The version is calculated from what the server currently holds, so a client can calculate it + * from the entries returned by the server. Saving the map to a file can change some values and + * comments when the server next starts (for example by trimming whitespace), so a version + * calculated before a restart may no longer match. + * + * @param property + * The entry, or null if there is no entry for the key. + * @return The entry's version. + */ + public static String versionOf(ConfigurationProperty property) { + if (property == null) { + return ""; + } + + String value = property.getValue() == null ? "" : property.getValue(); + String comment = property.getComment() == null || property.getComment().isBlank() ? "" : property.getComment(); + + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + return HexFormat.of().formatHex(digest.digest((value + '\u001F' + comment).getBytes(StandardCharsets.UTF_8))); + } catch (NoSuchAlgorithmException e) { + // Every Java platform is required to support SHA-256 + throw new IllegalStateException(e); + } + } + @Override public String toString() { StringBuilder builder = new StringBuilder(); diff --git a/server/src/test/java/com/mirth/connect/util/ConfigurationPropertyTest.java b/server/src/test/java/com/mirth/connect/util/ConfigurationPropertyTest.java new file mode 100644 index 0000000000..de69e268ab --- /dev/null +++ b/server/src/test/java/com/mirth/connect/util/ConfigurationPropertyTest.java @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2026 Tony Germano + +package com.mirth.connect.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +import org.junit.Test; + +public class ConfigurationPropertyTest { + + // Expected values calculated with: printf '%s\037%s' '' '' | sha256sum + + @Test + public void versionOf_ValueAndComment() { + assertEquals("c1dcc868b7a3de969e0d671b3fce131d00d46d3da2a79805d91774ccfabaa8de", ConfigurationProperty.versionOf(new ConfigurationProperty("1", "comment"))); + } + + @Test + public void versionOf_NonAscii() { + assertEquals("b736f6189175d479edd12794d68abfdc2832ca4c2e3d5c87cc9e9ebe5c08a131", ConfigurationProperty.versionOf(new ConfigurationProperty("é", "✓"))); + } + + @Test + public void versionOf_NullAndBlankCommentsAreEmpty() { + String expected = "2b8d9cc6944cb101828f20db1b3c405182192eaf2800cf6687ae528349ec1d73"; + assertEquals(expected, ConfigurationProperty.versionOf(new ConfigurationProperty("1", null))); + assertEquals(expected, ConfigurationProperty.versionOf(new ConfigurationProperty("1", ""))); + assertEquals(expected, ConfigurationProperty.versionOf(new ConfigurationProperty("1", " \n"))); + } + + @Test + public void versionOf_NullValueIsEmpty() { + String expected = "ffe679bb831c95b67dc17819c63c5090d221aac6f4c7bf530f594ab43d21fa1e"; + assertEquals(expected, ConfigurationProperty.versionOf(new ConfigurationProperty(null, null))); + assertEquals(expected, ConfigurationProperty.versionOf(new ConfigurationProperty("", null))); + } + + @Test + public void versionOf_MissingEntryIsEmptyString() { + assertEquals("", ConfigurationProperty.versionOf(null)); + } + + @Test + public void versionOf_ValueAndCommentAreSeparated() { + assertNotEquals(ConfigurationProperty.versionOf(new ConfigurationProperty("ab", null)), ConfigurationProperty.versionOf(new ConfigurationProperty("a", "b"))); + } +} From ccbc62924f3cc76e6e95af90ff06f9e006bc623c Mon Sep 17 00:00:00 2001 From: Tony Germano Date: Tue, 27 Oct 2026 05:26:12 -0400 Subject: [PATCH 5/5] Add an expected version to the configuration map entry update PUT /configurationMap/{key} takes an optional expectedVersion query parameter. If it is set and doesn't match the current entry's version from ConfigurationProperty.versionOf, nothing is changed and the endpoint returns false. An empty string requires that the key doesn't exist yet. Without the parameter the entry is updated unconditionally, as before, and the endpoint returns true. This lets two clients updating the same key detect a lost update without storing a revision. Reporting it as a false result follows the revision checks on channels, channel groups and code templates, which return false instead of an error status. The check runs inside the controller's synchronized update. The smoke test runs the update against a live server, including calculating the version from the entry the server returned. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Tony Germano --- .../com/mirth/connect/client/core/Client.java | 9 +- .../ConfigurationServletInterface.java | 8 +- .../api/servlets/ConfigurationServlet.java | 4 +- .../controllers/ConfigurationController.java | 7 +- .../DefaultConfigurationController.java | 7 +- .../DefaultConfigurationControllerTest.java | 63 +++++++++++++- .../smoketest/ConfigurationMapEntryTest.java | 82 ++++++++++++++++++- 7 files changed, 168 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/com/mirth/connect/client/core/Client.java b/server/src/main/java/com/mirth/connect/client/core/Client.java index f3ebd6b301..6635335f42 100644 --- a/server/src/main/java/com/mirth/connect/client/core/Client.java +++ b/server/src/main/java/com/mirth/connect/client/core/Client.java @@ -810,13 +810,16 @@ public void setConfigurationMap(Map map) throws C } /** - * Updates a single entry in the configuration map, leaving the rest alone. + * Updates a single entry in the configuration map, leaving the rest alone. If expectedVersion is + * not null and doesn't match the entry's current version, nothing is changed and false is + * returned. * * @see ConfigurationServletInterface#setConfigurationProperty + * @see ConfigurationProperty#versionOf */ @Override - public void setConfigurationProperty(String key, ConfigurationProperty property) throws ClientException { - getServlet(ConfigurationServletInterface.class).setConfigurationProperty(key, property); + public boolean setConfigurationProperty(String key, ConfigurationProperty property, String expectedVersion) throws ClientException { + return getServlet(ConfigurationServletInterface.class).setConfigurationProperty(key, property, expectedVersion); } /** diff --git a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java index db57984e0b..2c54864e3f 100644 --- a/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java +++ b/server/src/main/java/com/mirth/connect/client/core/api/servlets/ConfigurationServletInterface.java @@ -310,13 +310,17 @@ public void setConfigurationMap(@Param("map") @RequestBody(description = "The ne @Path("/configurationMap/{key}") @Operation(summary = "Updates a single entry in the configuration map, leaving the rest alone.") @MirthOperation(name = "setConfigurationProperty", display = "Set configuration map entry", permission = Permissions.CONFIGURATION_MAP_EDIT) - public void setConfigurationProperty( + @Produces({ MediaType.APPLICATION_JSON, MediaType.TEXT_PLAIN }) + public boolean setConfigurationProperty( @Param("key") @Parameter(description = "The key of the entry to set. It can't be blank, and a / in the key has to be encoded as %2F.", required = true) @PathParam("key") String key, @Param("property") @RequestBody(description = "The value and comment to store under the key.", required = true, content = { @Content(mediaType = MediaType.APPLICATION_XML, examples = { @ExampleObject(name = "configurationProperty", ref = "../apiexamples/configuration_property_xml") }), @Content(mediaType = MediaType.APPLICATION_JSON, examples = { - @ExampleObject(name = "configurationProperty", ref = "../apiexamples/configuration_property_json") }) }) ConfigurationProperty property) throws ClientException; + @ExampleObject(name = "configurationProperty", ref = "../apiexamples/configuration_property_json") }) }) ConfigurationProperty property, + @Param("expectedVersion") @Parameter(description = "If set, the entry is only updated if its current version matches. Otherwise nothing is changed and false is returned. " + + "An entry's version is the hex SHA-256 of the UTF-8 bytes of its value, a unit separator (U+001F), and its comment, with a null value and a null or blank comment treated as empty strings. The comparison ignores case. " + + "Use an empty string to require that the key doesn't exist yet. If not set, the entry is updated unconditionally.") @QueryParam("expectedVersion") String expectedVersion) throws ClientException; @GET @Path("/databaseDrivers") diff --git a/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java b/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java index 6afd337564..644af6cd28 100644 --- a/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java +++ b/server/src/main/java/com/mirth/connect/server/api/servlets/ConfigurationServlet.java @@ -309,13 +309,13 @@ public void setConfigurationMap(Map map) { } @Override - public void setConfigurationProperty(String key, ConfigurationProperty property) { + public boolean setConfigurationProperty(String key, ConfigurationProperty property, String expectedVersion) { if (StringUtils.isBlank(key)) { throw new MirthApiException(Response.status(Response.Status.BAD_REQUEST).entity("The configuration map key cannot be blank.").build()); } try { - configurationController.setConfigurationProperty(key, property); + return configurationController.setConfigurationProperty(key, property, expectedVersion); } catch (ControllerException e) { throw new MirthApiException(e); } diff --git a/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java b/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java index 7aa1d49dd1..18f74ff062 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/ConfigurationController.java @@ -338,8 +338,13 @@ public static ConfigurationController getInstance() { * The key of the entry to set. * @param property * The value and comment to store under the key. + * @param expectedVersion + * If not null, the entry is only set if this matches the current entry's version, + * as returned by {@link ConfigurationProperty#versionOf}, ignoring case. An empty + * string requires that the key doesn't exist yet. + * @return false if expectedVersion didn't match and nothing was changed, otherwise true. */ - public abstract void setConfigurationProperty(String key, ConfigurationProperty property) throws ControllerException; + public abstract boolean setConfigurationProperty(String key, ConfigurationProperty property, String expectedVersion) throws ControllerException; // properties public Properties getPropertiesForGroup(String group) { diff --git a/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java b/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java index bc01bd9e94..3fa96e461b 100644 --- a/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java +++ b/server/src/main/java/com/mirth/connect/server/controllers/DefaultConfigurationController.java @@ -939,10 +939,15 @@ public synchronized void setConfigurationProperties(Map properties = getConfigurationProperties(); + if (expectedVersion != null && !expectedVersion.equalsIgnoreCase(ConfigurationProperty.versionOf(properties.get(key)))) { + return false; + } + properties.put(key, property); setConfigurationProperties(properties, true); + return true; } @Override diff --git a/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java b/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java index 571c4f3dad..f1b56db6d8 100644 --- a/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java +++ b/server/src/test/java/com/mirth/connect/server/controllers/DefaultConfigurationControllerTest.java @@ -10,6 +10,7 @@ package com.mirth.connect.server.controllers; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; @@ -293,7 +294,7 @@ public void setConfigurationProperty_KeepsOtherEntries() throws Exception { AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); - configurationController.setConfigurationProperty("B", new ConfigurationProperty("2", "second")); + assertTrue(configurationController.setConfigurationProperty("B", new ConfigurationProperty("2", "second"), null)); Map expected = new HashMap(); expected.put("A", "1"); @@ -308,6 +309,66 @@ public void setConfigurationProperty_KeepsOtherEntries() throws Exception { assertEquals("second", saved.get("B").getComment()); } + @Test + public void setConfigurationProperty_ExpectedVersionMatches() throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + String version = ConfigurationProperty.versionOf(new ConfigurationProperty("1", "comment")); + + assertTrue(configurationController.setConfigurationProperty("A", new ConfigurationProperty("2", "comment"), version)); + assertEquals("2", configurationController.getConfigurationMap().get("A")); + } + + @Test + public void setConfigurationProperty_ExpectedVersionMatchesIgnoringCase() throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + String version = ConfigurationProperty.versionOf(new ConfigurationProperty("1", "comment")).toUpperCase(); + + assertTrue(configurationController.setConfigurationProperty("A", new ConfigurationProperty("2", "comment"), version)); + assertEquals("2", configurationController.getConfigurationMap().get("A")); + } + + @Test + public void setConfigurationProperty_ExpectedVersionMismatch() throws Exception { + assertConflict("A", ConfigurationProperty.versionOf(new ConfigurationProperty("0", "comment"))); + } + + @Test + public void setConfigurationProperty_ExpectedMissing_KeyExists() throws Exception { + assertConflict("A", ""); + } + + @Test + public void setConfigurationProperty_ExpectedVersion_KeyMissing() throws Exception { + assertConflict("B", ConfigurationProperty.versionOf(new ConfigurationProperty("1", "comment"))); + } + + @Test + public void setConfigurationProperty_ExpectedMissing_KeyMissing() throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + + assertTrue(configurationController.setConfigurationProperty("B", new ConfigurationProperty("2", null), "")); + + Map expected = new HashMap(); + expected.put("A", "1"); + expected.put("B", "2"); + assertEquals(expected, configurationController.getConfigurationMap()); + } + + private void assertConflict(String key, String expectedVersion) throws Exception { + AtomicReference database = new AtomicReference(INITIAL_CONFIGURATION_MAP_XML); + DefaultConfigurationController configurationController = databaseBackedConfigurationController(database); + Map before = configurationController.getConfigurationMap(); + String savedBefore = database.get(); + + assertFalse(configurationController.setConfigurationProperty(key, new ConfigurationProperty("2", "second"), expectedVersion)); + + assertEquals(savedBefore, database.get()); + assertEquals(before, configurationController.getConfigurationMap()); + } + private DefaultConfigurationController databaseBackedConfigurationController(AtomicReference database) { DefaultConfigurationController.mirthConfig.setProperty(DefaultConfigurationController.CONFIGURATION_MAP_LOCATION, "database"); diff --git a/smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java b/smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java index 7ce65ac79b..ea44646016 100644 --- a/smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java +++ b/smoketest/src/test/java/org/openintegrationengine/smoketest/ConfigurationMapEntryTest.java @@ -3,9 +3,15 @@ package org.openintegrationengine.smoketest; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Map; +import java.util.UUID; + +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -13,20 +19,92 @@ import com.mirth.connect.client.core.api.servlets.ConfigurationServletInterface; import com.mirth.connect.util.ConfigurationProperty; -/** Exercises the single-entry configuration map update through the Java client and a live server. */ +/** + * Exercises the single-entry configuration map update and its expected-version check through the + * Java client and a live server. The matching rules themselves are covered by + * DefaultConfigurationControllerTest; these tests cover what only a live server can show. + */ @DisplayName("Configuration map entry update") class ConfigurationMapEntryTest { + /** Unique per run, so the test never touches entries it didn't create. */ + private static final String PREFIX = "smoketest." + UUID.randomUUID() + "."; + + private static final String KEY = PREFIX + "key"; + + @AfterEach + void removeTestEntries() throws Exception { + Map map = configuration().getConfigurationMap(); + if (map.keySet().removeIf(key -> key.startsWith(PREFIX))) { + configuration().setConfigurationMap(map); + } + } + /** The key check runs in the servlet, so only a request to a live server reaches it. */ @Test @DisplayName("rejects a blank key") void rejectsBlankKey() throws Exception { - assertThrows(ClientException.class, () -> configuration().setConfigurationProperty(" ", new ConfigurationProperty("1", null))); + assertThrows(ClientException.class, () -> configuration().setConfigurationProperty(" ", new ConfigurationProperty("1", null), null)); assertFalse(configuration().getConfigurationMap().containsKey(" ")); } + /** + * A null expected version has to be left out of the request. Sent as an empty string, it would + * require that the key doesn't exist. + */ + @Test + @DisplayName("updates unconditionally without an expected version") + void updatesWithoutExpectedVersion() throws Exception { + assertTrue(configuration().setConfigurationProperty(KEY, new ConfigurationProperty("1", null), null)); + + assertTrue(configuration().setConfigurationProperty(KEY, new ConfigurationProperty("2", null), null)); + + assertEquals("2", value(KEY)); + } + + /** An empty expected version has to reach the server as an empty string, not as null. */ + @Test + @DisplayName("creates an entry with an empty expected version only if the key is missing") + void createsOnlyIfMissing() throws Exception { + assertTrue(configuration().setConfigurationProperty(KEY, new ConfigurationProperty("1", null), "")); + + assertFalse(configuration().setConfigurationProperty(KEY, new ConfigurationProperty("2", null), "")); + assertEquals("1", value(KEY)); + } + + /** + * The version a client calculates from what the server returned has to match the one the server + * calculates, including for multi-line and non-ASCII content. + */ + @Test + @DisplayName("updates when the expected version matches the entry read from the server") + void updatesWhenVersionMatches() throws Exception { + configuration().setConfigurationProperty(KEY, new ConfigurationProperty("line 1\nline 2 é", "comment ✓"), null); + ConfigurationProperty current = configuration().getConfigurationMap().get(KEY); + + assertTrue(configuration().setConfigurationProperty(KEY, new ConfigurationProperty("2", null), ConfigurationProperty.versionOf(current))); + + assertEquals("2", value(KEY)); + } + + /** A rejected update has to come back as false and leave the stored entry unchanged. */ + @Test + @DisplayName("rejects an update based on a stale version") + void rejectsStaleVersion() throws Exception { + configuration().setConfigurationProperty(KEY, new ConfigurationProperty("1", null), null); + String staleVersion = ConfigurationProperty.versionOf(configuration().getConfigurationMap().get(KEY)); + assertTrue(configuration().setConfigurationProperty(KEY, new ConfigurationProperty("2", null), staleVersion)); + + assertFalse(configuration().setConfigurationProperty(KEY, new ConfigurationProperty("3", null), staleVersion)); + assertEquals("2", value(KEY)); + } + private static ConfigurationServletInterface configuration() { return SharedServer.get().servlet(ConfigurationServletInterface.class); } + + private static String value(String key) throws Exception { + return configuration().getConfigurationMap().get(key).getValue(); + } }