ibessonov commented on code in PR #5816:
URL: https://github.com/apache/ignite-3/pull/5816#discussion_r2102361557
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/storage/Data.java:
##########
@@ -38,17 +38,17 @@ public class Data {
* @param changeId Version.
*/
public Data(Map<String, ? extends Serializable> values, long changeId) {
- this.values = values;
+ this.values = Collections.unmodifiableMap(values);
this.changeId = changeId;
}
/**
* Get values.
*
- * @return Unmodifiable map with data values.
+ * @return values.
Review Comment:
Maybe we can delete this line whatsoever, it doesn't add much. But if you
don't want to delete it then please capitalize the first letter, thank you!
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/KeysTrackingConfigurationVisitor.java:
##########
@@ -195,32 +202,53 @@ private void endVisit(int previousKeyLength) {
currentKey.setLength(previousKeyLength);
currentPath.remove(currentPath.size() - 1);
- legacyNames.remove(legacyNames.size() - 1);
+
+ String[] legacyNames =
currentLegacyNames.remove(currentLegacyNames.size() - 1);
+ currentLegacyNamesCount -= legacyNames.length;
}
- /** Calls consumer for all legacy paths. */
- protected void processLegacyPaths(ArrayList<String> path,
BiConsumer<String, String> legacyKeyNewKeyConsumer) {
- if (path.size() == currentPath().size() - 1) {
- for (String legacyName : legacyNames.get(currentPath().size() -
1)) {
- String legacyKey = join(appendKey(path, legacyName));
- String newKey = currentKey();
+ /** Calls consumer for all variations of legacy paths, i.e. to remove them
from the storage. */
+ protected void processLegacyPaths(Consumer<String> legacyKeyConsumer) {
+ // Current path doesn't contain any legacy names.
+ if (currentLegacyNamesCount == 0) {
+ return;
+ }
- legacyKeyNewKeyConsumer.accept(legacyKey, newKey);
- }
+ // Contains all combinations of current and old names.
+ var pathVariations = new ArrayList<List<String>>(currentPath.size());
+ for (int i = 0; i < currentPath.size(); i++) {
+ ArrayList<String> variations = new ArrayList<>();
Review Comment:
```suggestion
List<String> variations = new ArrayList<>();
```
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/KeysTrackingConfigurationVisitor.java:
##########
@@ -195,32 +202,53 @@ private void endVisit(int previousKeyLength) {
currentKey.setLength(previousKeyLength);
currentPath.remove(currentPath.size() - 1);
- legacyNames.remove(legacyNames.size() - 1);
+
+ String[] legacyNames =
currentLegacyNames.remove(currentLegacyNames.size() - 1);
+ currentLegacyNamesCount -= legacyNames.length;
}
- /** Calls consumer for all legacy paths. */
- protected void processLegacyPaths(ArrayList<String> path,
BiConsumer<String, String> legacyKeyNewKeyConsumer) {
- if (path.size() == currentPath().size() - 1) {
- for (String legacyName : legacyNames.get(currentPath().size() -
1)) {
- String legacyKey = join(appendKey(path, legacyName));
- String newKey = currentKey();
+ /** Calls consumer for all variations of legacy paths, i.e. to remove them
from the storage. */
+ protected void processLegacyPaths(Consumer<String> legacyKeyConsumer) {
+ // Current path doesn't contain any legacy names.
+ if (currentLegacyNamesCount == 0) {
+ return;
+ }
- legacyKeyNewKeyConsumer.accept(legacyKey, newKey);
- }
+ // Contains all combinations of current and old names.
+ var pathVariations = new ArrayList<List<String>>(currentPath.size());
Review Comment:
I don't really get why you need to create a separate list if all the
information is already accessible in `currentPath` and `currentLegacyNames`.
Also, legacy names inside of named lists won't be handled correctly, as far
as I see. This part is missing in deprecated configurations as well, this means
that we need to create new Jira and add a huge TODO somewhere here. This is
dangerous. Could you please do it?
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
