diff --git a/CHANGELOG.md b/CHANGELOG.md index f4fe0e3..8d73f79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +## 1.7.1-wip + +* [FIX] restore robust cyclic detection when `filter` callbacks wrap values in fresh containers by tracking object identity before filter/date transformations +* [FIX] improve deep path handling in encoder key materialization/dot-encoding via iterative `KeyPathNode` caching (avoids recursive overflow risk and reuses ancestor caches) +* [CHORE] refactor encoder internals to share immutable frame config through new `EncodeConfig` and reduce per-frame option duplication +* [CHORE] replace `weak_map` usage in encode cycle tracking with identity-based `Set` side-channel and remove `weak_map` dependency +* [CHORE] expand encoder regression coverage with new tests for filter-wrapped cycles, `KeyPathNode` caching/encoding edge cases, and `EncodeConfig.copyWith` sentinel behavior +* [CHORE] refine decode internals with clearer duplicate-handling branching and a small dot-decoding fast-path guard (`cleanRoot.contains('%2')`) + ## 1.7.0 * [FEAT] add `DecodeOptions.throwOnLimitExceeded` for strict limit enforcement on parameter, list, and depth overflows diff --git a/lib/src/extensions/decode.dart b/lib/src/extensions/decode.dart index 35d42f6..af9f1c6 100644 --- a/lib/src/extensions/decode.dart +++ b/lib/src/extensions/decode.dart @@ -191,10 +191,19 @@ extension _$Decode on QS { // Duplicate key policy: combine/first/last (default: combine). final bool existing = obj.containsKey(key); - if (existing && options.duplicates == Duplicates.combine) { - obj[key] = Utils.combine(obj[key], val, listLimit: options.listLimit); - } else if (!existing || options.duplicates == Duplicates.last) { - obj[key] = val; + switch ((existing, options.duplicates)) { + case (true, Duplicates.combine): + // Existing key + `combine` policy: merge old/new values. + obj[key] = Utils.combine(obj[key], val, listLimit: options.listLimit); + break; + case (false, _): + case (true, Duplicates.last): + // New key, or `last` policy: store the current value. + obj[key] = val; + break; + case (true, Duplicates.first): + // Existing key + `first` policy: keep the original value. + break; } } @@ -285,7 +294,7 @@ extension _$Decode on QS { final bool wasBracketed = root.startsWith('[') && root.endsWith(']'); final String cleanRoot = wasBracketed ? root.slice(1, root.length - 1) : root; - String decodedRoot = options.decodeDotInKeys + String decodedRoot = options.decodeDotInKeys && cleanRoot.contains('%2') ? cleanRoot.replaceAll('%2E', '.').replaceAll('%2e', '.') : cleanRoot; @@ -301,8 +310,8 @@ extension _$Decode on QS { int opens = 0, closes = 0; for (int k = 0; k < decodedRoot.length; k++) { final cu = decodedRoot.codeUnitAt(k); - if (cu == 0x5B) opens++; - if (cu == 0x5D) closes++; + if (cu == 0x5B) opens++; // '[' + if (cu == 0x5D) closes++; // ']' } if (opens > closes) { decodedRoot = decodedRoot.substring(0, decodedRoot.length - 1); @@ -404,9 +413,9 @@ extension _$Decode on QS { // Balance nested '[' and ']' within this group. while (i < n) { final int cu = key.codeUnitAt(i); - if (cu == 0x5B) { + if (cu == 0x5B /* '[' */) { level++; - } else if (cu == 0x5D) { + } else if (cu == 0x5D /* ']' */) { level--; if (level == 0) { close = i; diff --git a/lib/src/extensions/encode.dart b/lib/src/extensions/encode.dart index 96530d2..7b980a2 100644 --- a/lib/src/extensions/encode.dart +++ b/lib/src/extensions/encode.dart @@ -11,11 +11,20 @@ part of '../qs.dart'; /// Implementation notes: /// - *undefined* (bool parameter): marks a missing value (e.g., absent map key) rather than /// a present-but-null value. This affects whether we emit a key or skip it. -/// - *sideChannel* ([WeakMap]): threads state through recursive calls to detect cycles -/// without retaining the entire object graph. -/// - *prefix*: current key path being built (e.g., `user[address]`), with optional `?` prefix. +/// - *sideChannel* (`Set`): tracks the active traversal path for O(1) +/// cycle detection. +/// - *prefix*: seeds the root [KeyPathNode] for traversal; child paths are +/// advanced via `KeyPathNode.append(...)`. extension _$Encode on QS { + static final ListFormatGenerator _indicesGenerator = + ListFormat.indices.generator; + static final ListFormatGenerator _bracketsGenerator = + ListFormat.brackets.generator; + static final ListFormatGenerator _repeatGenerator = + ListFormat.repeat.generator; + static final ListFormatGenerator _commaGenerator = ListFormat.comma.generator; + /// Core encoder (iterative, stack-based). /// /// Returns a `List` of encoded fragments; the top-level caller joins @@ -24,112 +33,62 @@ extension _$Encode on QS { /// Parameters (most mirror Node `qs`): /// - [object]: The current value to encode (map/iterable/scalar/byte buffer/date). /// - [undefined]: Marks a *missing* value (e.g., absent map key). When `true`, nothing is emitted. - /// - [sideChannel]: Weak side-channel used for cycle detection across recursive calls. - /// - [prefix]: Current key path (e.g., `user[address]`). If `addQueryPrefix` is true at the root, we start with `?`. - /// - [generateArrayPrefix]: Strategy for array key generation (brackets/indices/repeat/comma). - /// - [commaRoundTrip]: When true and a single-element list is encountered under `.comma`, emit `[]` to ensure the value round-trips back to an array. - /// - [commaCompactNulls]: When true, nulls are omitted from `.comma` lists. - /// - [allowEmptyLists]: If a list is empty, emit `key[]` instead of skipping. - /// - [strictNullHandling]: If a present value is `null`, emit only the key (no `=`) instead of `key=`. - /// - [skipNulls]: Skip keys whose value is `null`. - /// - [encodeDotInKeys]: Replace literal `.` in keys with `%2E`. - /// - [encoder]: Optional percent-encoder for values (and keys when `encodeValuesOnly == false`). - /// - [serializeDate]: Optional serializer for `DateTime` → String *before* encoding. - /// - [sort]: Optional comparator for determining key order at each object depth. - /// - [filter]: Either a function `(key, value) → value` or an iterable that constrains emitted keys. - /// - [allowDots]: When true, dot notation is used between path segments instead of brackets. - /// - [format]: RFC3986 or RFC1738 — influences space/plus behavior via [formatter]. - /// - [formatter]: Converts scalar strings to their final on-wire form (applies percent-encoding). - /// - [encodeValuesOnly]: When true, keys are left as-is and only values are encoded by [encoder]. - /// - [charset]: Present for parity; encoding is delegated to [encoder]/[formatter]. - /// - [addQueryPrefix]: At the root, prefix output with `?`. + /// - [sideChannel]: Active-path set used for cycle detection across traversal frames. + /// - [prefix]: Root path seed used to initialize the first [KeyPathNode]. + /// - [rootConfig]: Immutable encode options shared across traversal frames. static dynamic _encode( dynamic object, { required bool undefined, - required WeakMap sideChannel, - String? prefix, - ListFormatGenerator? generateArrayPrefix, - bool? commaRoundTrip, - bool commaCompactNulls = false, - bool allowEmptyLists = false, - bool strictNullHandling = false, - bool skipNulls = false, - bool encodeDotInKeys = false, - Encoder? encoder, - DateSerializer? serializeDate, - Sorter? sort, - dynamic filter, - bool allowDots = false, - Format format = Format.rfc3986, - Formatter? formatter, - bool encodeValuesOnly = false, - Encoding charset = utf8, - bool addQueryPrefix = false, + required Set sideChannel, + required String prefix, + required EncodeConfig rootConfig, }) { - prefix ??= addQueryPrefix ? '?' : ''; - generateArrayPrefix ??= ListFormat.indices.generator; - commaRoundTrip ??= - identical(generateArrayPrefix, ListFormat.comma.generator); - formatter ??= format.formatter; - List? result; final List stack = [ EncodeFrame( object: object, undefined: undefined, sideChannel: sideChannel, - prefix: prefix, - generateArrayPrefix: generateArrayPrefix, - commaRoundTrip: commaRoundTrip, - commaCompactNulls: commaCompactNulls, - allowEmptyLists: allowEmptyLists, - strictNullHandling: strictNullHandling, - skipNulls: skipNulls, - encodeDotInKeys: encodeDotInKeys, - encoder: encoder, - serializeDate: serializeDate, - sort: sort, - filter: filter, - allowDots: allowDots, - format: format, - formatter: formatter, - encodeValuesOnly: encodeValuesOnly, - charset: charset, + path: KeyPathNode.fromMaterialized(prefix), + config: rootConfig, onResult: (List value) => result = value, ), ]; while (stack.isNotEmpty) { final EncodeFrame frame = stack.last; + final EncodeConfig config = frame.config; if (!frame.prepared) { dynamic obj = frame.object; + String? pathText; + String materializedPath() => pathText ??= frame.path.materialize(); + final bool trackObject = obj is Map || (obj is Iterable && obj is! String); if (trackObject) { - if (frame.sideChannel.contains(obj)) { + final tracked = obj as Object; + if (frame.sideChannel.contains(tracked)) { throw RangeError('Cyclic object value'); } - frame.sideChannel[obj] = true; - frame.tracked = true; - frame.trackedObject = obj as Object; + frame.sideChannel.add(tracked); + frame.trackedObject = tracked; } - // Apply filter hook first. For dates, serialize them before any list/comma handling. - if (frame.filter is Function) { - obj = frame.filter.call(frame.prefix, obj); + // After cycle detection on the original node identity, apply filter/date/comma transforms. + if (config.filter is Function) { + obj = config.filter.call(materializedPath(), obj); } else if (obj is DateTime) { - obj = switch (frame.serializeDate) { + obj = switch (config.serializeDate) { null => obj.toIso8601String(), - _ => frame.serializeDate!(obj), + _ => config.serializeDate!(obj), }; - } else if (identical( - frame.generateArrayPrefix, ListFormat.comma.generator) && + } else if (identical(config.generateArrayPrefix, _commaGenerator) && obj is Iterable) { obj = Utils.apply( obj, (value) => value is DateTime - ? (frame.serializeDate?.call(value) ?? value.toIso8601String()) + ? (config.serializeDate?.call(value) ?? value.toIso8601String()) : value, ); } @@ -138,13 +97,15 @@ extension _$Encode on QS { // - If the value is *present* and null and strictNullHandling is on, emit only the key. // - Otherwise, treat null as an empty string. if (!frame.undefined && obj == null) { - if (frame.strictNullHandling) { + if (config.strictNullHandling) { final String keyOnly = - frame.encoder != null && !frame.encodeValuesOnly - ? frame.encoder!(frame.prefix) - : frame.prefix; - if (frame.tracked) { - frame.sideChannel.remove(frame.trackedObject ?? frame.object); + config.encoder != null && !config.encodeValuesOnly + ? config.encoder!(materializedPath()) + : materializedPath(); + final tracked = frame.trackedObject; + if (tracked != null) { + frame.sideChannel.remove(tracked); + frame.trackedObject = null; } stack.removeLast(); frame.onResult([keyOnly]); @@ -154,18 +115,18 @@ extension _$Encode on QS { } // Fast path for primitives and byte buffers → return a single key=value fragment. - if (Utils.isNonNullishPrimitive(obj, frame.skipNulls) || + if (Utils.isNonNullishPrimitive(obj, config.skipNulls) || obj is ByteBuffer) { late final String fragment; - if (frame.encoder != null) { - final String keyValue = frame.encodeValuesOnly - ? frame.prefix - : frame.encoder!(frame.prefix); + if (config.encoder != null) { + final String keyValue = config.encodeValuesOnly + ? materializedPath() + : config.encoder!(materializedPath()); fragment = - '${frame.formatter(keyValue)}=${frame.formatter(frame.encoder!(obj))}'; + '${config.formatter(keyValue)}=${config.formatter(config.encoder!(obj))}'; } else { final String valueString = obj is ByteBuffer - ? (frame.charset == utf8 + ? (config.charset == utf8 ? utf8.decode( obj.asUint8List(), allowMalformed: true, @@ -173,10 +134,12 @@ extension _$Encode on QS { : latin1.decode(obj.asUint8List())) : obj.toString(); fragment = - '${frame.formatter(frame.prefix)}=${frame.formatter(valueString)}'; + '${config.formatter(materializedPath())}=${config.formatter(valueString)}'; } - if (frame.tracked) { - frame.sideChannel.remove(frame.trackedObject ?? frame.object); + final tracked = frame.trackedObject; + if (tracked != null) { + frame.sideChannel.remove(tracked); + frame.trackedObject = null; } stack.removeLast(); frame.onResult([fragment]); @@ -185,8 +148,10 @@ extension _$Encode on QS { // Collect per-branch fragments; empty list signifies "emit nothing" for this path. if (frame.undefined) { - if (frame.tracked) { - frame.sideChannel.remove(frame.trackedObject ?? frame.object); + final tracked = frame.trackedObject; + if (tracked != null) { + frame.sideChannel.remove(tracked); + frame.trackedObject = null; } stack.removeLast(); frame.onResult(const []); @@ -206,23 +171,24 @@ extension _$Encode on QS { // - For `.comma` lists we join values in-place. // - If `filter` is Iterable, it constrains the key set. // - Otherwise derive keys from Map/Iterable, and optionally sort them. - if (identical(frame.generateArrayPrefix, ListFormat.comma.generator) && + if (identical(config.generateArrayPrefix, _commaGenerator) && obj is Iterable) { final Iterable iterableObj = obj; final List commaItems = iterableObj is List ? iterableObj : iterableObj.toList(growable: false); - final List filteredItems = frame.commaCompactNulls + final List filteredItems = config.commaCompactNulls ? commaItems.where((dynamic item) => item != null).toList() : commaItems; commaEffectiveLength = filteredItems.length; - final Iterable joinIterable = frame.encodeValuesOnly && - frame.encoder != null - ? (Utils.apply(filteredItems, frame.encoder!) as Iterable) - : filteredItems; + final Iterable joinIterable = + config.encodeValuesOnly && config.encoder != null + ? (Utils.apply(filteredItems, config.encoder!) + as Iterable) + : filteredItems; final List joinList = joinIterable is List ? joinIterable @@ -242,8 +208,8 @@ extension _$Encode on QS { {'value': const Undefined()}, ]; } - } else if (frame.filter is Iterable) { - objKeys = List.of(frame.filter); + } else if (config.filter is Iterable) { + objKeys = List.of(config.filter); } else { late final Iterable keys; if (obj is Map) { @@ -254,36 +220,37 @@ extension _$Encode on QS { } else { keys = const []; } - objKeys = frame.sort != null - ? (keys.toList()..sort(frame.sort)) + objKeys = config.sort != null + ? (keys.toList()..sort(config.sort)) : keys.toList(); } // Key-path formatting: // - Optionally encode literal dots. // - Under `.comma` with single-element lists and round-trip enabled, append []. - final String encodedPrefix = frame.encodeDotInKeys - ? frame.prefix.replaceAll('.', '%2E') - : frame.prefix; + final KeyPathNode pathForChildren = + config.encodeDotInKeys ? frame.path.asDotEncoded() : frame.path; - final bool shouldAppendRoundTripMarker = (frame.commaRoundTrip == - true) && + final bool shouldAppendRoundTripMarker = config.commaRoundTrip && seqList != null && - (identical(frame.generateArrayPrefix, ListFormat.comma.generator) && + (identical(config.generateArrayPrefix, _commaGenerator) && commaEffectiveLength != null ? commaEffectiveLength == 1 : seqList.length == 1); - final String adjustedPrefix = - shouldAppendRoundTripMarker ? '$encodedPrefix[]' : encodedPrefix; + final KeyPathNode adjustedPath = shouldAppendRoundTripMarker + ? pathForChildren.append('[]') + : pathForChildren; // Emit `key[]` when an empty list is allowed, to preserve shape on round-trip. - if (frame.allowEmptyLists && seqList != null && seqList.isEmpty) { - if (frame.tracked) { - frame.sideChannel.remove(frame.trackedObject ?? frame.object); + if (config.allowEmptyLists && seqList != null && seqList.isEmpty) { + final tracked = frame.trackedObject; + if (tracked != null) { + frame.sideChannel.remove(tracked); + frame.trackedObject = null; } stack.removeLast(); - frame.onResult(['$adjustedPrefix[]']); + frame.onResult([adjustedPath.append('[]').materialize()]); continue; } @@ -292,13 +259,15 @@ extension _$Encode on QS { frame.objKeys = objKeys; frame.seqList = seqList; frame.commaEffectiveLength = commaEffectiveLength; - frame.adjustedPrefix = adjustedPrefix; + frame.adjustedPath = adjustedPath; continue; } if (frame.index >= frame.objKeys.length) { - if (frame.tracked) { - frame.sideChannel.remove(frame.trackedObject ?? frame.object); + final tracked = frame.trackedObject; + if (tracked != null) { + frame.sideChannel.remove(tracked); + frame.trackedObject = null; } stack.removeLast(); frame.onResult(frame.values); @@ -341,51 +310,51 @@ extension _$Encode on QS { } } - if (frame.skipNulls && value == null) { + if (config.skipNulls && value == null) { continue; } // Build the next key path segment using either bracket or dot notation. - final String encodedKey = frame.allowDots && frame.encodeDotInKeys + final String encodedKey = config.allowDots && config.encodeDotInKeys ? key.toString().replaceAll('.', '%2E') : key.toString(); final bool isCommaSentinel = key is Map && key.containsKey('value'); - final String keyPrefix = (isCommaSentinel && - identical(frame.generateArrayPrefix, ListFormat.comma.generator)) - ? frame.adjustedPrefix! + // Comma lists collapse to a sentinel key and reuse `frame.adjustedPath`, + // so `_buildSequenceChildPath` is not called with `_commaGenerator`. + final KeyPathNode keyPath = (isCommaSentinel && + identical(config.generateArrayPrefix, _commaGenerator)) + ? frame.adjustedPath! : (frame.seqList != null - ? frame.generateArrayPrefix(frame.adjustedPrefix!, encodedKey) - : '${frame.adjustedPrefix!}${frame.allowDots ? '.$encodedKey' : '[$encodedKey]'}'); + ? _buildSequenceChildPath( + frame.adjustedPath!, + encodedKey, + config.generateArrayPrefix, + ) + : (config.allowDots + ? frame.adjustedPath!.append('.$encodedKey') + : frame.adjustedPath!.append('[$encodedKey]'))); + + final Encoder? childEncoder = identical( + config.generateArrayPrefix, + _commaGenerator, + ) && + config.encodeValuesOnly && + frame.seqList != null + ? null + : config.encoder; + final EncodeConfig childConfig = identical(childEncoder, config.encoder) + ? config + : config.withEncoder(childEncoder); stack.add( EncodeFrame( object: value, undefined: valueUndefined, sideChannel: frame.sideChannel, - prefix: keyPrefix, - generateArrayPrefix: frame.generateArrayPrefix, - commaRoundTrip: frame.commaRoundTrip, - commaCompactNulls: frame.commaCompactNulls, - allowEmptyLists: frame.allowEmptyLists, - strictNullHandling: frame.strictNullHandling, - skipNulls: frame.skipNulls, - encodeDotInKeys: frame.encodeDotInKeys, - encoder: identical( - frame.generateArrayPrefix, ListFormat.comma.generator) && - frame.encodeValuesOnly && - frame.seqList != null - ? null - : frame.encoder, - serializeDate: frame.serializeDate, - sort: frame.sort, - filter: frame.filter, - allowDots: frame.allowDots, - format: frame.format, - formatter: frame.formatter, - encodeValuesOnly: frame.encodeValuesOnly, - charset: frame.charset, + path: keyPath, + config: childConfig, onResult: (List encoded) { frame.values.addAll(encoded); }, @@ -395,4 +364,25 @@ extension _$Encode on QS { return result ?? const []; } + + /// For custom [generator] callbacks, the callback owns the full key path + /// string and receives `adjustedPath.materialize()` as input. The fallback + /// uses [KeyPathNode.fromMaterialized], which creates a fresh depth-1 root + /// without sharing ancestor nodes, so incremental path caching is not reused. + static KeyPathNode _buildSequenceChildPath( + KeyPathNode adjustedPath, + String encodedKey, + ListFormatGenerator generator, + ) => + switch (generator) { + ListFormatGenerator gen when identical(gen, _indicesGenerator) => + adjustedPath.append('[$encodedKey]'), + ListFormatGenerator gen when identical(gen, _bracketsGenerator) => + adjustedPath.append('[]'), + ListFormatGenerator gen when identical(gen, _repeatGenerator) => + adjustedPath, + _ => KeyPathNode.fromMaterialized( + generator(adjustedPath.materialize(), encodedKey), + ), + }; } diff --git a/lib/src/models/encode_config.dart b/lib/src/models/encode_config.dart new file mode 100644 index 0000000..f26af9e --- /dev/null +++ b/lib/src/models/encode_config.dart @@ -0,0 +1,156 @@ +import 'dart:convert' show Encoding; + +import 'package:equatable/equatable.dart'; +import 'package:meta/meta.dart' show internal; +import 'package:qs_dart/src/enums/format.dart'; +import 'package:qs_dart/src/enums/list_format.dart'; +import 'package:qs_dart/src/models/encode_options.dart'; + +/// Immutable configuration shared across encoder traversal frames. +@internal +final class EncodeConfig with EquatableMixin { + const EncodeConfig({ + required this.generateArrayPrefix, + required this.commaRoundTrip, + required this.commaCompactNulls, + required this.allowEmptyLists, + required this.strictNullHandling, + required this.skipNulls, + required this.encodeDotInKeys, + required this.encoder, + required this.serializeDate, + required this.sort, + required this.filter, + required this.allowDots, + required this.format, + required this.formatter, + required this.encodeValuesOnly, + required this.charset, + }); + + static const _NotSet _notSet = _NotSet(); + + final ListFormatGenerator generateArrayPrefix; + final bool commaRoundTrip; + final bool commaCompactNulls; + final bool allowEmptyLists; + final bool strictNullHandling; + final bool skipNulls; + final bool encodeDotInKeys; + final Encoder? encoder; + final DateSerializer? serializeDate; + final Sorter? sort; + final dynamic filter; + final bool allowDots; + final Format format; + final Formatter formatter; + final bool encodeValuesOnly; + final Encoding charset; + + EncodeConfig copyWith({ + ListFormatGenerator? generateArrayPrefix, + bool? commaRoundTrip, + bool? commaCompactNulls, + bool? allowEmptyLists, + bool? strictNullHandling, + bool? skipNulls, + bool? encodeDotInKeys, + Object? encoder = _notSet, + Object? serializeDate = _notSet, + Object? sort = _notSet, + Object? filter = _notSet, + bool? allowDots, + Format? format, + Formatter? formatter, + bool? encodeValuesOnly, + Encoding? charset, + }) { + final nextGenerateArrayPrefix = + generateArrayPrefix ?? this.generateArrayPrefix; + final nextCommaRoundTrip = commaRoundTrip ?? this.commaRoundTrip; + final nextCommaCompactNulls = commaCompactNulls ?? this.commaCompactNulls; + final nextAllowEmptyLists = allowEmptyLists ?? this.allowEmptyLists; + final nextStrictNullHandling = + strictNullHandling ?? this.strictNullHandling; + final nextSkipNulls = skipNulls ?? this.skipNulls; + final nextEncodeDotInKeys = encodeDotInKeys ?? this.encodeDotInKeys; + final Encoder? nextEncoder = + identical(encoder, _notSet) ? this.encoder : encoder as Encoder?; + final DateSerializer? nextSerializeDate = identical(serializeDate, _notSet) + ? this.serializeDate + : serializeDate as DateSerializer?; + final Sorter? nextSort = + identical(sort, _notSet) ? this.sort : sort as Sorter?; + final nextFilter = identical(filter, _notSet) ? this.filter : filter; + final nextAllowDots = allowDots ?? this.allowDots; + final nextFormat = format ?? this.format; + final nextFormatter = formatter ?? this.formatter; + final nextEncodeValuesOnly = encodeValuesOnly ?? this.encodeValuesOnly; + final nextCharset = charset ?? this.charset; + + if (identical(nextGenerateArrayPrefix, this.generateArrayPrefix) && + nextCommaRoundTrip == this.commaRoundTrip && + nextCommaCompactNulls == this.commaCompactNulls && + nextAllowEmptyLists == this.allowEmptyLists && + nextStrictNullHandling == this.strictNullHandling && + nextSkipNulls == this.skipNulls && + nextEncodeDotInKeys == this.encodeDotInKeys && + identical(nextEncoder, this.encoder) && + identical(nextSerializeDate, this.serializeDate) && + identical(nextSort, this.sort) && + identical(nextFilter, this.filter) && + nextAllowDots == this.allowDots && + identical(nextFormat, this.format) && + identical(nextFormatter, this.formatter) && + nextEncodeValuesOnly == this.encodeValuesOnly && + identical(nextCharset, this.charset)) { + return this; + } + + return EncodeConfig( + generateArrayPrefix: nextGenerateArrayPrefix, + commaRoundTrip: nextCommaRoundTrip, + commaCompactNulls: nextCommaCompactNulls, + allowEmptyLists: nextAllowEmptyLists, + strictNullHandling: nextStrictNullHandling, + skipNulls: nextSkipNulls, + encodeDotInKeys: nextEncodeDotInKeys, + encoder: nextEncoder, + serializeDate: nextSerializeDate, + sort: nextSort, + filter: nextFilter, + allowDots: nextAllowDots, + format: nextFormat, + formatter: nextFormatter, + encodeValuesOnly: nextEncodeValuesOnly, + charset: nextCharset, + ); + } + + EncodeConfig withEncoder(Encoder? value) => copyWith(encoder: value); + + @override + List get props => [ + generateArrayPrefix, + commaRoundTrip, + commaCompactNulls, + allowEmptyLists, + strictNullHandling, + skipNulls, + encodeDotInKeys, + encoder, + serializeDate, + sort, + filter, + allowDots, + format, + formatter, + encodeValuesOnly, + charset, + ]; +} + +/// Private compile-time sentinel for copyWith optional arguments. +final class _NotSet { + const _NotSet(); +} diff --git a/lib/src/models/encode_frame.dart b/lib/src/models/encode_frame.dart index 30daf33..f8bcfc1 100644 --- a/lib/src/models/encode_frame.dart +++ b/lib/src/models/encode_frame.dart @@ -1,10 +1,6 @@ -import 'dart:convert' show Encoding; - import 'package:meta/meta.dart' show internal; -import 'package:qs_dart/src/enums/format.dart'; -import 'package:qs_dart/src/enums/list_format.dart'; -import 'package:qs_dart/src/models/encode_options.dart'; -import 'package:weak_map/weak_map.dart'; +import 'package:qs_dart/src/models/encode_config.dart'; +import 'package:qs_dart/src/models/key_path_node.dart'; /// Internal encoder stack frame used by the iterative `_encode` traversal. /// @@ -17,23 +13,8 @@ final class EncodeFrame { required this.object, required this.undefined, required this.sideChannel, - required this.prefix, - required this.generateArrayPrefix, - required this.commaRoundTrip, - required this.commaCompactNulls, - required this.allowEmptyLists, - required this.strictNullHandling, - required this.skipNulls, - required this.encodeDotInKeys, - required this.encoder, - required this.serializeDate, - required this.sort, - required this.filter, - required this.allowDots, - required this.format, - required this.formatter, - required this.encodeValuesOnly, - required this.charset, + required this.path, + required this.config, required this.onResult, }); @@ -43,70 +24,22 @@ final class EncodeFrame { /// Whether the value is "missing" rather than explicitly present (qs semantics). final bool undefined; - /// Weak side-channel for cycle detection across the traversal path. - final WeakMap sideChannel; - - /// Fully-qualified key path prefix for this frame. - final String prefix; - - /// List key generator (indices/brackets/repeat/comma). - final ListFormatGenerator generateArrayPrefix; - - /// Emit a round-trip marker for comma lists with a single element. - final bool commaRoundTrip; - - /// Drop nulls before joining comma lists. - final bool commaCompactNulls; - - /// Whether empty lists should emit `key[]`. - final bool allowEmptyLists; - - /// Emit bare keys for explicit nulls (no `=`). - final bool strictNullHandling; - - /// Skip keys whose values are null. - final bool skipNulls; - - /// Encode literal dots in keys as `%2E`. - final bool encodeDotInKeys; + /// Active-path set used for cycle detection across the traversal path. + final Set sideChannel; - /// Optional value encoder (and key encoder when `encodeValuesOnly` is false). - final Encoder? encoder; + /// Fully-qualified key path for this frame. + final KeyPathNode path; - /// Optional serializer for DateTime values. - final DateSerializer? serializeDate; - - /// Optional key sorter for deterministic ordering. - final Sorter? sort; - - /// Filter hook or whitelist for keys at this level. - final dynamic filter; - - /// Whether to use dot notation between segments. - final bool allowDots; - - /// Output formatting mode. - final Format format; - - /// Formatter applied to already-encoded tokens. - final Formatter formatter; - - /// Encode values only (leave keys unencoded). - final bool encodeValuesOnly; - - /// Declared charset (used by encoder/formatter hooks). - final Encoding charset; + /// Shared immutable options for this frame and its siblings. + final EncodeConfig config; /// Callback invoked with this frame's encoded fragments. final void Function(List result) onResult; - /// Whether this frame has been initialized (keys computed, prefix adjusted). + /// Whether this frame has been initialized (keys/path computed). bool prepared = false; - /// Whether this frame registered a cycle-tracking entry. - bool tracked = false; - - /// The object used for cycle tracking (after filter/date transforms). + /// The object identity registered in [sideChannel] for cycle tracking. Object? trackedObject; /// Keys/indices to iterate at this level. @@ -121,8 +54,8 @@ final class EncodeFrame { /// Effective comma list length after filtering nulls. int? commaEffectiveLength; - /// Prefix after dot-encoding and comma round-trip adjustment. - String? adjustedPrefix; + /// Path after dot-encoding and comma round-trip adjustment. + KeyPathNode? adjustedPath; /// Accumulated encoded fragments from child frames. List values = []; diff --git a/lib/src/models/key_path_node.dart b/lib/src/models/key_path_node.dart new file mode 100644 index 0000000..4d6f45e --- /dev/null +++ b/lib/src/models/key_path_node.dart @@ -0,0 +1,113 @@ +import 'package:meta/meta.dart' show internal; + +/// Logically immutable linked node representation of an encoder key path. +/// +/// Structure is fixed at construction time via [KeyPathNode._] (`_parent`, +/// `_segment`, `_depth`). `_dotEncoded` and `_materialized` are lazily-populated +/// mutable caches that do not change a node's structural identity. +/// +/// Paths are rendered lazily so deep traversals only materialize key strings +/// at leaf emission points. +@internal +final class KeyPathNode { + KeyPathNode._(this._parent, this._segment) + : _depth = (_parent?._depth ?? 0) + 1; + + final KeyPathNode? _parent; + final String _segment; + final int _depth; + + KeyPathNode? _dotEncoded; + String? _materialized; + + static KeyPathNode fromMaterialized(String value) => + KeyPathNode._(null, value); + + KeyPathNode append(String segment) => + segment.isEmpty ? this : KeyPathNode._(this, segment); + + /// Returns a cached view with every literal dot replaced by `%2E`. + KeyPathNode asDotEncoded() { + final KeyPathNode? cached = _dotEncoded; + if (cached != null) { + return cached; + } + + final List uncached = []; + KeyPathNode? cursor = this; + while (cursor != null && cursor._dotEncoded == null) { + uncached.add(cursor); + cursor = cursor._parent; + } + + KeyPathNode? encodedParent = cursor?._dotEncoded; + for (int i = uncached.length - 1; i >= 0; i--) { + final KeyPathNode node = uncached[i]; + final String encodedSegment = _replaceDots(node._segment); + final KeyPathNode? nodeParent = node._parent; + final KeyPathNode encodedNode = switch (nodeParent) { + null => identical(encodedSegment, node._segment) + ? node + : KeyPathNode._(null, encodedSegment), + final KeyPathNode parent => identical(encodedParent, parent) && + identical(encodedSegment, node._segment) + ? node + : KeyPathNode._(encodedParent, encodedSegment), + }; + + node._dotEncoded = encodedNode; + encodedParent = encodedNode; + } + + // Safe: when this method starts uncached, `this` is included in `uncached`, + // and the rebuild loop always assigns `node._dotEncoded`. + return _dotEncoded!; + } + + /// Materializes the full path once and caches it. + String materialize() { + final String? cached = _materialized; + if (cached != null) return cached; + + final KeyPathNode? parent = _parent; + if (parent == null) { + _materialized = _segment; + // Safe: root branch assigns `_materialized` immediately above. + return _materialized!; + } + + if (_depth == 2) { + final String parentSegment = parent._materialized ?? parent._segment; + _materialized = '$parentSegment$_segment'; + // Safe: depth-2 fast path assigns `_materialized` immediately above. + return _materialized!; + } + + final List suffixNodes = []; + KeyPathNode? current = this; + String base = ''; + while (current != null) { + final String? cachedCurrent = current._materialized; + if (cachedCurrent != null) { + base = cachedCurrent; + break; + } + suffixNodes.add(current); + current = current._parent; + } + + final StringBuffer out = StringBuffer(base); + for (int i = suffixNodes.length - 1; i >= 0; i--) { + final KeyPathNode node = suffixNodes[i]; + out.write(node._segment); + node._materialized ??= out.toString(); + } + + // Safe: this node is part of suffixNodes when uncached, so the loop above + // always initializes `_materialized` before this return. + return _materialized!; + } + + static String _replaceDots(String value) => + value.contains('.') ? value.replaceAll('.', '%2E') : value; +} diff --git a/lib/src/qs.dart b/lib/src/qs.dart index d9a1a4d..35fe8ad 100644 --- a/lib/src/qs.dart +++ b/lib/src/qs.dart @@ -1,3 +1,4 @@ +import 'dart:collection' show HashSet; import 'dart:convert' show latin1, utf8, Encoding; import 'dart:typed_data' show ByteBuffer; @@ -7,11 +8,12 @@ import 'package:qs_dart/src/enums/list_format.dart'; import 'package:qs_dart/src/enums/sentinel.dart'; import 'package:qs_dart/src/extensions/extensions.dart'; import 'package:qs_dart/src/models/decode_options.dart'; +import 'package:qs_dart/src/models/encode_config.dart'; import 'package:qs_dart/src/models/encode_frame.dart'; import 'package:qs_dart/src/models/encode_options.dart'; +import 'package:qs_dart/src/models/key_path_node.dart'; import 'package:qs_dart/src/models/undefined.dart'; import 'package:qs_dart/src/utils.dart'; -import 'package:weak_map/weak_map.dart'; // Re-export for public API: consumers can `import 'package:qs_dart/qs.dart'` and access DecodeKind export 'package:qs_dart/src/enums/decode_kind.dart'; @@ -136,8 +138,32 @@ final class QS { objKeys.sort(options.sort); } - // Internal side-channel used by the encoder to detect cycles and share state. - final WeakMap sideChannel = WeakMap(); + // Active-path set used by the encoder for cycle detection across frames. + final Set sideChannel = HashSet.identity(); + final ListFormatGenerator gen = options.listFormat.generator; + final bool crt = identical(gen, ListFormat.comma.generator) && + options.commaRoundTrip == true; + final bool ccn = identical(gen, ListFormat.comma.generator) && + options.commaCompactNulls == true; + final EncodeConfig rootConfig = EncodeConfig( + generateArrayPrefix: gen, + commaRoundTrip: crt, + commaCompactNulls: ccn, + allowEmptyLists: options.allowEmptyLists, + strictNullHandling: options.strictNullHandling, + skipNulls: options.skipNulls, + encodeDotInKeys: options.encodeDotInKeys, + encoder: options.encode ? options.encoder : null, + serializeDate: options.serializeDate, + sort: options.sort, + filter: options.filter, + allowDots: options.allowDots, + format: options.format, + formatter: options.formatter, + encodeValuesOnly: options.encodeValuesOnly, + charset: options.charset, + ); + for (int i = 0; i < objKeys.length; i++) { final key = objKeys[i]; @@ -145,34 +171,12 @@ final class QS { continue; } - final ListFormatGenerator gen = options.listFormat.generator; - final bool crt = identical(gen, ListFormat.comma.generator) && - options.commaRoundTrip == true; - final bool ccn = identical(gen, ListFormat.comma.generator) && - options.commaCompactNulls == true; - final encoded = _$Encode._encode( obj[key], undefined: !obj.containsKey(key), - prefix: key, - generateArrayPrefix: gen, - commaRoundTrip: crt, - commaCompactNulls: ccn, - allowEmptyLists: options.allowEmptyLists, - strictNullHandling: options.strictNullHandling, - skipNulls: options.skipNulls, - encodeDotInKeys: options.encodeDotInKeys, - encoder: options.encode ? options.encoder : null, - serializeDate: options.serializeDate, - filter: options.filter, - sort: options.sort, - allowDots: options.allowDots, - format: options.format, - formatter: options.formatter, - encodeValuesOnly: options.encodeValuesOnly, - charset: options.charset, - addQueryPrefix: options.addQueryPrefix, sideChannel: sideChannel, + prefix: key, + rootConfig: rootConfig, ); if (encoded is Iterable) { diff --git a/pubspec.yaml b/pubspec.yaml index 16ce7b7..bb0715d 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -16,7 +16,6 @@ dependencies: collection: ^1.18.0 equatable: ^2.0.7 meta: ^1.9.1 - weak_map: ^4.0.1 dev_dependencies: cli_script: ^1.0.0 diff --git a/test/unit/encode_edge_cases_test.dart b/test/unit/encode_edge_cases_test.dart index 0785f05..7a22989 100644 --- a/test/unit/encode_edge_cases_test.dart +++ b/test/unit/encode_edge_cases_test.dart @@ -86,6 +86,37 @@ void main() { expect(encoded.contains('b%5Bz%5D=1'), isTrue); }); + test( + 'cycle detection still throws when filter wraps maps with fresh containers', + () { + final cyclic = {}; + cyclic['self'] = cyclic; + + var wraps = 0; + final options = EncodeOptions( + encode: false, + filter: (prefix, value) { + if (value is Map) { + wraps++; + if (wraps > 8) { + // Keep this bounded so a regression fails quickly instead of + // risking unbounded expansion during traversal. + return 'stop'; + } + + return {'wrapped': value}; + } + + return value; + }, + ); + + expect( + () => QS.encode({'root': cyclic}, options), + throwsA(isA()), + ); + }); + test('strictNullHandling with custom encoder emits only encoded key', () { final encoded = QS.encode( { diff --git a/test/unit/models/encode_config_test.dart b/test/unit/models/encode_config_test.dart new file mode 100644 index 0000000..a5ca7a2 --- /dev/null +++ b/test/unit/models/encode_config_test.dart @@ -0,0 +1,109 @@ +import 'dart:convert' show Encoding, utf8; + +import 'package:qs_dart/src/enums/format.dart'; +import 'package:qs_dart/src/enums/list_format.dart'; +import 'package:qs_dart/src/models/encode_config.dart'; +import 'package:qs_dart/src/models/encode_options.dart'; +import 'package:test/test.dart'; + +String _encoder(dynamic value, {Encoding? charset, Format? format}) => + value.toString(); +String _serializeDate(DateTime date) => date.toIso8601String(); +int _sort(dynamic a, dynamic b) => 0; + +void main() { + group('EncodeConfig', () { + test('copyWith returns same instance when unchanged', () { + final config = _baseConfig(); + + expect(identical(config.copyWith(), config), isTrue); + }); + + test('withEncoder delegates to copyWith and can clear encoder', () { + final config = _baseConfig(encoder: _encoder); + + final cleared = config.withEncoder(null); + + expect(cleared, equals(config.copyWith(encoder: null))); + expect(cleared.encoder, isNull); + expect(identical(cleared, config), isFalse); + }); + + test('copyWith can clear nullable fields', () { + final config = _baseConfig( + encoder: _encoder, + serializeDate: _serializeDate, + sort: _sort, + filter: const ['a', 'b'], + ); + + final copy = config.copyWith( + encoder: null, + serializeDate: null, + sort: null, + filter: null, + ); + + expect(copy.encoder, isNull); + expect(copy.serializeDate, isNull); + expect(copy.sort, isNull); + expect(copy.filter, isNull); + expect(copy.generateArrayPrefix, same(config.generateArrayPrefix)); + expect(copy.formatter, same(config.formatter)); + }); + + test('copyWith treats const Object filter as explicit override', () { + const marker = Object(); + final config = _baseConfig(filter: null); + + final copy = config.copyWith(filter: marker); + + expect(identical(copy.filter, marker), isTrue); + }); + + test('equatable props compare all fields', () { + final a = _baseConfig(); + final b = _baseConfig(); + + expect(a, equals(b)); + expect(a.copyWith(skipNulls: true), isNot(equals(b))); + }); + }); +} + +EncodeConfig _baseConfig({ + ListFormatGenerator? generateArrayPrefix, + bool commaRoundTrip = false, + bool commaCompactNulls = false, + bool allowEmptyLists = false, + bool strictNullHandling = false, + bool skipNulls = false, + bool encodeDotInKeys = false, + Encoder? encoder, + DateSerializer? serializeDate, + Sorter? sort, + dynamic filter, + bool allowDots = false, + Format format = Format.rfc3986, + Formatter? formatter, + bool encodeValuesOnly = false, +}) { + return EncodeConfig( + generateArrayPrefix: generateArrayPrefix ?? ListFormat.indices.generator, + commaRoundTrip: commaRoundTrip, + commaCompactNulls: commaCompactNulls, + allowEmptyLists: allowEmptyLists, + strictNullHandling: strictNullHandling, + skipNulls: skipNulls, + encodeDotInKeys: encodeDotInKeys, + encoder: encoder, + serializeDate: serializeDate, + sort: sort, + filter: filter, + allowDots: allowDots, + format: format, + formatter: formatter ?? format.formatter, + encodeValuesOnly: encodeValuesOnly, + charset: utf8, + ); +} diff --git a/test/unit/models/key_path_node_test.dart b/test/unit/models/key_path_node_test.dart new file mode 100644 index 0000000..96c7e8b --- /dev/null +++ b/test/unit/models/key_path_node_test.dart @@ -0,0 +1,97 @@ +import 'package:qs_dart/src/models/key_path_node.dart'; +import 'package:test/test.dart'; + +void main() { + group('KeyPathNode', () { + test('append returns same node for empty segment', () { + final root = KeyPathNode.fromMaterialized('a'); + + expect(identical(root.append(''), root), isTrue); + }); + + test('materialize composes full path once', () { + final node = + KeyPathNode.fromMaterialized('a').append('[b]').append('[c]'); + + final first = node.materialize(); + final second = node.materialize(); + + expect(first, equals('a[b][c]')); + expect(identical(first, second), isTrue); + }); + + test('materialize composes from nearest cached ancestor', () { + final parent = + KeyPathNode.fromMaterialized('a').append('[b]').append('[c]'); + final leaf = parent.append('[d]'); + + expect(parent.materialize(), equals('a[b][c]')); + final first = leaf.materialize(); + final second = leaf.materialize(); + + expect(first, equals('a[b][c][d]')); + expect(identical(first, second), isTrue); + }); + + test('asDotEncoded returns same node when there are no dots', () { + final node = KeyPathNode.fromMaterialized('a').append('[b]'); + + final encoded = node.asDotEncoded(); + + expect(identical(encoded, node), isTrue); + }); + + test('asDotEncoded encodes dots in a root node', () { + final root = KeyPathNode.fromMaterialized('a.b.c'); + + expect(root.asDotEncoded().materialize(), equals('a%2Eb%2Ec')); + }); + + test('asDotEncoded caches encoded view across calls', () { + final node = + KeyPathNode.fromMaterialized('a.b').append('[c.d]').append('[e]'); + + final first = node.asDotEncoded(); + final second = node.asDotEncoded(); + + expect(first.materialize(), equals('a%2Eb[c%2Ed][e]')); + expect(identical(first, second), isTrue); + }); + + test('asDotEncoded handles deep uncached chains without recursion', () { + KeyPathNode node = KeyPathNode.fromMaterialized('root.part'); + for (int i = 0; i < 5000; i++) { + node = node.append('[k$i.v$i]'); + } + + final encodedFirst = node.asDotEncoded(); + final encodedSecond = node.asDotEncoded(); + + expect(identical(encodedFirst, encodedSecond), isTrue); + expect(encodedFirst.materialize().startsWith('root%2Epart[k0%2Ev0]'), + isTrue); + expect(encodedFirst.materialize().contains('%2E'), isTrue); + }); + + test('deep chain materialize and asDotEncoded use cached results', () { + final node = KeyPathNode.fromMaterialized('a.b') + .append('[c.d]') + .append('[e.f]') + .append('[g.h]') + .append('[i]'); + + final materializedFirst = node.materialize(); + final materializedSecond = node.materialize(); + + expect(materializedFirst, equals('a.b[c.d][e.f][g.h][i]')); + expect(identical(materializedFirst, materializedSecond), isTrue); + + final encodedFirst = node.asDotEncoded(); + final encodedSecond = node.asDotEncoded(); + + expect( + encodedFirst.materialize(), equals('a%2Eb[c%2Ed][e%2Ef][g%2Eh][i]')); + expect(identical(encodedFirst, encodedSecond), isTrue); + }); + }); +}