You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2025/05/28 18:01:10 UTC
[PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
gnodet opened a new pull request, #2404:
URL: https://github.com/apache/maven/pull/2404
## Description
This PR fixes MNG-8746 by implementing property insertion order preservation in Maven's WrapperProperties class. The issue was that properties accessed through `model.getProperties()` were not maintaining their insertion order, leading to inconsistent behavior.
## Problem
The WrapperProperties class in the Maven compat layer was not preserving the insertion order of properties. When properties were added through the Properties API and then accessed via methods like `keySet()` or `entrySet()`, they would be returned in an arbitrary order (often alphabetical) rather than the order they were inserted.
## Solution
Implemented a comprehensive order preservation mechanism:
### Key Changes:
1. **Enhanced WrapperProperties Template** (`src/mdo/java/WrapperProperties.java`):
- Added internal `OrderedProperties` class that maintains insertion order
- Implemented caching mechanism to preserve order across read/write operations
- Modified all read operations to use the ordered cache
- Simplified write operations to work directly with the ordered properties
2. **OrderedProperties Implementation**:
- Uses `ArrayList<Object> keyOrder` to track key insertion order
- Overrides `put()`, `setProperty()`, and `remove()` methods to maintain order
- Custom `KeySet` and `EntrySet` implementations that iterate in insertion order
- Removed unnecessary synchronized wrappers that interfered with ordering
3. **Architecture Simplification**:
- WrapperProperties maintains its own ordered state independently
- Only syncs content (not order) with the v4 API model when needed
- Eliminated complex cache invalidation logic that was causing order loss
## Testing
Added comprehensive tests:
- `PropertiesOrderTest`: Tests v4 API model order preservation
- `WrapperPropertiesOrderTest`: Tests compat layer order preservation
- Covers various scenarios including property modification and initial properties
## Verification
The implementation ensures that:
```java
Properties props = model.getProperties();
props.setProperty("key3", "value3");
props.setProperty("key1", "value1");
props.setProperty("key2", "value2");
// keySet() now returns ["key3", "key1", "key2"] in insertion order
// instead of ["key1", "key2", "key3"] in alphabetical order
```
## Impact
- ✅ Preserves property insertion order for consistent behavior
- ✅ Maintains backward compatibility
- ✅ No breaking changes to existing APIs
- ✅ Improves reproducible builds
Fixes: https://issues.apache.org/jira/browse/MNG-8746
---
Pull Request opened by [Augment Code](https://www.augmentcode.com/) with guidance from the PR author
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2114031877
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
+ private final List<Object> keyOrder = new CopyOnWriteArrayList<>();
+
+ @Override
+ public synchronized void putAll(Map<?, ?> t) {
+ t.forEach(this::put);
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ return new KeySet();
+ }
+
+ @Override
+ public Set<Map.Entry<Object, Object>> entrySet() {
+ return new EntrySet();
+ }
+
+ @Override
+ public synchronized Object put(Object key, Object value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.put(key, value);
+ }
+
+ @Override
+ public synchronized Object setProperty(String key, String value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.setProperty(key, value);
+ }
+
+ @Override
+ public synchronized Object remove(Object key) {
+ keyOrder.remove(key);
+ return super.remove(key);
+ }
+
+ @Override
+ public synchronized void forEach(BiConsumer<? super Object, ? super Object> action) {
+ entrySet().forEach(e -> action.accept(e.getKey(), e.getValue()));
+ }
+
+ private class EntrySet extends AbstractSet<Map.Entry<Object, Object>> {
+ @Override
+ public Iterator<Map.Entry<Object, Object>> iterator() {
+ return new Iterator<Map.Entry<Object, Object>>() {
+ Iterator<Object> keyIterator = keyOrder.iterator();
+ @Override
+ public boolean hasNext() {
+ return keyIterator.hasNext();
+ }
+
+ @Override
+ public Map.Entry<Object, Object> next() {
+ Object key = keyIterator.next();
+ return new Map.Entry<>() {
+ @Override
+ public Object getKey() {
+ return key;
+ }
+
+ @Override
+ public Object getValue() {
+ return get(key);
+ }
+
+ @Override
+ public Object setValue(Object value) {
+ return WrapperProperties.this.put(key, value);
+ }
+ };
+ }
+ };
+ }
+
+ @Override
+ public int size() {
+ return keyOrder.size();
+ }
+ }
+
+ private class KeySet extends AbstractSet<Object> {
Review Comment:
No
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113612293
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
WrapperProperties(Supplier<Map<String, String>> getter, Consumer<Properties> setter) {
this.getter = getter;
this.setter = setter;
}
+ private synchronized void ensureInitialized() {
+ if (!initialized) {
+ Map<String, String> initial = getter.get();
+ orderedProps.putAll(initial);
Review Comment:
```suggestion
orderedProps.putAll(getter.get());
```
plz inline. Was wondered/searching why its not returned or used again.
type does not matter, its random impl. detail.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113619155
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
+ private final List<Object> keyOrder = new CopyOnWriteArrayList<>();
+
+ @Override
+ public synchronized void putAll(Map<?, ?> t) {
+ t.forEach(this::put);
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ return new KeySet();
+ }
+
+ @Override
+ public Set<Map.Entry<Object, Object>> entrySet() {
+ return new EntrySet();
+ }
+
+ @Override
+ public synchronized Object put(Object key, Object value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.put(key, value);
+ }
+
+ @Override
+ public synchronized Object setProperty(String key, String value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.setProperty(key, value);
+ }
+
+ @Override
+ public synchronized Object remove(Object key) {
+ keyOrder.remove(key);
+ return super.remove(key);
+ }
+
+ @Override
+ public synchronized void forEach(BiConsumer<? super Object, ? super Object> action) {
+ entrySet().forEach(e -> action.accept(e.getKey(), e.getValue()));
+ }
+
+ private class EntrySet extends AbstractSet<Map.Entry<Object, Object>> {
Review Comment:
this can stay internal in new dedicated `OrderedProperties` file.
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
+ private final List<Object> keyOrder = new CopyOnWriteArrayList<>();
+
+ @Override
+ public synchronized void putAll(Map<?, ?> t) {
+ t.forEach(this::put);
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ return new KeySet();
+ }
+
+ @Override
+ public Set<Map.Entry<Object, Object>> entrySet() {
+ return new EntrySet();
+ }
+
+ @Override
+ public synchronized Object put(Object key, Object value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.put(key, value);
+ }
+
+ @Override
+ public synchronized Object setProperty(String key, String value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.setProperty(key, value);
+ }
+
+ @Override
+ public synchronized Object remove(Object key) {
+ keyOrder.remove(key);
+ return super.remove(key);
+ }
+
+ @Override
+ public synchronized void forEach(BiConsumer<? super Object, ? super Object> action) {
+ entrySet().forEach(e -> action.accept(e.getKey(), e.getValue()));
+ }
+
+ private class EntrySet extends AbstractSet<Map.Entry<Object, Object>> {
+ @Override
+ public Iterator<Map.Entry<Object, Object>> iterator() {
+ return new Iterator<Map.Entry<Object, Object>>() {
+ Iterator<Object> keyIterator = keyOrder.iterator();
+ @Override
+ public boolean hasNext() {
+ return keyIterator.hasNext();
+ }
+
+ @Override
+ public Map.Entry<Object, Object> next() {
+ Object key = keyIterator.next();
+ return new Map.Entry<>() {
+ @Override
+ public Object getKey() {
+ return key;
+ }
+
+ @Override
+ public Object getValue() {
+ return get(key);
+ }
+
+ @Override
+ public Object setValue(Object value) {
+ return WrapperProperties.this.put(key, value);
+ }
+ };
+ }
+ };
+ }
+
+ @Override
+ public int size() {
+ return keyOrder.size();
+ }
+ }
+
+ private class KeySet extends AbstractSet<Object> {
Review Comment:
this can stay internal in new dedicated `OrderedProperties` file.
```suggestion
private static class KeySet extends AbstractSet<Object> {
```
will work too, right?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113674878
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
WrapperProperties(Supplier<Map<String, String>> getter, Consumer<Properties> setter) {
this.getter = getter;
this.setter = setter;
}
+ private synchronized void ensureInitialized() {
+ if (!initialized) {
+ Map<String, String> initial = getter.get();
+ orderedProps.putAll(initial);
Review Comment:
same topic as `Local variables should not be declared and then immediately returned or thrown`
https://sonarsource.github.io/rspec/#/rspec/S1488
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #2404:
URL: https://github.com/apache/maven/pull/2404
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113612293
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
WrapperProperties(Supplier<Map<String, String>> getter, Consumer<Properties> setter) {
this.getter = getter;
this.setter = setter;
}
+ private synchronized void ensureInitialized() {
+ if (!initialized) {
+ Map<String, String> initial = getter.get();
+ orderedProps.putAll(initial);
Review Comment:
```suggestion
orderedProps.putAll(getter.get());
```
plz inline. Was wondered/searching why its not returned or used again.
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
Review Comment:
```suggestion
private boolean initialized;
```
false is default
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
+ private final List<Object> keyOrder = new CopyOnWriteArrayList<>();
+
+ @Override
+ public synchronized void putAll(Map<?, ?> t) {
+ t.forEach(this::put);
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ return new KeySet();
+ }
+
+ @Override
+ public Set<Map.Entry<Object, Object>> entrySet() {
+ return new EntrySet();
+ }
+
+ @Override
+ public synchronized Object put(Object key, Object value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.put(key, value);
+ }
+
+ @Override
+ public synchronized Object setProperty(String key, String value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.setProperty(key, value);
+ }
+
+ @Override
+ public synchronized Object remove(Object key) {
+ keyOrder.remove(key);
+ return super.remove(key);
+ }
+
+ @Override
+ public synchronized void forEach(BiConsumer<? super Object, ? super Object> action) {
+ entrySet().forEach(e -> action.accept(e.getKey(), e.getValue()));
+ }
+
+ private class EntrySet extends AbstractSet<Map.Entry<Object, Object>> {
+ @Override
+ public Iterator<Map.Entry<Object, Object>> iterator() {
+ return new Iterator<Map.Entry<Object, Object>>() {
+ Iterator<Object> keyIterator = keyOrder.iterator();
+ @Override
+ public boolean hasNext() {
+ return keyIterator.hasNext();
+ }
+
+ @Override
+ public Map.Entry<Object, Object> next() {
+ Object key = keyIterator.next();
+ return new Map.Entry<>() {
+ @Override
+ public Object getKey() {
+ return key;
+ }
+
+ @Override
+ public Object getValue() {
+ return get(key);
+ }
+
+ @Override
+ public Object setValue(Object value) {
+ return WrapperProperties.this.put(key, value);
+ }
+ };
+ }
+ };
+ }
+
+ @Override
+ public int size() {
+ return keyOrder.size();
+ }
+ }
+
+ private class KeySet extends AbstractSet<Object> {
Review Comment:
this can stay internal.
```suggestion
private static class KeySet extends AbstractSet<Object> {
```
will work too, right?
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
+ private final List<Object> keyOrder = new CopyOnWriteArrayList<>();
+
+ @Override
+ public synchronized void putAll(Map<?, ?> t) {
+ t.forEach(this::put);
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ return new KeySet();
+ }
+
+ @Override
+ public Set<Map.Entry<Object, Object>> entrySet() {
+ return new EntrySet();
+ }
+
+ @Override
+ public synchronized Object put(Object key, Object value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.put(key, value);
+ }
+
+ @Override
+ public synchronized Object setProperty(String key, String value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.setProperty(key, value);
+ }
+
+ @Override
+ public synchronized Object remove(Object key) {
+ keyOrder.remove(key);
+ return super.remove(key);
+ }
+
+ @Override
+ public synchronized void forEach(BiConsumer<? super Object, ? super Object> action) {
+ entrySet().forEach(e -> action.accept(e.getKey(), e.getValue()));
+ }
+
+ private class EntrySet extends AbstractSet<Map.Entry<Object, Object>> {
Review Comment:
this can stay internal.
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
WrapperProperties(Supplier<Map<String, String>> getter, Consumer<Properties> setter) {
this.getter = getter;
this.setter = setter;
}
+ private synchronized void ensureInitialized() {
+ if (!initialized) {
+ Map<String, String> initial = getter.get();
+ orderedProps.putAll(initial);
+ initialized = true;
+ }
+ }
+
@Override
public String getProperty(String key) {
- return getter.get().get(key);
+ ensureInitialized();
Review Comment:
why not call once in constructor, post construct, as we mimic CDI?
this seems redundant.
- https://stackoverflow.com/questions/8519187/spring-postconstruct-vs-init-method-attribute
- https://docs.oracle.com/javase/8/docs/api/javax/annotation/PostConstruct.html
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
Review Comment:
might plz consider give dedicated file, as this is quite packed already.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2114076510
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
+ private final List<Object> keyOrder = new CopyOnWriteArrayList<>();
+
+ @Override
+ public synchronized void putAll(Map<?, ?> t) {
+ t.forEach(this::put);
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ return new KeySet();
+ }
+
+ @Override
+ public Set<Map.Entry<Object, Object>> entrySet() {
+ return new EntrySet();
+ }
+
+ @Override
+ public synchronized Object put(Object key, Object value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.put(key, value);
+ }
+
+ @Override
+ public synchronized Object setProperty(String key, String value) {
+ if (!keyOrder.contains(key)) {
+ keyOrder.add(key);
+ }
+ return super.setProperty(key, value);
+ }
+
+ @Override
+ public synchronized Object remove(Object key) {
+ keyOrder.remove(key);
+ return super.remove(key);
+ }
+
+ @Override
+ public synchronized void forEach(BiConsumer<? super Object, ? super Object> action) {
+ entrySet().forEach(e -> action.accept(e.getKey(), e.getValue()));
+ }
+
+ private class EntrySet extends AbstractSet<Map.Entry<Object, Object>> {
+ @Override
+ public Iterator<Map.Entry<Object, Object>> iterator() {
+ return new Iterator<Map.Entry<Object, Object>>() {
+ Iterator<Object> keyIterator = keyOrder.iterator();
+ @Override
+ public boolean hasNext() {
+ return keyIterator.hasNext();
+ }
+
+ @Override
+ public Map.Entry<Object, Object> next() {
+ Object key = keyIterator.next();
+ return new Map.Entry<>() {
+ @Override
+ public Object getKey() {
+ return key;
+ }
+
+ @Override
+ public Object getValue() {
+ return get(key);
+ }
+
+ @Override
+ public Object setValue(Object value) {
+ return WrapperProperties.this.put(key, value);
+ }
+ };
+ }
+ };
+ }
+
+ @Override
+ public int size() {
+ return keyOrder.size();
+ }
+ }
+
+ private class KeySet extends AbstractSet<Object> {
Review Comment:
ok sorry, i always aim to make static final.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113621527
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
Review Comment:
might plz consider give dedicated file `OrderedProperties`, as this is quite packed already.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113688639
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
WrapperProperties(Supplier<Map<String, String>> getter, Consumer<Properties> setter) {
this.getter = getter;
this.setter = setter;
}
+ private synchronized void ensureInitialized() {
+ if (!initialized) {
+ Map<String, String> initial = getter.get();
+ orderedProps.putAll(initial);
Review Comment:
- https://github.com/pmd/pmd/issues/5771
- https://github.com/pmd/pmd/issues/5770
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113899008
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -377,4 +369,131 @@ private Object writeReplace() throws java.io.ObjectStreamException {
props.putAll(getter.get());
return props;
}
+
+ private class OrderedProperties extends Properties {
Review Comment:
It's a non static class
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113612293
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
WrapperProperties(Supplier<Map<String, String>> getter, Consumer<Properties> setter) {
this.getter = getter;
this.setter = setter;
}
+ private synchronized void ensureInitialized() {
+ if (!initialized) {
+ Map<String, String> initial = getter.get();
+ orderedProps.putAll(initial);
Review Comment:
```suggestion
orderedProps.putAll(getter.get());
```
consider inline. was wondered and searching why its not returned or used again.
type does not matter, its random impl. detail.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-8746] Preserve property insertion order in WrapperProperties [maven]
Posted by "Pankraz76 (via GitHub)" <gi...@apache.org>.
Pankraz76 commented on code in PR #2404:
URL: https://github.com/apache/maven/pull/2404#discussion_r2113622004
##########
src/mdo/java/WrapperProperties.java:
##########
@@ -45,30 +50,44 @@ class WrapperProperties extends Properties {
final Supplier<Map<String, String>> getter;
final Consumer<Properties> setter;
+ private final OrderedProperties orderedProps = new OrderedProperties();
+ private boolean initialized = false;
WrapperProperties(Supplier<Map<String, String>> getter, Consumer<Properties> setter) {
this.getter = getter;
this.setter = setter;
}
+ private synchronized void ensureInitialized() {
+ if (!initialized) {
+ Map<String, String> initial = getter.get();
+ orderedProps.putAll(initial);
+ initialized = true;
+ }
+ }
+
@Override
public String getProperty(String key) {
- return getter.get().get(key);
+ ensureInitialized();
Review Comment:
why not call once in constructor or post construct, as we mimic `CDI`?
this seems redundant.
- https://stackoverflow.com/questions/8519187/spring-postconstruct-vs-init-method-attribute
- https://docs.oracle.com/javase/8/docs/api/javax/annotation/PostConstruct.html
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org