From 685c66ba61cb2bb9742a7ec373a056167ee7defe Mon Sep 17 00:00:00 2001 From: Shaw Young Date: Thu, 28 Nov 2024 16:17:23 +0000 Subject: [PATCH 1/4] AVRO-4094: [C#] Updating mapped namespaces referenced in types --- .../csharp/src/apache/main/CodeGen/CodeGen.cs | 105 ++++++++++++++++++ .../apache/test/AvroGen/AvroGenSchemaTests.cs | 71 ++++++++++++ 2 files changed, 176 insertions(+) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index 73b95852d7b..75614711784 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -25,7 +25,10 @@ using System.Reflection; using System.Text; using System.Text.RegularExpressions; +using Avro.IO.Parsing; using Microsoft.CSharp; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; namespace Avro { @@ -113,6 +116,8 @@ public virtual void AddProtocol(string protocolText, IEnumerable + /// Replace namespaces in a parsed JSON schema object for all "type" fields. + /// + /// The JSON schema as a string. + /// The mapping of old namespaces to new namespaces. + /// The updated JSON schema as a string. + private static string ReplaceMappedNamespacesInSchemaTypes(string schemaJson, IEnumerable> namespaceMapping) + { + if (string.IsNullOrWhiteSpace(schemaJson) || namespaceMapping == null) + return schemaJson; + + var schemaToken = JToken.Parse(schemaJson); + + UpdateNamespacesInJToken(schemaToken, namespaceMapping); + + return schemaToken.ToString(Formatting.Indented); + } + + /// + /// Recursively navigates and updates "type" fields in a JToken. + /// + /// The current JToken to process. + /// The mapping of old namespaces to new namespaces. + private static void UpdateNamespacesInJToken(JToken token, IEnumerable> namespaceMapping) + { + if (token is JObject obj) + { + if (obj.ContainsKey("type")) + { + var typeToken = obj["type"]; + if (typeToken is JValue) // Single type + { + string type = typeToken.ToString(); + obj["type"] = ReplaceNamespace(type, namespaceMapping); + } + else if (typeToken is JArray typeArray) // Array of types + { + for (int i = 0; i < typeArray.Count; i++) + { + var arrayItem = typeArray[i]; + if (arrayItem is JValue) // Simple type + { + string type = arrayItem.ToString(); + typeArray[i] = ReplaceNamespace(type, namespaceMapping); + } + else if (arrayItem is JObject nestedObj) // Nested object + { + UpdateNamespacesInJToken(nestedObj, namespaceMapping); + } + } + } + else if (typeToken is JObject nestedTypeObj) // Complex type + { + UpdateNamespacesInJToken(nestedTypeObj, namespaceMapping); + } + } + + // Recurse into all properties of the object + foreach (var property in obj.Properties()) + { + UpdateNamespacesInJToken(property.Value, namespaceMapping); + } + } + else if (token is JArray array) + { + // Recurse into all elements of the array + foreach (var element in array) + { + UpdateNamespacesInJToken(element, namespaceMapping); + } + } } + + /// + /// Replace a namespace in a string based on the provided mapping. + /// + /// The original namespace string. + /// The mapping of old namespaces to new namespaces. + /// The updated namespace string. + private static string ReplaceNamespace(string originalNamespace, IEnumerable> namespaceMapping) + { + foreach (var mapping in namespaceMapping) + { + if (originalNamespace == mapping.Key) + { + return mapping.Value; + } + else if (originalNamespace.StartsWith($"{mapping.Key}.")) + { + return $"{mapping.Value}.{originalNamespace.Substring(mapping.Key.Length + 1)}"; + } + } + return originalNamespace; + } + +} } diff --git a/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs b/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs index 807acbda92a..34d610869af 100644 --- a/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs +++ b/lang/csharp/src/apache/test/AvroGen/AvroGenSchemaTests.cs @@ -309,6 +309,46 @@ class AvroGenSchemaTests ] }"; + private const string _fullyQualifiedTypeReferences = @" +[ + { + ""namespace"": ""org.apache.avro.codegentest.testdata.common"", + ""type"": ""enum"", + ""name"": ""Planet"", + ""doc"" : ""Test mapping of types in other namespaces post-map"", + ""symbols"": [ + ""Mercury"", + ""Venus"", + ""Earth"", + ""Mars"", + ""Jupiter"", + ""Saturn"", + ""Neptune"", + ""Uranus"" + ], + }, + { + ""namespace"": ""org.apache.avro.codegentest.testdata.users"", + ""type"": ""record"", + ""name"": ""User"", + ""doc"" : ""Test mapping of types in other namespaces post-map"", + ""fields"": [ + { + ""name"": ""homePlanet"", + ""type"": ""org.apache.avro.codegentest.testdata.common.Planet"" + }, + { + ""name"": ""favouritePlanet"", + ""type"": [ + ""null"", + ""org.apache.avro.codegentest.testdata.common.Planet"" + ], + ""default"": null + }] + }, + +]"; + private Assembly TestSchema( string schema, IEnumerable typeNamesToCheck = null, @@ -606,6 +646,37 @@ public void GenerateSchemaWithNamespaceMapping( AvroGenHelper.TestSchema(schema, typeNamesToCheck, new Dictionary { { namespaceMappingFrom, namespaceMappingTo } }, generatedFilesToCheck); } + [TestCase(_fullyQualifiedTypeReferences, + new string[] + { + "org.apache.avro.codegentest.testdata.common:Test.Common", + "org.apache.avro.codegentest.testdata.users:Test.Users" + }, + new string[] + { + "Test.Common.Planet", + "Test.Users.User", + }, + new string[] + { + "Test/Common/Planet.cs", + "Test/Users/User.cs" + })] + public void GenerateSchemaWithMultipleNamespaceMapping( + string schema, + IEnumerable namespaceMappings, + IEnumerable typeNamesToCheck, + IEnumerable generatedFilesToCheck) + { + var namespaceMappingsDict = new Dictionary(); + + foreach(var mapping in namespaceMappings) + { + namespaceMappingsDict.Add(mapping.Split(':')[0], mapping.Split(':')[1]); + } + AvroGenHelper.TestSchema(schema, typeNamesToCheck, namespaceMappingsDict, generatedFilesToCheck); + } + [TestCase(_logicalTypesWithCustomConversion, typeof(AvroTypeException))] [TestCase(_customConversionWithLogicalTypes, typeof(SchemaParseException))] public void NotSupportedSchema(string schema, Type expectedException) From 1fee52f2af057e723ed820d7dbdd5b64a738cfdf Mon Sep 17 00:00:00 2001 From: Shaw Young Date: Thu, 30 Jan 2025 21:15:29 +0000 Subject: [PATCH 2/4] Update lang/csharp/src/apache/main/CodeGen/CodeGen.cs Co-authored-by: Paul Podgorsek --- lang/csharp/src/apache/main/CodeGen/CodeGen.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index 75614711784..42df5bc394c 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -1285,7 +1285,9 @@ private static string ReplaceMappedNamespacesInSchema(string input, IEnumerable< private static string ReplaceMappedNamespacesInSchemaTypes(string schemaJson, IEnumerable> namespaceMapping) { if (string.IsNullOrWhiteSpace(schemaJson) || namespaceMapping == null) + { return schemaJson; + } var schemaToken = JToken.Parse(schemaJson); From 2f11cd6f84a5c8a6f5922a53bafc312886ac9105 Mon Sep 17 00:00:00 2001 From: Shaw Young Date: Thu, 30 Jan 2025 21:15:35 +0000 Subject: [PATCH 3/4] Update lang/csharp/src/apache/main/CodeGen/CodeGen.cs Co-authored-by: Paul Podgorsek --- lang/csharp/src/apache/main/CodeGen/CodeGen.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index 42df5bc394c..a2d912a686b 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -1273,9 +1273,6 @@ private static string ReplaceMappedNamespacesInSchema(string input, IEnumerable< return $@"""namespace""{m.Groups[1].Value}:{m.Groups[2].Value}""{ns}"""; }); } - - - /// /// Replace namespaces in a parsed JSON schema object for all "type" fields. /// From a78b3228e27b4271dd330d2ce8643808b8523ad3 Mon Sep 17 00:00:00 2001 From: Shaw Young Date: Thu, 6 Mar 2025 22:28:41 +0000 Subject: [PATCH 4/4] Update CodeGen.cs Changing string comparison in ReplaceNamespace to use Ordinal string comparison Adding blacklist for unqualified props which we shouldn't be looking to do any namespace replaces on --- .../csharp/src/apache/main/CodeGen/CodeGen.cs | 154 +++++++++--------- 1 file changed, 81 insertions(+), 73 deletions(-) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index a2d912a686b..a62c4947f52 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -78,6 +78,12 @@ public class CodeGen /// protected Dictionary NamespaceLookup { get; private set; } + /// + /// Set of schema property names which could contain a namespace reference. + /// + private static readonly HashSet UnqualifiedProps = new HashSet() { "doc", "symbols", "default", "size" }; + + /// /// Initializes a new instance of the class. /// @@ -1273,102 +1279,104 @@ private static string ReplaceMappedNamespacesInSchema(string input, IEnumerable< return $@"""namespace""{m.Groups[1].Value}:{m.Groups[2].Value}""{ns}"""; }); } - /// - /// Replace namespaces in a parsed JSON schema object for all "type" fields. - /// - /// The JSON schema as a string. - /// The mapping of old namespaces to new namespaces. - /// The updated JSON schema as a string. - private static string ReplaceMappedNamespacesInSchemaTypes(string schemaJson, IEnumerable> namespaceMapping) - { - if (string.IsNullOrWhiteSpace(schemaJson) || namespaceMapping == null) + /// + /// Replace namespaces in a parsed JSON schema object for all "type" fields. + /// + /// The JSON schema as a string. + /// The mapping of old namespaces to new namespaces. + /// The updated JSON schema as a string. + private static string ReplaceMappedNamespacesInSchemaTypes(string schemaJson, IEnumerable> namespaceMapping) { - return schemaJson; - } + if (string.IsNullOrWhiteSpace(schemaJson) || namespaceMapping == null) + { + return schemaJson; + } - var schemaToken = JToken.Parse(schemaJson); + var schemaToken = JToken.Parse(schemaJson); - UpdateNamespacesInJToken(schemaToken, namespaceMapping); + UpdateNamespacesInJToken(schemaToken, namespaceMapping); - return schemaToken.ToString(Formatting.Indented); - } + return schemaToken.ToString(Formatting.Indented); + } - /// - /// Recursively navigates and updates "type" fields in a JToken. - /// - /// The current JToken to process. - /// The mapping of old namespaces to new namespaces. - private static void UpdateNamespacesInJToken(JToken token, IEnumerable> namespaceMapping) - { - if (token is JObject obj) + /// + /// Recursively navigates and updates "type" fields in a JToken. + /// + /// The current JToken to process. + /// The mapping of old namespaces to new namespaces. + private static void UpdateNamespacesInJToken(JToken token, IEnumerable> namespaceMapping) { - if (obj.ContainsKey("type")) + if (token is JObject obj) { - var typeToken = obj["type"]; - if (typeToken is JValue) // Single type + if (obj.ContainsKey("type")) { - string type = typeToken.ToString(); - obj["type"] = ReplaceNamespace(type, namespaceMapping); - } - else if (typeToken is JArray typeArray) // Array of types - { - for (int i = 0; i < typeArray.Count; i++) + var typeToken = obj["type"]; + if (typeToken is JValue) // Single type { - var arrayItem = typeArray[i]; - if (arrayItem is JValue) // Simple type - { - string type = arrayItem.ToString(); - typeArray[i] = ReplaceNamespace(type, namespaceMapping); - } - else if (arrayItem is JObject nestedObj) // Nested object + string type = typeToken.ToString(); + obj["type"] = ReplaceNamespace(type, namespaceMapping); + } + else if (typeToken is JArray typeArray) // Array of types + { + for (int i = 0; i < typeArray.Count; i++) { - UpdateNamespacesInJToken(nestedObj, namespaceMapping); + var arrayItem = typeArray[i]; + if (arrayItem is JValue) // Simple type + { + string type = arrayItem.ToString(); + typeArray[i] = ReplaceNamespace(type, namespaceMapping); + } + else if (arrayItem is JObject nestedObj) // Nested object + { + UpdateNamespacesInJToken(nestedObj, namespaceMapping); + } } } + else if (typeToken is JObject nestedTypeObj) // Complex type + { + UpdateNamespacesInJToken(nestedTypeObj, namespaceMapping); + } } - else if (typeToken is JObject nestedTypeObj) // Complex type + + // Recurse into all properties of the object + foreach (var property in obj.Properties()) { - UpdateNamespacesInJToken(nestedTypeObj, namespaceMapping); + if (UnqualifiedProps.Contains(property.Name)) + continue; + + UpdateNamespacesInJToken(property.Value, namespaceMapping); } } - - // Recurse into all properties of the object - foreach (var property in obj.Properties()) + else if (token is JArray array) { - UpdateNamespacesInJToken(property.Value, namespaceMapping); - } - } - else if (token is JArray array) - { - // Recurse into all elements of the array - foreach (var element in array) - { - UpdateNamespacesInJToken(element, namespaceMapping); + // Recurse into all elements of the array + foreach (var element in array) + { + UpdateNamespacesInJToken(element, namespaceMapping); + } } } - } - /// - /// Replace a namespace in a string based on the provided mapping. - /// - /// The original namespace string. - /// The mapping of old namespaces to new namespaces. - /// The updated namespace string. - private static string ReplaceNamespace(string originalNamespace, IEnumerable> namespaceMapping) - { - foreach (var mapping in namespaceMapping) + /// + /// Replace a namespace in a string based on the provided mapping. + /// + /// The original namespace string. + /// The mapping of old namespaces to new namespaces. + /// The updated namespace string. + private static string ReplaceNamespace(string originalNamespace, IEnumerable> namespaceMapping) { - if (originalNamespace == mapping.Key) - { - return mapping.Value; - } - else if (originalNamespace.StartsWith($"{mapping.Key}.")) + foreach (var mapping in namespaceMapping) { - return $"{mapping.Value}.{originalNamespace.Substring(mapping.Key.Length + 1)}"; + if (originalNamespace == mapping.Key) + { + return mapping.Value; + } + else if (originalNamespace.StartsWith($"{mapping.Key}.", StringComparison.Ordinal)) + { + return $"{mapping.Value}.{originalNamespace.Substring(mapping.Key.Length + 1)}"; + } } + return originalNamespace; } - return originalNamespace; } - -} }