Skip to content

Commit 3160e01

Browse files
tyler-blipmartin-g
andauthored
AVRO-4173: [js] Fix namespace inheritance for nested types in schema parsing (#3466)
* AVRO-4173: [JS] Fix namespace inheritance for nested types * AVRO-4173: [JS] Refactor getOpts to prevent shared state * Simplify registry and logicalTypes initialization * Revert "Simplify registry and logicalTypes initialization" This reverts commit 28fb6c0. --------- Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com> Co-authored-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
1 parent 6ac501e commit 3160e01

File tree

2 files changed

+178
-4
lines changed

2 files changed

+178
-4
lines changed

lang/js/lib/schemas.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2029,10 +2029,23 @@ function getOpts(attrs, opts) {
20292029
throw new Error('invalid type: null (did you mean "null"?)');
20302030
}
20312031
opts = opts || {};
2032-
opts.registry = opts.registry || {};
2033-
opts.namespace = attrs.namespace || opts.namespace;
2034-
opts.logicalTypes = opts.logicalTypes || {};
2035-
return opts;
2032+
2033+
// Ensure registry and logicalTypes exist and are preserved by reference so
2034+
// type definitions can accumulate across recursive calls.
2035+
if (!opts.registry) {
2036+
opts.registry = {};
2037+
}
2038+
if (!opts.logicalTypes) {
2039+
opts.logicalTypes = {};
2040+
}
2041+
2042+
return {
2043+
registry: opts.registry, // Preserve same object reference
2044+
namespace: attrs.namespace || opts.namespace,
2045+
logicalTypes: opts.logicalTypes, // Preserve same object reference
2046+
typeHook: opts.typeHook,
2047+
assertLogicalTypes: opts.assertLogicalTypes
2048+
};
20362049
}
20372050

20382051
/**

lang/js/test/test_schemas.js

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2188,6 +2188,167 @@ describe('types', function () {
21882188
assert.equal(type._fields[1]._type._name, 'all.Alien');
21892189
});
21902190

2191+
it('namespace inheritance', function () {
2192+
// When nested types don't specify a namespace, they should inherit the parent's
2193+
// namespace, but other nested types with explicit namespaces shouldn't corrupt
2194+
// the inherited context.
2195+
var schema = {
2196+
type: 'record',
2197+
name: 'Parent',
2198+
namespace: 'parent.ns',
2199+
fields: [
2200+
{
2201+
name: 'child_field',
2202+
type: {
2203+
type: 'record',
2204+
name: 'Child', // No namespace - should inherit 'parent.ns'
2205+
fields: [{name: 'value', type: 'int'}]
2206+
}
2207+
},
2208+
{
2209+
name: 'other_field',
2210+
type: {
2211+
type: 'record',
2212+
name: 'Other',
2213+
namespace: 'different.ns', // Different namespace
2214+
fields: [{name: 'data', type: 'int'}]
2215+
}
2216+
},
2217+
{
2218+
name: 'reference_field',
2219+
type: 'Child' // Should resolve to 'parent.ns.Child'
2220+
}
2221+
]
2222+
};
2223+
2224+
var type = createType(schema);
2225+
assert.equal(type.getName(), 'parent.ns.Parent');
2226+
2227+
var fields = type.getFields();
2228+
assert.equal(fields.length, 3);
2229+
2230+
// Test all field types to demonstrate the fix works for all namespace scenarios
2231+
assert.equal(fields[0].getType().getName(), 'parent.ns.Child'); // Inherited namespace
2232+
assert.equal(fields[1].getType().getName(), 'different.ns.Other'); // Explicit namespace
2233+
assert.equal(fields[2].getType().getName(), 'parent.ns.Child'); // Reference resolution
2234+
2235+
// Verify the schema works for serialization
2236+
var testData = {
2237+
child_field: { value: 42 },
2238+
other_field: { data: 123 },
2239+
reference_field: { value: 99 }
2240+
};
2241+
assert(type.isValid(testData));
2242+
var buf = type.toBuffer(testData);
2243+
assert.deepEqual(type.fromBuffer(buf), testData);
2244+
});
2245+
2246+
it('deep namespace inheritance', function () {
2247+
// Test namespace inheritance across multiple levels of nesting with various
2248+
// namespace changes to ensure the fix works robustly in complex scenarios.
2249+
var schema = {
2250+
type: 'record',
2251+
name: 'Root',
2252+
namespace: 'level1',
2253+
fields: [
2254+
{
2255+
name: 'level2_inherited',
2256+
type: {
2257+
type: 'record',
2258+
name: 'Level2Inherited', // Inherits 'level1'
2259+
fields: [
2260+
{
2261+
name: 'level3_inherited',
2262+
type: {
2263+
type: 'record',
2264+
name: 'Level3Inherited', // Also inherits 'level1'
2265+
fields: [{name: 'deep_value', type: 'int'}]
2266+
}
2267+
},
2268+
{
2269+
name: 'level3_override',
2270+
type: {
2271+
type: 'record',
2272+
name: 'Level3Override',
2273+
namespace: 'level3.ns', // Changes namespace context
2274+
fields: [{name: 'override_value', type: 'string'}]
2275+
}
2276+
}
2277+
]
2278+
}
2279+
},
2280+
{
2281+
name: 'level2_different',
2282+
type: {
2283+
type: 'record',
2284+
name: 'Level2Different',
2285+
namespace: 'level2.ns', // Different namespace
2286+
fields: [
2287+
{
2288+
name: 'nested_inherited',
2289+
type: {
2290+
type: 'record',
2291+
name: 'NestedInherited', // Should inherit 'level2.ns'
2292+
fields: [{name: 'nested_data', type: 'double'}]
2293+
}
2294+
}
2295+
]
2296+
}
2297+
},
2298+
{
2299+
name: 'ref_level2_inherited',
2300+
type: 'Level2Inherited' // Should resolve to 'level1.Level2Inherited'
2301+
},
2302+
{
2303+
name: 'ref_level3_inherited',
2304+
type: 'Level3Inherited' // Should resolve to 'level1.Level3Inherited'
2305+
}
2306+
]
2307+
};
2308+
2309+
var type = createType(schema);
2310+
assert.equal(type.getName(), 'level1.Root');
2311+
2312+
var fields = type.getFields();
2313+
assert.equal(fields.length, 4);
2314+
2315+
// Verify deep inheritance worked correctly
2316+
assert.equal(fields[0].getType().getName(), 'level1.Level2Inherited');
2317+
assert.equal(fields[1].getType().getName(), 'level2.ns.Level2Different');
2318+
2319+
// Critical tests: references should resolve to correct namespaces
2320+
assert.equal(fields[2].getType().getName(), 'level1.Level2Inherited');
2321+
assert.equal(fields[3].getType().getName(), 'level1.Level3Inherited');
2322+
2323+
// Verify nested types have correct namespaces
2324+
var level2Fields = fields[0].getType().getFields();
2325+
assert.equal(level2Fields[0].getType().getName(), 'level1.Level3Inherited');
2326+
assert.equal(level2Fields[1].getType().getName(), 'level3.ns.Level3Override');
2327+
2328+
var level2DiffFields = fields[1].getType().getFields();
2329+
assert.equal(level2DiffFields[0].getType().getName(), 'level2.ns.NestedInherited');
2330+
2331+
// Test serialization works correctly
2332+
var testData = {
2333+
level2_inherited: {
2334+
level3_inherited: { deep_value: 42 },
2335+
level3_override: { override_value: 'test' }
2336+
},
2337+
level2_different: {
2338+
nested_inherited: { nested_data: 3.14 }
2339+
},
2340+
ref_level2_inherited: {
2341+
level3_inherited: { deep_value: 99 },
2342+
level3_override: { override_value: 'ref' }
2343+
},
2344+
ref_level3_inherited: { deep_value: 123 }
2345+
};
2346+
2347+
assert(type.isValid(testData));
2348+
var buf = type.toBuffer(testData);
2349+
assert.deepEqual(type.fromBuffer(buf), testData);
2350+
});
2351+
21912352
it('wrapped primitive', function () {
21922353
var type = createType({
21932354
type: 'record',

0 commit comments

Comments
 (0)