Skip to content

AVRO-4187: [csharp] fix bug causing csharp Avrogen generated "SCHEMA" property to contain csharp instead of avro namespaces when using --namespace flag #3516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DavidNeukamXXXLutz wants to merge 1 commit into from
Closed

AVRO-4187: [csharp] fix bug causing csharp Avrogen generated "SCHEMA" property to contain csharp instead of avro namespaces when using --namespace flag #3516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 41 additions & 5 deletions lang/csharp/src/apache/main/CodeGen/CodeGen.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@
/// </value>
protected Dictionary<string, CodeNamespace> NamespaceLookup { get; private set; }

/// <summary>
/// Gets the collection of namespace mappings used to replace namespaces in the schema text when adding schemas or protocols.
/// Used to reverse the namespace replacement later to keep the original schema text intact within the "Schema" property.
/// </summary>
protected IDictionary<string, string> ReplacedNamespaceMappings { get; private set; }

/// <summary>
/// Initializes a new instance of the <see cref="CodeGen"/> class.
/// </summary>
Expand All @@ -83,6 +89,7 @@
Schemas = new List<Schema>();
Protocols = new List<Protocol>();
NamespaceLookup = new Dictionary<string, CodeNamespace>(StringComparer.Ordinal);
ReplacedNamespaceMappings = new Dictionary<string, string>();
}

/// <summary>
Expand Down Expand Up @@ -134,9 +141,26 @@
public virtual void AddSchema(string schemaText, IEnumerable<KeyValuePair<string, string>> namespaceMapping = null)
{
// Map namespaces
schemaText = ReplaceMappedNamespacesInSchema(schemaText, namespaceMapping);
Schema schema = Schema.Parse(schemaText);
Schemas.Add(schema);
var namespaceAdjustedSchemaText = ReplaceMappedNamespacesInSchema(schemaText, namespaceMapping);
Schema namspaceAdjustedSchema = Schema.Parse(namespaceAdjustedSchemaText);
Schemas.Add(namspaceAdjustedSchema);

AddReplacedNamespaceMappings(namespaceMapping);
}

/// <summary>
/// Adds namespace mappings in the schema text to ReplacedNamespaceMappings, so that they can be reversed later to keep the original schema text intact within the "Schema" property.
/// </summary>
/// <param name="namespaceMapping"></param>
protected void AddReplacedNamespaceMappings(IEnumerable<KeyValuePair<string, string>> namespaceMapping)
{
if(namespaceMapping == null)
return;

foreach (var item in namespaceMapping.Where(item => !ReplacedNamespaceMappings.ContainsKey(item.Key)))
{
ReplacedNamespaceMappings.Add(item.Key, item.Value);
}
}

/// <summary>
Expand Down Expand Up @@ -206,22 +230,22 @@
/// <exception cref="CodeGenException">Names in schema should only be of type NamedSchema, type found " + sn.Value.Tag.</exception>
protected virtual void ProcessSchemas()
{
foreach (Schema schema in Schemas)
foreach (var schema in Schemas)
{
SchemaNames names = GenerateNames(schema);
foreach (KeyValuePair<SchemaName, NamedSchema> sn in names)
{
switch (sn.Value.Tag)
{
case Schema.Type.Enumeration: processEnum(sn.Value); break;
case Schema.Type.Fixed: processFixed(sn.Value); break;
case Schema.Type.Record: processRecord(sn.Value); break;
case Schema.Type.Error: processRecord(sn.Value); break;
default:
throw new CodeGenException("Names in schema should only be of type NamedSchema, type found " + sn.Value.Tag);
}
}
}

Check notice

Code scanning / CodeQL

Missed opportunity to use Select Note

This foreach loop immediately
maps its iteration variable to another variable
Loading
- consider mapping the sequence explicitly using '.Select(...)'.
}

/// <summary>
Expand Down Expand Up @@ -1073,8 +1097,19 @@
var codeField = new CodeMemberField(ctrfield, schemaFname);
codeField.Attributes = MemberAttributes.Public | MemberAttributes.Static;


//restore Avro Namespaces
var inverseDict = new Dictionary<string, string>();

foreach (var kvp in ReplacedNamespaceMappings)
{
inverseDict[kvp.Value] = kvp.Key;
}

var namespaceRestoredSchemaText = ReplaceMappedNamespacesInSchema(schema.ToString(), inverseDict);

// create function call Schema.Parse(json)
var cpe = new CodePrimitiveExpression(schema.ToString());
var cpe = new CodePrimitiveExpression(namespaceRestoredSchemaText);
var cmie = new CodeMethodInvokeExpression(
new CodeMethodReferenceExpression(new CodeTypeReferenceExpression(ctrfield), "Parse"),
new CodeExpression[] { cpe });
Expand Down Expand Up @@ -1266,5 +1301,6 @@
return $@"""namespace""{m.Groups[1].Value}:{m.Groups[2].Value}""{ns}""";
});
}

}
}
18 changes: 15 additions & 3 deletions lang/csharp/src/apache/test/AvroGen/AvroGenHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ public static Assembly CompileCSharpFilesAndCheckTypes(
string outputDir,
string assemblyName,
IEnumerable<string> typeNamesToCheck = null,
IEnumerable<string> generatedFilesToCheck = null)
IEnumerable<string> generatedFilesToCheck = null,
IEnumerable<KeyValuePair<string, string>> namespaceMapping = null)
{
// Check if all generated files exist
if (generatedFilesToCheck != null)
Expand Down Expand Up @@ -230,6 +231,17 @@ public static Assembly CompileCSharpFilesAndCheckTypes(
{
// Read record's schema object
Assert.That(record.Schema, Is.Not.Null);

if(namespaceMapping is not null)
{
var schema = record.Schema.ToString();
foreach (var mapping in namespaceMapping.Where(mapping => mapping.Key != mapping.Value))
{
Assert.That(schema, Does.Contain($"\"namespace\":\"{mapping.Key}\""));
Assert.That(schema, Does.Not.Contain($"\"namespace\":\"{mapping.Value}\""));
}
}

// Force exception by reading/writing invalid field
Assert.Throws<AvroRuntimeException>(() => record.Get(-1));
Assert.Throws<AvroRuntimeException>(() => record.Put(-1, null));
Expand Down Expand Up @@ -260,7 +272,7 @@ public static Assembly TestSchema(
// Generate from schema file
Assert.That(AvroGenTool.GenSchema(schemaFileName, outputDir, namespaceMapping ?? new Dictionary<string, string>(), skipDirectories), Is.EqualTo(0));

return CompileCSharpFilesAndCheckTypes(outputDir, uniqueId, typeNamesToCheck, generatedFilesToCheck);
return CompileCSharpFilesAndCheckTypes(outputDir, uniqueId, typeNamesToCheck, generatedFilesToCheck, namespaceMapping ?? new Dictionary<string, string>());
}
finally
{
Expand All @@ -286,7 +298,7 @@ public static Assembly TestProtocol(
// Generate from protocol file
Assert.That(AvroGenTool.GenProtocol(schemaFileName, outputDir, namespaceMapping ?? new Dictionary<string, string>()), Is.EqualTo(0));

return CompileCSharpFilesAndCheckTypes(outputDir, uniqueId, typeNamesToCheck, generatedFilesToCheck);
return CompileCSharpFilesAndCheckTypes(outputDir, uniqueId, typeNamesToCheck, generatedFilesToCheck, namespaceMapping ?? new Dictionary<string, string>());
}
finally
{
Expand Down