Skip to content

Commit

Permalink
Adds an option to control utf8Encoding of strings. (#567)
Browse files Browse the repository at this point in the history
* Adding config option to disable utf8 encoding

* Cleanup

* Cleanup of comments

* Cleanup of stale comments

* Resolved comments. PTAL.

* Fixing build error.

* Throws an exception if utf8Encoding is set to true without setting methodStringRepresentation and stringRepresentation to CharSequence.
  • Loading branch information
jairajsolanke authored Jan 29, 2025
1 parent 997baf1 commit 17a828a
Show file tree
Hide file tree
Showing 13 changed files with 458 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public static void main(String[] args) throws Exception {

long optionParseStart = System.currentTimeMillis();
OptionParser parser = new OptionParser();

OptionSpec<String> inputOpt = parser.accepts("input", "Schema or directory of schemas to compile [REQUIRED]")
.withRequiredArg().required()
.describedAs("file");
Expand Down Expand Up @@ -114,6 +115,13 @@ public static void main(String[] args) throws Exception {
.defaultsTo("false")
.describedAs("true/false");

OptionSpec<String> enableUtf8Encoding = parser.accepts("enableUtf8Encoding",
"enable encoding strings to their utf8 values throughout generated code. This overrides enableUtf8EncodingInPutByIndex."
+ "Requires --stringRepresentation and --methodStringRepresentation to be CharSequence")
.withOptionalArg()
.defaultsTo("true")
.describedAs("true/false");

//allow plugins to add CLI options
for (BuilderPlugin plugin : plugins) {
plugin.customizeCLI(parser);
Expand Down Expand Up @@ -238,6 +246,21 @@ public static void main(String[] args) throws Exception {
skipCodegenIfSchemaOnClasspath = Boolean.TRUE.equals(Boolean.parseBoolean(value));
}

boolean handleUtf8Encoding = true;
if (options.has(enableUtf8Encoding)) {
String value = options.valueOf(enableUtf8Encoding);
handleUtf8Encoding = Boolean.TRUE.equals(Boolean.parseBoolean(value));
if (handleUtf8Encoding) {
if (methodStringRepresentation.equals(StringRepresentation.CharSequence) && stringRepresentation.equals(
StringRepresentation.CharSequence)) {
handleUtf8EncodingInPutByIndex = handleUtf8Encoding;
} else {
throw new IllegalArgumentException("--methodStringRepresentation and --stringRepresentation both need to be"
+ " CharSequence for enableUtf8Encoding to be supported.");
}
}
}

//allow plugins to parse and validate their own added options
for (BuilderPlugin plugin : plugins) {
plugin.parseAndValidateOptions(options);
Expand All @@ -258,7 +281,8 @@ public static void main(String[] args) throws Exception {
minAvroVer,
handleAvro702,
handleUtf8EncodingInPutByIndex,
skipCodegenIfSchemaOnClasspath
skipCodegenIfSchemaOnClasspath,
handleUtf8Encoding
);

opConfig.validateParameters();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class CodeGenOpConfig {
boolean avro702Handling;
boolean utf8EncodingPutByIndex;
boolean skipCodegenIfSchemaOnClasspath;
boolean enableUtf8Encoding;

@Deprecated
public CodeGenOpConfig(
Expand Down Expand Up @@ -77,6 +78,7 @@ public CodeGenOpConfig(
this.minAvroVersion = minAvroVersion;
this.avro702Handling = avro702Handling;
this.utf8EncodingPutByIndex = true;
this.enableUtf8Encoding = true;
}

@Deprecated
Expand Down Expand Up @@ -107,6 +109,7 @@ public CodeGenOpConfig(
this.minAvroVersion = minAvroVersion;
this.avro702Handling = avro702Handling;
this.utf8EncodingPutByIndex = true;
this.enableUtf8Encoding = true;
}

@Deprecated
Expand Down Expand Up @@ -136,6 +139,7 @@ public CodeGenOpConfig(List<File> inputRoots,
this.minAvroVersion = minAvroVersion;
this.avro702Handling = avro702Handling;
this.utf8EncodingPutByIndex = handleUtf8EncodingInPutByIndex;
this.enableUtf8Encoding = true;
}

@Deprecated
Expand Down Expand Up @@ -167,8 +171,10 @@ public CodeGenOpConfig(List<File> inputRoots,
this.avro702Handling = avro702Handling;
this.utf8EncodingPutByIndex = handleUtf8EncodingInPutByIndex;
this.skipCodegenIfSchemaOnClasspath = skipCodegenIfSchemaOnClasspath;
this.enableUtf8Encoding = true;
}

@Deprecated
public CodeGenOpConfig(List<File> inputRoots,
List<File> nonImportableSourceRoots,
boolean includeClasspath,
Expand Down Expand Up @@ -199,6 +205,41 @@ public CodeGenOpConfig(List<File> inputRoots,
this.avro702Handling = avro702Handling;
this.utf8EncodingPutByIndex = handleUtf8EncodingInPutByIndex;
this.skipCodegenIfSchemaOnClasspath = skipCodegenIfSchemaOnClasspath;
this.enableUtf8Encoding = true;
}

public CodeGenOpConfig(List<File> inputRoots,
List<File> nonImportableSourceRoots,
boolean includeClasspath,
File outputSpecificRecordClassesRoot,
File outputExpandedSchemasRoot,
List<File> resolverPath,
CodeGenerator generatorType,
DuplicateSchemaBehaviour dupBehaviour,
List<String> duplicateSchemasToIgnore,
StringRepresentation stringRepresentation,
StringRepresentation methodStringRepresentation,
AvroVersion minAvroVersion,
boolean avro702Handling,
boolean handleUtf8EncodingInPutByIndex,
boolean skipCodegenIfSchemaOnClasspath,
boolean handleUtf8Encoding) {
this.inputRoots = inputRoots;
this.nonImportableSourceRoots = nonImportableSourceRoots;
this.includeClasspath = includeClasspath;
this.outputSpecificRecordClassesRoot = outputSpecificRecordClassesRoot;
this.outputExpandedSchemasRoot = outputExpandedSchemasRoot;
this.resolverPath = resolverPath;
this.generatorType = generatorType;
this.dupBehaviour = dupBehaviour;
this.duplicateSchemasToIgnore = duplicateSchemasToIgnore;
this.stringRepresentation = stringRepresentation;
this.methodStringRepresentation = methodStringRepresentation;
this.minAvroVersion = minAvroVersion;
this.avro702Handling = avro702Handling;
this.utf8EncodingPutByIndex = handleUtf8EncodingInPutByIndex;
this.skipCodegenIfSchemaOnClasspath = skipCodegenIfSchemaOnClasspath;
this.enableUtf8Encoding = handleUtf8Encoding;
}

/**
Expand Down Expand Up @@ -312,6 +353,10 @@ public boolean shouldSkipCodegenIfSchemaOnClasspath() {
return skipCodegenIfSchemaOnClasspath;
}

public boolean isUtf8EncodingEnabled() {
return enableUtf8Encoding;
}

private void validateInput(Collection<File> files, String desc) {
for (File f : files) {
if (!f.exists()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ private void generateCode(OperationContext opContext) {
SpecificRecordGenerationConfig.getBroadCompatibilitySpecificRecordGenerationConfig(
AvroJavaStringRepresentation.fromJson(config.getStringRepresentation().toString()),
AvroJavaStringRepresentation.fromJson(config.getMethodStringRepresentation().toString()),
config.getMinAvroVersion(), config.isUtf8EncodingPutByIndexEnabled());
config.getMinAvroVersion(), config.isUtf8EncodingPutByIndexEnabled(), config.isUtf8EncodingEnabled());
final SpecificRecordClassGenerator generator = new SpecificRecordClassGenerator();
List<JavaFile> generatedClasses = allNamedSchemas.stream().collect(StreamUtil.toParallelStream(namedSchema -> {
try {
Expand Down
68 changes: 68 additions & 0 deletions avro-builder/tests/codegen-no-utf8-encoding/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright 2022 LinkedIn Corp.
* Licensed under the BSD 2-Clause License (the "License").
* See License in the project root for license information.
*/

plugins {
id "java-library"
}

configurations {
codegen
}

sourceSets {
main {
java {
srcDir 'src/main/java'
srcDir "$buildDir/generated/sources/avro/java/main"
}
resources {
srcDirs = [
"src/main/avro"
]
}
}
}

dependencies {
codegen project(":avro-builder:builder")
}

task runOwnCodegen {
description = 'generate specific classes using own codegen utility'

dependsOn configurations.codegen

doLast {
javaexec {
classpath configurations.codegen
main = 'com.linkedin.avroutil1.builder.SchemaBuilder'
args = [
"--input", "$projectDir/src/main/avro",
"--output", "$buildDir/generated/sources/avro/java/main",
"--generator", "AVRO_UTIL",
"--stringRepresentation", "CharSequence",
"--methodStringRepresentation", "CharSequence",
"--enableUtf8Encoding", "false"
]
}
}
}

compileJava.dependsOn runOwnCodegen

dependencies {
codegen project(":avro-builder:builder")

implementation("org.apache.avro:avro:1.4.1") {
exclude group: "org.mortbay.jetty"
exclude group: "org.apache.velocity"
exclude group: "commons-lang"
exclude group: "org.jboss.netty"
exclude group: "com.thoughtworks.paranamer", module: "paranamer-ant"
}
//required because generated code depends on the helper
implementation project(":helper:helper")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"type": "record",
"namespace": "noutf8encoding",
"name": "ArrayOfStringRecord",
"doc": "Array of String Record",
"fields": [
{
"name": "arOfRec",
"type": {
"type":"array",
"items": "string"
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"type": "record",
"namespace": "noutf8encoding",
"name": "HasNoSimpleString",
"fields": [
{
"name": "one",
"type": "float"
},
{
"name": "two",
"type": ["string"]
},
{
"name": "three",
"type": "int"
},
{
"name": "four",
"type": "boolean"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
{
"name": "TestCollections",
"namespace": "noutf8encoding",
"fields": [
{
"name": "str",
"type": "string"
},
{
"name": "strAr",
"type": {
"type": "array",
"items": "string"
}
},
{
"name": "strArAr",
"type": {
"type": "array",
"items": {
"type": "array",
"items": "string"
}
}
},
{
"name": "unionOfArray",
"type": [
"null",
{
"type": "array",
"items": "string"
}
]
},
{
"name": "arOfMap",
"type": {
"type": "array",
"items": {
"type": "map",
"values": "string"
}
}
},
{
"name": "unionOfMap",
"type": [
"null",
{
"type": "map",
"values": "string"
}
]
},
{
"name": "arOfUnionOfStr",
"type": {
"type": "array",
"items": ["null", "string"]
}
},
{
"name": "arOfMapOfUnionOfArray",
"type": {
"type": "array",
"items": {
"type": "map",
"values": ["null", {
"type": "array",
"items": "string"
}]
}
}
},
{
"name": "intAr",
"type": {
"type": "array",
"items": "int"
}
},
{
"name": "unionOfIntMap",
"type": [
"null",
{
"type": "map",
"values": "int"
}
]
}
],
"type": "record"
}
3 changes: 3 additions & 0 deletions avro-builder/tests/tests-allavro/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ dependencies {
testImplementation (project(":avro-builder:tests:codegen-no-utf8-in-putbyindex")) {
exclude group: "org.apache.avro"
}
testImplementation (project(":avro-builder:tests:codegen-no-utf8-encoding")) {
exclude group: "org.apache.avro"
}

testImplementation "com.google.guava:guava:28.2-jre"
testImplementation "org.mockito:mockito-core:3.2.4"
Expand Down
Loading

0 comments on commit 17a828a

Please sign in to comment.