[
https://issues.apache.org/jira/browse/ARTEMIS-4025?focusedWorklogId=815617&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-815617
]
ASF GitHub Bot logged work on ARTEMIS-4025:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 11/Oct/22 10:59
Start Date: 11/Oct/22 10:59
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4241:
URL: https://github.com/apache/activemq-artemis/pull/4241#discussion_r992158342
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java:
##########
@@ -523,22 +542,140 @@ public void parsePrefixedProperties(Properties
properties, String prefix) throws
}
key = entry.getKey().toString().substring(prefix.length());
}
- String value =
XMLUtil.replaceSystemPropsInString(entry.getValue().toString());
+ String value = entry.getValue().toString();
+
+ checksum.update(key.getBytes(StandardCharsets.UTF_8));
+ checksum.update('=');
+ checksum.update(value.getBytes(StandardCharsets.UTF_8));
+
+ value = XMLUtil.replaceSystemPropsInString(value);
value = PasswordMaskingUtil.resolveMask(isMaskPassword(), value,
getPasswordCodec());
key = XMLUtil.replaceSystemPropsInString(key);
logger.debug("Property config, " + key + "=" + value);
beanProperties.put(key, value);
}
+ alder32Hash = checksum.getValue();
}
+ updateReadPropertiesStatus(name, alder32Hash);
if (!beanProperties.isEmpty()) {
- populateWithProperties(beanProperties);
+ populateWithProperties(name, beanProperties);
}
}
- public void populateWithProperties(Map<String, Object> beanProperties)
throws InvocationTargetException, IllegalAccessException {
+ public void populateWithProperties(final String propsId, Map<String,
Object> beanProperties) throws InvocationTargetException,
IllegalAccessException {
CollectionAutoFillPropertiesUtil autoFillCollections = new
CollectionAutoFillPropertiesUtil();
- BeanUtilsBean beanUtils = new BeanUtilsBean(new ConvertUtilsBean(),
autoFillCollections);
+ BeanUtilsBean beanUtils = new BeanUtilsBean(new ConvertUtilsBean(),
autoFillCollections) {
+ // override to treat missing properties as errors, not skip as the
default impl does
+ @Override
+ public void setProperty(final Object bean, String name, final Object
value) throws InvocationTargetException, IllegalAccessException {
+ {
+ logger.trace("setProperty on {}, name: {}, value: {}",
bean.getClass(), name, value);
Review Comment:
As a 3-arg it should use isTraceEnabled() gate to avoid array overheads when
not in use.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java:
##########
@@ -611,13 +614,24 @@ public void setProperties(String
fileUrltoBrokerProperties) {
propertiesFileUrl = fileUrltoBrokerProperties;
}
+ @Override
+ public String getStatus() {
+ return serverStatus.asJson();
+ }
+
+ @Override
+ public void updateStatus(String component, String statusJson) {
+ serverStatus.update(component, statusJson);
+ }
+
private void internalStart() throws Exception {
if (state != SERVER_STATE.STOPPED) {
logger.debug("Server already started!");
return;
}
configuration.parseProperties(propertiesFileUrl);
+ updateStatus("configuration", configuration.getStatus());
Review Comment:
It could just be in the impl classes if its tied to the impl+tests, or a
specific class for more general ones. Anywhere really, that helps tie the
related bits together rather than expecting people will properly
follow/associate literals later. This class has 2 related updates for
"configuration" but thats not so obvious..whereas following a shared ref to the
name would make it much more so.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java:
##########
@@ -599,7 +736,51 @@ public <T> T convert(Class<T> type, Object value) {
BeanSupport.customise(beanUtils);
- beanUtils.populate(this, beanProperties);
+ logger.trace("populate: bean: {} with {}", this, beanProperties);
+
+ HashMap<String, String> errors = new HashMap<>();
+ // Loop through the property name/value pairs to be set
+ for (final Map.Entry<String, ? extends Object> entry :
beanProperties.entrySet()) {
+ // Identify the property name and value(s) to be assigned
+ final String name = entry.getKey();
+ try {
+ // Perform the assignment for this property
+ beanUtils.setProperty(this, name, entry.getValue());
+ } catch (InvocationTargetException invocationTargetException) {
+ logger.trace("failed to populate property with key: {}", name,
invocationTargetException);
+ Throwable toLog = invocationTargetException;
+ if (invocationTargetException.getCause() != null) {
+ toLog = invocationTargetException.getCause();
+ }
+ trackError(errors, entry, toLog);
+
+ } catch (Exception oops) {
+ trackError(errors, entry, oops);
+ }
+ }
+ updateApplyStatus(propsId, errors);
+ }
+
+ private void trackError(HashMap<String, String> errors, Map.Entry<String,?>
entry, Throwable oops) {
+ logger.debug("failed to populate property entry({}), reason: {}",
entry.toString(), oops.getLocalizedMessage(), oops);
Review Comment:
Ditto
Issue Time Tracking
-------------------
Worklog Id: (was: 815617)
Time Spent: 1h 10m (was: 1h)
> properties config - provide error status for invalid properties
> ----------------------------------------------------------------
>
> Key: ARTEMIS-4025
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4025
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: Configuration
> Affects Versions: 2.26.0
> Reporter: Gary Tully
> Assignee: Gary Tully
> Priority: Major
> Fix For: 2.27.0
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> following up on the availability of a [status json|ARTEMIS-4007] - trap any
> errors from bean util property failure to apply, for any in invalid property.
> Currently all failures to find setters are silently ignored.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
