Copilot commented on code in PR #15648:
URL: https://github.com/apache/pinot/pull/15648#discussion_r2138020083
##########
pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java:
##########
@@ -411,4 +486,22 @@ public static PropertiesConfiguration.PropertiesWriter
getPropertiesWriterFromWr
propertiesWriter.setGlobalSeparator(VERSIONED_CONFIG_SEPARATOR);
return propertiesWriter;
}
+
+ /**
+ * A custom {@link PropertiesConfiguration} implementation that throws an
exception when duplicate keys are added.
+ * This should only be used when reading from a file, after this, we should
copy the content to
+ * a {@link PropertiesConfiguration} object.
+ */
+ public static class NoDuplicateKeyPropertiesConfiguration extends
PropertiesConfiguration {
+ private final Set<String> _seenKeys = new HashSet<>();
+
+ @Override
+ protected void addPropertyDirect(String key, Object value) {
+ if (_seenKeys.contains(key)) {
+ throw new IllegalStateException("Duplicate key detected: " + key);
Review Comment:
Instead of throwing an IllegalStateException when a duplicate key is
detected, consider wrapping or converting it to a ConfigurationException. This
will ensure that the exception aligns with the expected exception type in the
tests and maintains consistent API behavior.
```suggestion
throw new ConfigurationException("Duplicate key detected: " + key);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
