From 0c141d8b8f82ba4bab2c9bebbf04f28a7b66b258 Mon Sep 17 00:00:00 2001 From: Kelvin Kavisi <68240897+kekavc24@users.noreply.github.com> Date: Fri, 15 Mar 2024 21:27:51 +0000 Subject: [PATCH 1/2] fix: prevent counter from leaking matches > make MatchedNodeData a mutable standalone class > override increment method in counter to only add values below limit > try updating counter before yielding matches. Let counter decide the validity of the match after matching it. --- .../data/matched_node_data.dart | 29 +++++------ .../finders/custom_tracker.dart | 48 ++++++++++++++++--- .../yaml_transformers/finders/finder.dart | 21 +++++--- 3 files changed, 67 insertions(+), 31 deletions(-) diff --git a/lib/src/core/yaml_transformers/data/matched_node_data.dart b/lib/src/core/yaml_transformers/data/matched_node_data.dart index bc5fe34..453e0b1 100644 --- a/lib/src/core/yaml_transformers/data/matched_node_data.dart +++ b/lib/src/core/yaml_transformers/data/matched_node_data.dart @@ -2,12 +2,9 @@ part of '../yaml_transformer.dart'; /// Data object specifically created when a `Finder` finds it based on some /// predefined condition -@immutable -class MatchedNodeData extends NodeData { - const MatchedNodeData( - super.precedingKeys, - super.key, - super.value, +class MatchedNodeData { + MatchedNodeData( + this.node, this.matchedKeys, this.matchedValue, this.matchedPairs, @@ -21,20 +18,21 @@ class MatchedNodeData extends NodeData { required Map matchedPairs, }) { return MatchedNodeData( - nodeData.precedingKeys, - nodeData.key, - nodeData.value, + nodeData, matchedKeys, matchedValue, matchedPairs, ); } + /// Denotes [NodeData] belonging to an indexed node that has been matched + final NodeData node; + /// List of keys in [ NodeData ] path that matched any preset conditions final List matchedKeys; /// First/Only value matched from any of the values provided - final String matchedValue; + String matchedValue; /// Map of pairs that matched any pairs provided final Map matchedPairs; @@ -48,11 +46,11 @@ class MatchedNodeData extends NodeData { /// Get list of keys upto the last renameable key Iterable getUptoLastRenameable() { - final keys = super.getKeysAsString(); + final keys = node.getKeysAsString(); final lastIndex = matchedKeys.map(keys.lastIndexOf).max; // Keys to be taken, include last index plus one - return super.getKeys().take(lastIndex + 1); + return node.getKeys().take(lastIndex + 1); } /// Get path of keys upto the last renameable key @@ -61,10 +59,5 @@ class MatchedNodeData extends NodeData { } @override - List get props => [ - ...super.props, - matchedKeys, - matchedPairs, - matchedValue, - ]; + String toString() => node.toString(); } diff --git a/lib/src/core/yaml_transformers/finders/custom_tracker.dart b/lib/src/core/yaml_transformers/finders/custom_tracker.dart index 37c1c45..2f64c3d 100644 --- a/lib/src/core/yaml_transformers/finders/custom_tracker.dart +++ b/lib/src/core/yaml_transformers/finders/custom_tracker.dart @@ -5,9 +5,11 @@ part of 'finder.dart'; /// /// See [Counter]. final class MatchCounter extends CounterWithHistory { - MatchCounter({required int? limit}) : _limit = limit; + MatchCounter({this.limit}) : isStrict = limit != null; - final int? _limit; + final bool isStrict; + + final int? limit; /// Increments from [ MatchedNodeData ]. /// @@ -17,26 +19,58 @@ final class MatchCounter extends CounterWithHistory { bool incrementUsingMatch(MatchedNodeData data) { // Add any matched keys if (data.matchedKeys.isNotEmpty) { - increment(data.matchedKeys, origin: Origin.key); + final ignoredKeys = increment(data.matchedKeys, origin: Origin.key); + + data.matchedKeys.removeWhere(ignoredKeys.contains); // Remove ignored keys } // Add matched value if not empty if (data.matchedValue.isNotEmpty) { - increment([data.matchedValue], origin: Origin.value); + if (increment([data.matchedValue], origin: Origin.value).isNotEmpty) { + data.matchedValue = ''; // Remove value + } } // Add all pairs if (data.matchedPairs.isNotEmpty) { - increment(data.matchedPairs.entries, origin: Origin.pair); + final ignoredPairs = increment( + data.matchedPairs.entries, + origin: Origin.pair, + ) as List>; + + data.matchedPairs.removeWhere( + (key, value) => ignoredPairs.any( + (element) => element.key == key && element.value == value, + ), + ); } /// All must be equal or greater than limit. Other keys may have /// been found before more than once. /// /// Get set of all counts and check if any is below limit - final anyBelowLimit = _limit == null || - super.trackerState.values.toSet().any((element) => element < _limit!); + final anyBelowLimit = limit == null || + super.trackerState.values.toSet().any((element) => element < limit!); return !anyBelowLimit; } + + @override + List increment(Iterable values, {required Origin origin}) { + final ignored = []; + final valuesToAdd = [...values]; + + // In strict mode, ensure we only add those below limit + if (isStrict) { + for (final value in values) { + if (getCount(value, origin: origin) == limit) { + ignored.add(value); + valuesToAdd.remove(value); + } + } + } + + super.increment(valuesToAdd, origin: origin); + return ignored; + } } diff --git a/lib/src/core/yaml_transformers/finders/finder.dart b/lib/src/core/yaml_transformers/finders/finder.dart index 2ea0eb6..e6771f2 100644 --- a/lib/src/core/yaml_transformers/finders/finder.dart +++ b/lib/src/core/yaml_transformers/finders/finder.dart @@ -131,8 +131,8 @@ abstract base class Finder { yield* findAllSync(prefilledCounter: true).takeWhile( (value) { - /// Last value may be ignored. Last value itself causes the limit - /// to be reached. The limit is never reaches before. + /// Last value may be ignored. The last value itself causes the limit + /// to be reached. if (value.reachedLimit) lastValue = value; return !value.reachedLimit; }, @@ -150,19 +150,28 @@ abstract base class Finder { Iterable findAllSync({bool prefilledCounter = false}) sync* { /// Incase this method is called indirectly via [Finder.find] /// - /// [Finder.findByCount] always prefills the counter thus always - /// sets up the [MatchCounter] + /// [Finder.findByCount] always prefills the counter and sets up the + /// [MatchCounter] if (!prefilledCounter) _setUpCounter(null); for (final nodeData in _generator) { // Generate matched node data final matchedNodeData = generateMatch(nodeData); - // We only yield it if it is valid + /// Try increment with the counter first. + /// + /// The counter checks to make sure this [MatchedNodeData] has valid + /// matches. Matches below the limit. The [MatchedNodeData] will be + /// modified any redudant matches dropped. + /// + /// Nothing happens if below the limit + final reachedLimit = counter!.incrementUsingMatch(matchedNodeData); + + // If still valid after any modification, yield! if (matchedNodeData.isValidMatch()) { yield ( data: matchedNodeData, - reachedLimit: counter!.incrementUsingMatch(matchedNodeData), + reachedLimit: reachedLimit, ); } } From 0de12cac9baaf6cec3d5942834bd69d1b1f4ec9c Mon Sep 17 00:00:00 2001 From: Kelvin Kavisi <68240897+kekavc24@users.noreply.github.com> Date: Fri, 15 Mar 2024 21:29:14 +0000 Subject: [PATCH 2/2] refactor: update code referencing MatchedNodeData > call internal node member as MatchedNode is no longer a subclass of NodeData --- lib/src/core/yaml_transformers/replacers/replacer.dart | 2 +- .../core/yaml_transformers/replacers/value_replacer.dart | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/src/core/yaml_transformers/replacers/replacer.dart b/lib/src/core/yaml_transformers/replacers/replacer.dart index 3fd5413..31800c8 100644 --- a/lib/src/core/yaml_transformers/replacers/replacer.dart +++ b/lib/src/core/yaml_transformers/replacers/replacer.dart @@ -71,7 +71,7 @@ abstract class Replacer { // Just return the value instead as a string return _getReplacementCandidate( - matchedNodeData.data, + matchedNodeData.node.data, useFirst: useFirst, ) as T; } diff --git a/lib/src/core/yaml_transformers/replacers/value_replacer.dart b/lib/src/core/yaml_transformers/replacers/value_replacer.dart index 80998d7..74a4421 100644 --- a/lib/src/core/yaml_transformers/replacers/value_replacer.dart +++ b/lib/src/core/yaml_transformers/replacers/value_replacer.dart @@ -24,14 +24,14 @@ class ValueReplacer extends Replacer { final updatedMap = modifiable.updateIndexedMap( replacement, - target: matchedNodeData.key, - path: matchedNodeData.precedingKeys, + target: matchedNodeData.node.key, + path: matchedNodeData.node.precedingKeys, keyAndReplacement: {}, - value: matchedNodeData.value, + value: matchedNodeData.node.value, ); return ( - mapping: {matchedNodeData.data: replacement}, + mapping: {matchedNodeData.node.data: replacement}, updatedMap: YamlMap.wrap(updatedMap), ); }