diff --git a/CHANGELOG.md b/CHANGELOG.md index 28f7da47..059972a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ See [Conventional Commits](https://conventionalcommits.org) for commit guideline ### Fixed +* Preserve round trips for discriminated map codecs with strict output schemas, + including values used as another codec's output, without narrowing permissive + intermediate output policies. * Accept contextual Dart member names and deeply snapshot JSON-owned generated fields without changing opaque codec values. Preserve effective nullable overrides when inferring explicit JSON boundaries. diff --git a/packages/ack/CHANGELOG.md b/packages/ack/CHANGELOG.md index fe301be1..73345f92 100644 --- a/packages/ack/CHANGELOG.md +++ b/packages/ack/CHANGELOG.md @@ -2,6 +2,9 @@ ### Fixed +* Accept union-owned discriminators in strict map-backed codec outputs while + preserving declared discriminator rules and rejecting unrelated extra keys. + Already-permissive intermediate outputs retain their policies and values. * Expose the shared `deepUnmodifiableCopy` helper for generated immutable constructors. It snapshots nested collections without validating data or changing opaque noncollection objects. diff --git a/packages/ack/lib/src/schemas/codec_schema.dart b/packages/ack/lib/src/schemas/codec_schema.dart index 4a83b31d..a9b80ab2 100644 --- a/packages/ack/lib/src/schemas/codec_schema.dart +++ b/packages/ack/lib/src/schemas/codec_schema.dart @@ -185,10 +185,16 @@ final class CodecSchema } @override - CodecSchema copyWithInner(AnyAckSchema newInner) { + CodecSchema copyWithInner(AnyAckSchema newInner) => + _copyWithSchemas(input: newInner as AckSchema); + + CodecSchema _copyWithSchemas({ + AckSchema? input, + AckSchema? output, + }) { return CodecSchema._( - inputSchema: newInner as AckSchema, - outputSchema: outputSchema, + inputSchema: input ?? inputSchema, + outputSchema: output ?? outputSchema, decoder: _decoder, encoder: _encoder, decoderIdentity: _decoderIdentity, diff --git a/packages/ack/lib/src/schemas/discriminated_object_schema.dart b/packages/ack/lib/src/schemas/discriminated_object_schema.dart index 91cb0aa7..0973455e 100644 --- a/packages/ack/lib/src/schemas/discriminated_object_schema.dart +++ b/packages/ack/lib/src/schemas/discriminated_object_schema.dart @@ -246,6 +246,8 @@ final class DiscriminatedObjectSchema /// The effective schema includes this union's discriminator property as an /// exact branch literal, even when the authored branch omitted it. Wrappers /// around the branch (codecs, defaults) are preserved. + /// Object-backed codec outputs that omit the key accept its optional literal, + /// so decoded maps remain valid after union normalization adds the key. /// /// When [optionalDiscriminator] is true the injected literal is optional. /// This is used only on the encode path, where a branch encoder may legally @@ -265,15 +267,76 @@ final class DiscriminatedObjectSchema ); } - return effectiveDiscriminatedBranch( - discriminatorKey: discriminatorKey, - discriminatorValue: discriminatorValue, - branchSchema: branchSchema, - optionalDiscriminator: optionalDiscriminator, - ) + final effective = effectiveDiscriminatedBranch( + discriminatorKey: discriminatorKey, + discriminatorValue: discriminatorValue, + branchSchema: branchSchema, + optionalDiscriminator: optionalDiscriminator, + ); + return _withRuntimeDiscriminator(effective, discriminatorValue) as AckSchema; } + AnyAckSchema _withRuntimeDiscriminator( + AnyAckSchema schema, + String discriminatorValue, + ) { + if (schema is ObjectSchema) { + if (schema.additionalProperties || + schema.properties.containsKey(discriminatorKey)) { + return schema; + } + // A decoder may omit the union-owned key; normalization adds it later. + // Its output must also accept that normalized value on subsequent use. + return effectiveDiscriminatedObjectBranch( + discriminatorKey: discriminatorKey, + discriminatorValue: discriminatorValue, + objectSchema: schema, + optionalDiscriminator: true, + ); + } + if (schema is CodecSchema) { + final input = _withRuntimeDiscriminator( + schema.inputSchema, + discriminatorValue, + ); + final output = _withRuntimeDiscriminator( + schema.outputSchema as AnyAckSchema, + discriminatorValue, + ); + return identical(input, schema.inputSchema) && + identical(output, schema.outputSchema) + ? schema + : schema._copyWithSchemas(input: input, output: output); + } + if (schema is AnyOfSchema) { + final branches = [ + for (final branch in schema.schemas) + _withRuntimeDiscriminator(branch, discriminatorValue), + ]; + if (const ListEquality( + IdentityEquality(), + ).equals(branches, schema.schemas)) { + return schema; + } + return AnyOfSchema( + List.unmodifiable(branches), + isNullable: schema.isNullable, + isOptional: schema.isOptional, + description: schema.description, + constraints: schema.constraints, + refinements: schema.refinements, + ); + } + if (schema is WrapperSchema) { + final inner = _withRuntimeDiscriminator(schema.inner, discriminatorValue); + return identical(inner, schema.inner) + ? schema + : schema.copyWithInner(inner); + } + return schema; + } + @override @protected SchemaResult parseWithContext(Object? value, SchemaContext context) { diff --git a/packages/ack/test/schemas/discriminated_runtime_validation_test.dart b/packages/ack/test/schemas/discriminated_runtime_validation_test.dart index 6a3b3a36..040bc2a8 100644 --- a/packages/ack/test/schemas/discriminated_runtime_validation_test.dart +++ b/packages/ack/test/schemas/discriminated_runtime_validation_test.dart @@ -171,6 +171,264 @@ void main() { }); }); + group('map codec outputs with union-owned discriminators', () { + for (final wrapped in [false, true]) { + for (final emitsDiscriminator in [false, true]) { + test('round-trips strict ${wrapped ? 'wrapped' : 'plain'} outputs ' + '${emitsDiscriminator ? 'with' : 'without'} a discriminator', () { + final output = Ack.object({'name': Ack.string()}); + final branch = Ack.object({'name': Ack.string()}).codec( + decode: (map) => { + if (emitsDiscriminator) 'kind': 'cat', + 'name': map['name'], + }, + encode: (map) => {'name': map['name']}, + output: wrapped ? output.withDefault({'name': 'Mittens'}) : output, + ); + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': wrapped ? branch.withDefault({'name': 'Mittens'}) : branch, + }, + ); + final wire = {'kind': 'cat', 'name': 'Mittens'}; + final parsed = union.parse(wire)!; + expect(parsed, wire); + expect(() => parsed['kind'] = 'dog', throwsUnsupportedError); + expect(union.encode(parsed), wire); + final codec = _codecWithOutput(union, parsed); + expect(codec.parse('wire'), same(parsed)); + expect(codec.encode(parsed), 'wire'); + // Adapting a branch must not widen the original output schema. + expect( + _codecWithOutput(output, wire).safeParse('wire').isFail, + isTrue, + ); + }); + } + } + + test('round-trips through chained map codecs', () { + final inner = Ack.object({'name': Ack.string()}).codec( + decode: (map) => {'name': map['name']}, + encode: (map) => {'name': map['name']}, + output: Ack.object({'name': Ack.string()}), + ); + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': inner.codec( + decode: (map) => map, + encode: (map) => map, + output: Ack.object({'name': Ack.string()}), + ), + }, + ); + final wire = {'kind': 'cat', 'name': 'Mittens'}; + expect(union.encode(union.parse(wire)!), wire); + }); + + for (final unionOutput in [false, true]) { + test('preserves permissive intermediate codec outputs ' + '${unionOutput ? 'inside anyOf' : 'directly'}', () { + final output = Ack.object({ + 'name': Ack.string(), + }).passthrough().refine((map) => map['kind'] == 'internal'); + final inner = Ack.object({'name': Ack.string()}).codec( + decode: (map) => {'name': map['name'], 'kind': 'internal'}, + encode: (value) => {'name': (value as JsonMap)['name']}, + output: unionOutput ? Ack.anyOf([output]) : output, + ); + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': inner.codec( + decode: (value) => (value as JsonMap)['name']! as String, + encode: (name) => {'name': name, 'kind': 'internal'}, + ), + }, + ); + final wire = {'kind': 'cat', 'name': 'Mittens'}; + expect(union.parse(wire), 'Mittens'); + expect(union.encode('Mittens'), wire); + final codec = _codecWithOutput(union, 'Mittens'); + expect(codec.parse('wire'), 'Mittens'); + expect(codec.encode('Mittens'), 'wire'); + }); + } + + test('still rejects conflicts in permissive final map outputs', () { + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': Ack.object({'name': Ack.string()}).codec( + decode: (map) => {'name': map['name'], 'kind': 'internal'}, + encode: (map) => {'name': map['name']}, + output: Ack.object({'name': Ack.string()}).passthrough(), + ), + }, + ); + expect( + union.safeParse({'kind': 'cat', 'name': 'Mittens'}).isFail, + isTrue, + ); + expect( + union.safeEncode({'kind': 'internal', 'name': 'Mittens'}).isFail, + isTrue, + ); + }); + + test('adapts a nested codec output without running its conversions', () { + final output = Ack.object({'name': Ack.string()}).codec( + decode: (_) => throw StateError('Output decoder must not run'), + encode: (_) => throw StateError('Output encoder must not run'), + output: Ack.object({'name': Ack.string()}), + ); + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': Ack.object({'name': Ack.string()}).codec( + decode: (map) => {'name': map['name']}, + encode: (map) => {'name': map['name']}, + output: output, + ), + }, + ); + final wire = {'kind': 'cat', 'name': 'Mittens'}; + final parsed = union.parse(wire)!; + expect(union.encode(parsed), wire); + final codec = _codecWithOutput(union, parsed); + expect(codec.parse('wire'), same(parsed)); + expect(codec.encode(parsed), 'wire'); + }); + + test('round-trips strict object outputs inside a union', () { + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': Ack.object({'name': Ack.string()}).codec( + decode: (map) => {'name': map['name']}, + encode: (value) => value as JsonMap, + output: Ack.anyOf([ + Ack.object({'name': Ack.string()}), + Ack.instance(), + ]), + ), + }, + ); + final wire = {'kind': 'cat', 'name': 'Mittens'}; + final parsed = union.parse(wire)!; + expect(union.encode(parsed), wire); + final codec = _codecWithOutput(union, parsed); + expect(codec.parse('wire'), same(parsed)); + expect(codec.encode(parsed), 'wire'); + expect(codec.safeEncode({...wire, 'extra': true}).isFail, isTrue); + }); + + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': Ack.object({'name': Ack.string()}).codec( + decode: (map) => {'name': map['name']}, + encode: (map) => {'name': map['name']}, + output: Ack.object({ + 'name': Ack.string(), + 'details': Ack.object({'age': Ack.integer()}).optional(), + }), + ), + }, + ); + final invalidCases = <({String name, JsonMap value, String path})>[ + (name: 'missing name', value: {'kind': 'cat'}, path: '#/name'), + ( + name: 'wrong name type', + value: {'kind': 'cat', 'name': 42}, + path: '#/name', + ), + ( + name: 'unrelated extra property', + value: {'kind': 'cat', 'name': 'Mittens', 'extra': true}, + path: '#/extra', + ), + ( + name: 'nested discriminator property', + value: { + 'kind': 'cat', + 'name': 'Mittens', + 'details': {'age': 1, 'kind': 'cat'}, + }, + path: '#/details/kind', + ), + ]; + for (final testCase in invalidCases) { + test('still rejects ${testCase.name}', () { + final codec = _codecWithOutput(union, testCase.value); + for (final result in [ + codec.safeParse('wire'), + codec.safeEncode(testCase.value), + union.safeEncode(testCase.value), + ]) { + expect(result.isFail, isTrue); + expect(_errorPaths(result.getError()), contains(testCase.path)); + } + }); + } + + test('preserves an explicitly authored output discriminator', () { + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': Ack.object({'name': Ack.string()}).codec( + decode: (map) => {'kind': 'dog', 'name': map['name']}, + encode: (map) => {'name': map['name']}, + output: Ack.object({ + 'kind': Ack.literal('dog'), + 'name': Ack.string(), + }), + ), + }, + ); + final runtime = {'kind': 'cat', 'name': 'Mittens'}; + final codec = _codecWithOutput(union, runtime); + expect(codec.safeParse('wire').isFail, isTrue); + expect(codec.safeEncode(runtime).isFail, isTrue); + expect(union.safeEncode(runtime).isFail, isTrue); + }); + + test('membership never reruns branch conversions or refinements', () { + var decodes = 0; + var encodes = 0; + var refinements = 0; + final union = Ack.discriminated( + discriminatorKey: 'kind', + schemas: { + 'cat': Ack.object({'name': Ack.string()}).codec( + decode: (map) { + decodes++; + return {'name': map['name']}; + }, + encode: (map) { + encodes++; + return {'name': map['name']}; + }, + output: Ack.object({'name': Ack.string()}).refine((_) { + refinements++; + return true; + }), + ), + }, + ); + final runtime = {'kind': 'cat', 'name': 'Mittens'}; + final codec = _codecWithOutput(union, runtime); + expect(codec.parse('wire'), same(runtime)); + expect(refinements, 1); + expect(codec.encode(runtime), 'wire'); + expect(refinements, 2); + expect(decodes, 0); + expect(encodes, 0); + }); + }); + group('direct discriminated encoding callback counts', () { test('map branches do not acquire an extra runtime refinement pass', () { var refinements = 0;