From fcab9f976608b1278b593e941efe0a4f5cfc98d6 Mon Sep 17 00:00:00 2001 From: Tyler Harwood Date: Mon, 18 Aug 2025 16:56:43 -0400 Subject: [PATCH 1/4] AVRO-4173: [JS] Fix namespace inheritance for nested types --- lang/js/lib/schemas.js | 17 ++++++++++-- lang/js/test/test_schemas.js | 53 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/lang/js/lib/schemas.js b/lang/js/lib/schemas.js index cbf9e9600af..376b3b3e16e 100644 --- a/lang/js/lib/schemas.js +++ b/lang/js/lib/schemas.js @@ -2029,9 +2029,20 @@ function getOpts(attrs, opts) { throw new Error('invalid type: null (did you mean "null"?)'); } opts = opts || {}; - opts.registry = opts.registry || {}; - opts.namespace = attrs.namespace || opts.namespace; - opts.logicalTypes = opts.logicalTypes || {}; + // Create a new opts object if namespace would change to avoid modifying shared state + if (attrs.namespace && attrs.namespace !== opts.namespace) { + opts = { + registry: opts.registry || {}, + namespace: attrs.namespace, + logicalTypes: opts.logicalTypes || {}, + typeHook: opts.typeHook, + assertLogicalTypes: opts.assertLogicalTypes + }; + } else { + opts.registry = opts.registry || {}; + opts.namespace = attrs.namespace || opts.namespace; + opts.logicalTypes = opts.logicalTypes || {}; + } return opts; } diff --git a/lang/js/test/test_schemas.js b/lang/js/test/test_schemas.js index 0cc0996b894..68339cca316 100644 --- a/lang/js/test/test_schemas.js +++ b/lang/js/test/test_schemas.js @@ -2188,6 +2188,59 @@ describe('types', function () { assert.equal(type._fields[1]._type._name, 'all.Alien'); }); + it('namespace inheritance', function () { + // When nested types don't specify a namespace, they should inherit the parent's + // namespace, but other nested types with explicit namespaces shouldn't corrupt + // the inherited context. + var schema = { + type: 'record', + name: 'Parent', + namespace: 'parent.ns', + fields: [ + { + name: 'child_field', + type: { + type: 'record', + name: 'Child', // No namespace - should inherit 'parent.ns' + fields: [{name: 'value', type: 'int'}] + } + }, + { + name: 'other_field', + type: { + type: 'record', + name: 'Other', + namespace: 'different.ns', // Different namespace + fields: [{name: 'data', type: 'int'}] + } + }, + { + name: 'reference_field', + type: 'Child' // Should resolve to 'parent.ns.Child' + } + ] + }; + + var type = createType(schema); + assert.equal(type.getName(), 'parent.ns.Parent'); + + var fields = type.getFields(); + assert.equal(fields.length, 3); + + // The critical test: reference_field should resolve Child to parent.ns.Child + assert.equal(fields[2].getType().getName(), 'parent.ns.Child'); + + // Verify the schema works for serialization + var testData = { + child_field: { value: 42 }, + other_field: { data: 123 }, + reference_field: { value: 99 } + }; + assert(type.isValid(testData)); + var buf = type.toBuffer(testData); + assert.deepEqual(type.fromBuffer(buf), testData); + }); + it('wrapped primitive', function () { var type = createType({ type: 'record', From 1953f2163a40369758edc64f201dd26ac676c566 Mon Sep 17 00:00:00 2001 From: Tyler Harwood Date: Mon, 18 Aug 2025 16:57:27 -0400 Subject: [PATCH 2/4] AVRO-4173: [JS] Refactor getOpts to prevent shared state --- lang/js/lib/schemas.js | 30 +++++----- lang/js/test/test_schemas.js | 112 ++++++++++++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 16 deletions(-) diff --git a/lang/js/lib/schemas.js b/lang/js/lib/schemas.js index 376b3b3e16e..d99eedb8a10 100644 --- a/lang/js/lib/schemas.js +++ b/lang/js/lib/schemas.js @@ -2029,21 +2029,23 @@ function getOpts(attrs, opts) { throw new Error('invalid type: null (did you mean "null"?)'); } opts = opts || {}; - // Create a new opts object if namespace would change to avoid modifying shared state - if (attrs.namespace && attrs.namespace !== opts.namespace) { - opts = { - registry: opts.registry || {}, - namespace: attrs.namespace, - logicalTypes: opts.logicalTypes || {}, - typeHook: opts.typeHook, - assertLogicalTypes: opts.assertLogicalTypes - }; - } else { - opts.registry = opts.registry || {}; - opts.namespace = attrs.namespace || opts.namespace; - opts.logicalTypes = opts.logicalTypes || {}; + + // Ensure registry and logicalTypes exist and are preserved by reference so + // type definitions can accumulate across recursive calls. + if (!opts.registry) { + opts.registry = {}; } - return opts; + if (!opts.logicalTypes) { + opts.logicalTypes = {}; + } + + return { + registry: opts.registry, // Preserve same object reference + namespace: attrs.namespace || opts.namespace, + logicalTypes: opts.logicalTypes, // Preserve same object reference + typeHook: opts.typeHook, + assertLogicalTypes: opts.assertLogicalTypes + }; } /** diff --git a/lang/js/test/test_schemas.js b/lang/js/test/test_schemas.js index 68339cca316..d8bbcd116b7 100644 --- a/lang/js/test/test_schemas.js +++ b/lang/js/test/test_schemas.js @@ -2227,8 +2227,10 @@ describe('types', function () { var fields = type.getFields(); assert.equal(fields.length, 3); - // The critical test: reference_field should resolve Child to parent.ns.Child - assert.equal(fields[2].getType().getName(), 'parent.ns.Child'); + // Test all field types to demonstrate the fix works for all namespace scenarios + assert.equal(fields[0].getType().getName(), 'parent.ns.Child'); // Inherited namespace + assert.equal(fields[1].getType().getName(), 'different.ns.Other'); // Explicit namespace + assert.equal(fields[2].getType().getName(), 'parent.ns.Child'); // Reference resolution // Verify the schema works for serialization var testData = { @@ -2241,6 +2243,112 @@ describe('types', function () { assert.deepEqual(type.fromBuffer(buf), testData); }); + it('deep namespace inheritance', function () { + // Test namespace inheritance across multiple levels of nesting with various + // namespace changes to ensure the fix works robustly in complex scenarios. + var schema = { + type: 'record', + name: 'Root', + namespace: 'level1', + fields: [ + { + name: 'level2_inherited', + type: { + type: 'record', + name: 'Level2Inherited', // Inherits 'level1' + fields: [ + { + name: 'level3_inherited', + type: { + type: 'record', + name: 'Level3Inherited', // Also inherits 'level1' + fields: [{name: 'deep_value', type: 'int'}] + } + }, + { + name: 'level3_override', + type: { + type: 'record', + name: 'Level3Override', + namespace: 'level3.ns', // Changes namespace context + fields: [{name: 'override_value', type: 'string'}] + } + } + ] + } + }, + { + name: 'level2_different', + type: { + type: 'record', + name: 'Level2Different', + namespace: 'level2.ns', // Different namespace + fields: [ + { + name: 'nested_inherited', + type: { + type: 'record', + name: 'NestedInherited', // Should inherit 'level2.ns' + fields: [{name: 'nested_data', type: 'double'}] + } + } + ] + } + }, + { + name: 'ref_level2_inherited', + type: 'Level2Inherited' // Should resolve to 'level1.Level2Inherited' + }, + { + name: 'ref_level3_inherited', + type: 'Level3Inherited' // Should resolve to 'level1.Level3Inherited' + } + ] + }; + + var type = createType(schema); + assert.equal(type.getName(), 'level1.Root'); + + var fields = type.getFields(); + assert.equal(fields.length, 4); + + // Verify deep inheritance worked correctly + assert.equal(fields[0].getType().getName(), 'level1.Level2Inherited'); + assert.equal(fields[1].getType().getName(), 'level2.ns.Level2Different'); + + // Critical tests: references should resolve to correct namespaces + assert.equal(fields[2].getType().getName(), 'level1.Level2Inherited'); + assert.equal(fields[3].getType().getName(), 'level1.Level3Inherited'); + + // Verify nested types have correct namespaces + var level2Fields = fields[0].getType().getFields(); + assert.equal(level2Fields[0].getType().getName(), 'level1.Level3Inherited'); + assert.equal(level2Fields[1].getType().getName(), 'level3.ns.Level3Override'); + + var level2DiffFields = fields[1].getType().getFields(); + assert.equal(level2DiffFields[0].getType().getName(), 'level2.ns.NestedInherited'); + + // Test serialization works correctly + var testData = { + level2_inherited: { + level3_inherited: { deep_value: 42 }, + level3_override: { override_value: 'test' } + }, + level2_different: { + nested_inherited: { nested_data: 3.14 } + }, + ref_level2_inherited: { + level3_inherited: { deep_value: 99 }, + level3_override: { override_value: 'ref' } + }, + ref_level3_inherited: { deep_value: 123 } + }; + + assert(type.isValid(testData)); + var buf = type.toBuffer(testData); + assert.deepEqual(type.fromBuffer(buf), testData); + }); + it('wrapped primitive', function () { var type = createType({ type: 'record', From 28fb6c0fec5077b7f0a5fc36c983fb7e1c50b18b Mon Sep 17 00:00:00 2001 From: Martin Grigorov Date: Fri, 10 Oct 2025 09:31:44 +0300 Subject: [PATCH 3/4] Simplify registry and logicalTypes initialization --- lang/js/lib/schemas.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/lang/js/lib/schemas.js b/lang/js/lib/schemas.js index d99eedb8a10..c3b6feac9b4 100644 --- a/lang/js/lib/schemas.js +++ b/lang/js/lib/schemas.js @@ -2030,19 +2030,10 @@ function getOpts(attrs, opts) { } opts = opts || {}; - // Ensure registry and logicalTypes exist and are preserved by reference so - // type definitions can accumulate across recursive calls. - if (!opts.registry) { - opts.registry = {}; - } - if (!opts.logicalTypes) { - opts.logicalTypes = {}; - } - return { - registry: opts.registry, // Preserve same object reference + registry: opts.registry || {}, // Preserve same object reference namespace: attrs.namespace || opts.namespace, - logicalTypes: opts.logicalTypes, // Preserve same object reference + logicalTypes: opts.logicalTypes || {}, // Preserve same object reference typeHook: opts.typeHook, assertLogicalTypes: opts.assertLogicalTypes }; From 2db81e4c91e8974b076ae4a84753ca047fed482d Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Fri, 10 Oct 2025 09:42:54 +0300 Subject: [PATCH 4/4] Revert "Simplify registry and logicalTypes initialization" This reverts commit 28fb6c0fec5077b7f0a5fc36c983fb7e1c50b18b. --- lang/js/lib/schemas.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lang/js/lib/schemas.js b/lang/js/lib/schemas.js index c3b6feac9b4..d99eedb8a10 100644 --- a/lang/js/lib/schemas.js +++ b/lang/js/lib/schemas.js @@ -2030,10 +2030,19 @@ function getOpts(attrs, opts) { } opts = opts || {}; + // Ensure registry and logicalTypes exist and are preserved by reference so + // type definitions can accumulate across recursive calls. + if (!opts.registry) { + opts.registry = {}; + } + if (!opts.logicalTypes) { + opts.logicalTypes = {}; + } + return { - registry: opts.registry || {}, // Preserve same object reference + registry: opts.registry, // Preserve same object reference namespace: attrs.namespace || opts.namespace, - logicalTypes: opts.logicalTypes || {}, // Preserve same object reference + logicalTypes: opts.logicalTypes, // Preserve same object reference typeHook: opts.typeHook, assertLogicalTypes: opts.assertLogicalTypes };