Skip to content

ARTEMIS-5863 use empty string for null values to match properties fil… #6201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
clebertsuconic merged 1 commit into from
Jan 30, 2026
+28 −8
Merged

ARTEMIS-5863 use empty string for null values to match properties fil… #6201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions ...-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,12 +1105,9 @@ private void export(BeanUtilsBean beanUtils, Stack<String> nested, BufferedWrite
stream = stream.filter((Map.Entry<?, ?> entry)-> filterOn.isAssignableFrom(entry.getClass()));
}
stream.forEach(entry -> {
if (entry.getValue() != null) {
// nested by name
nested.push(String.valueOf(entry.getKey()));
export(beanUtils, nested, bufferedWriter, entry.getValue());
nested.pop();
}
nested.push(String.valueOf(entry.getKey()));
export(beanUtils, nested, bufferedWriter, entry.getValue());
nested.pop();
});
}
} else if (isComplexConfigObject(value)) {
Expand Down Expand Up @@ -1169,8 +1166,8 @@ private void export(BeanUtilsBean beanUtils, Stack<String> nested, BufferedWrite
}
});
} else {
// string form works ok otherwise
exportKeyValue(nested, bufferedWriter, String.valueOf(value));
// string form works ok otherwise, however we want an empty string for null to match properties syntax
exportKeyValue(nested, bufferedWriter, value == null ? "" : String.valueOf(value));
}
}

Expand All @@ -1190,6 +1187,9 @@ private void exportKeyValue(Stack<String> nested, BufferedWriter bufferedWriter,
}

private boolean isComplexConfigObject(Object value) {
if (value == null) {
return false;
}
return !(value instanceof SimpleString || value instanceof Enum<?>) && value.getClass().getPackage().getName().contains("artemis");
}

Expand Down
20 changes: 20 additions & 0 deletions ...ver/src/test/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.InputStream;
import java.io.PrintWriter;
import java.io.StringReader;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -3040,8 +3042,26 @@ public void testExportInvalidPropertyOnAcceptor() throws Exception {

// useKQueue here would generate a hashMap Value null, what would break the exportAsProperties.
configuration.addAcceptorConfiguration("test", "tcp://0.0.0.0:61616?useKQueue");

assertTrue(configuration.getAcceptorConfigurations().stream().findFirst().get().getCombinedParams().containsKey("useKQueue"));
// org.apache.activemq.artemis.utils.uri.URISchema.parseQuery generated the null on trimming the value
assertEquals(null, configuration.getAcceptorConfigurations().stream().findFirst().get().getCombinedParams().get("useKQueue"));

File fileOutput = new File(getTestDirfile(), "broker.properties");
assertDoesNotThrow(() -> configuration.exportAsProperties(fileOutput));
Properties properties = new Properties();
try (InputStream inStream = Files.newInputStream(fileOutput.toPath())) {
properties.load(inStream);
}
assertFalse(properties.isEmpty());
assertTrue(properties.containsKey("acceptorConfigurations.test.params.useKQueue"));
assertEquals("", properties.get("acceptorConfigurations.test.params.useKQueue"));
Copy link
Contributor

@clebertsuconic clebertsuconic Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should also parse the output, and make sure the value will be null on the TransportConnections.

Copy link
Contributor Author

@gtully gtully Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it won't be null, it will be an empty string!

Copy link
Contributor Author

@gtully gtully Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't currently have a way to set a null value, it looks like we may need that to keep this consistent, and export null values as the null string. and check for it when we set properties and replace it with the null object.

Copy link
Contributor

@clebertsuconic clebertsuconic Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just thinking now... I guess we should settle with empty strings... otherwise things can get messy

what if you make it null on an existing configuration.

It's kind of difficult to mess with setting as it could mess with existing configurations. Hard to predict


// a null value is not an option for properties
properties.put("networCheckNIC", "");
configuration.parsePrefixedProperties(properties, null);
assertEquals(1, configuration.getAcceptorConfigurations().size());
assertEquals("", configuration.getAcceptorConfigurations().stream().findFirst().get().getCombinedParams().get("useKQueue"));
}

/**
Expand Down