Skip to content
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.

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

Avro generation failed with enums containing values with special characters #422

Closed
pfr-enedis opened this issue Dec 15, 2023 · 8 comments
Closed
Labels
2.19 avro has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue

Comments

@pfr-enedis
Copy link

pfr-enedis commented Dec 15, 2023

Enums with '-' characters in values, fails

public enum EnumTypeContrat {
        CARD_S("CARD-S"),
        CARD_ELD("CARD-ELD"),
        GRD_F("GRD-F"),
        CSD("CSD"),
        GRD_RE("GRD-RE"),
        PROTOC_501("PROTOC-501"),
        CRAE("CRAE"),
        CARD_I("CARD-I"),
        CSD_I("CSD-I");

        private final String value;

        EnumTypeContrat(String value) {
            this.value = value;
        }
}

The error is :

org.apache.avro.SchemaParseException: Illegal character in: CARD-S
	at org.apache.avro.Schema.validateName(Schema.java:1566)
	at org.apache.avro.Schema.access$400(Schema.java:91)
	at org.apache.avro.Schema$Field.<init>(Schema.java:546)
	at org.apache.avro.Schema$Field.<init>(Schema.java:585)
	at com.fasterxml.jackson.dataformat.avro.schema.RecordVisitor.schemaFieldForWriter(RecordVisitor.java:189)
	at com.fasterxml.jackson.dataformat.avro.schema.RecordVisitor.optionalProperty(RecordVisitor.java:117)
	at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.depositSchemaProperty(BeanPropertyWriter.java:843)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.acceptJsonFormatVisitor(BeanSerializerBase.java:914)
	at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.acceptJsonFormatVisitor(DefaultSerializerProvider.java:565)
	at com.fasterxml.jackson.databind.ObjectMapper.acceptJsonFormatVisitor(ObjectMapper.java:4670)
	at com.fasterxml.jackson.databind.ObjectMapper.acceptJsonFormatVisitor(ObjectMapper.java:4649)

are there any reason for checking the enum value ? as it is generated as an avro string ?

@cowtowncoder cowtowncoder added the need-test-case To work on issue, a reproduction (ideally unit test) needed label Dec 16, 2023
@cowtowncoder
Copy link
Member

Please include actual code you are using: textual description is not enough to know what is being attempted.

Note, though, that the exception comes from Apache Avro library which does validation for generated schema.

@pfr-enedis
Copy link
Author

pfr-enedis commented Dec 18, 2023

Hi, @cowtowncoder , here is the reproductible example,
it seems the reproduction behaviour, is more linked to the JsonValue annotation,
when commenting this annotation, this work, and fail if the annotation is present

import java.util.HashMap;
import java.util.Map;

import javax.annotation.processing.Generated;

import org.junit.Test;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.databind.MapperFeature;
import com.fasterxml.jackson.dataformat.avro.AvroMapper;
import com.fasterxml.jackson.dataformat.avro.jsr310.AvroJavaTimeModule;
import com.fasterxml.jackson.dataformat.avro.schema.AvroSchemaGenerator;


/**
 * class to reproduce an avro error, for jackson databind
 */
public class GenerateAvroError {

    @Generated("jsonschema2pojo")
    public enum EnumTypeContrat {

        CARD_S("CARD-S"),
        CARD_ELD("CARD-ELD"),
        GRD_F("GRD-F"),
        CSD("CSD"),
        GRD_RE("GRD-RE"),
        PROTOC_501("PROTOC-501"),
        CRAE("CRAE"),
        CARD_I("CARD-I"),
        CSD_I("CSD-I");
        private final String value;
        private final static Map<String, EnumTypeContrat> CONSTANTS = new HashMap<String,EnumTypeContrat>();

        static {
            for (EnumTypeContrat c: values()) {
                CONSTANTS.put(c.value, c);
            }
        }

        EnumTypeContrat(String value) {
            this.value = value;
        }

        @Override
        public String toString() {
            return this.value;
        }

        @JsonValue
        public String value() {
            return this.value;
        }

        @JsonCreator
        public static EnumTypeContrat fromValue(String value) {
            EnumTypeContrat constant = CONSTANTS.get(value);
            if (constant == null) {
                throw new IllegalArgumentException(value);
            } else {
                return constant;
            }
        }

    }

	public static class DummyClassToReproduceError {

		private EnumTypeContrat contract;

		@JsonProperty("contr")
		public EnumTypeContrat getContract() {
			return contract;
		}

	}
	

	@Test
	public void testSerializeAvro() throws Exception {

		AvroSchemaGenerator gen = new AvroSchemaGenerator();

		AvroMapper avroMapper = AvroMapper.builder().disable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY)
				.addModule(new AvroJavaTimeModule()).build();

		gen.enableLogicalTypes();
		avroMapper.acceptJsonFormatVisitor(DummyClassToReproduceError.class, gen);
		com.fasterxml.jackson.dataformat.avro.AvroSchema schemaWrapper = gen.getGeneratedSchema();

		org.apache.avro.Schema avroSchema = schemaWrapper.getAvroSchema();
		String avroSchemaInJSON = avroSchema.toString(true);

		System.out.println(avroSchemaInJSON);
		
	}
	
	
}

the associated backtrace :

org.apache.avro.SchemaParseException: Illegal character in: CARD-S
	at org.apache.avro.Schema.validateName(Schema.java:1566)
	at org.apache.avro.Schema.access$400(Schema.java:91)
	at org.apache.avro.Schema$EnumSchema.<init>(Schema.java:1035)
	at org.apache.avro.Schema.createEnum(Schema.java:227)
	at com.fasterxml.jackson.dataformat.avro.schema.AvroSchemaHelper.createEnumSchema(AvroSchemaHelper.java:272)
	at com.fasterxml.jackson.dataformat.avro.schema.StringVisitor.builtAvroSchema(StringVisitor.java:54)
	at com.fasterxml.jackson.dataformat.avro.schema.VisitorFormatWrapperImpl.getAvroSchema(VisitorFormatWrapperImpl.java:103)
	at com.fasterxml.jackson.dataformat.avro.schema.RecordVisitor.schemaFieldForWriter(RecordVisitor.java:175)
	at com.fasterxml.jackson.dataformat.avro.schema.RecordVisitor.optionalProperty(RecordVisitor.java:117)
	at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.depositSchemaProperty(BeanPropertyWriter.java:843)
	at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.acceptJsonFormatVisitor(BeanSerializerBase.java:914)
	at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.acceptJsonFormatVisitor(DefaultSerializerProvider.java:565)
	at com.fasterxml.jackson.databind.ObjectMapper.acceptJsonFormatVisitor(ObjectMapper.java:4670)
	at com.fasterxml.jackson.databind.ObjectMapper.acceptJsonFormatVisitor(ObjectMapper.java:4649)
	at xxxxxx.testSerializeAvro(GenerateAvroError.java:92)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:93)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:40)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:529)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:756)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:452)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)


@cowtowncoder cowtowncoder added has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue 2.17 and removed need-test-case To work on issue, a reproduction (ideally unit test) needed labels Dec 22, 2023
@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 22, 2023

Thank you @pfr-enedis for the reproduction.

Yes, you are probably right wrt @JsonValue.

@cowtowncoder cowtowncoder added 2.19 and removed 2.17 labels Jan 4, 2025
@cowtowncoder
Copy link
Member

@pfr-enedis Quick question: do you think new AvroSchemaGenerator configuration option added via #494 would solve the issue? Problem here is, I think, two-fold:

  1. Since Enums are expressed as Java Strings otherwise (with JSON and many other formats), @JsonValue returning String isn't considered to change type from Enum to some other type, so type is generated as Avro Enum
  2. Avro Enum names have somewhat strict validity rules

But with AvroSchemaGenerator.enableWriteEnumAsString() would change actual Avro type used.

@MichalFoksa WDYT?

@MichalFoksa
Copy link
Contributor

With AvroSchemaGenerator.enableWriteEnumAsString() Avro type for EnumTypeContrat is string and the test above works.

schema:
{
  "type" : "record",
  "name" : "DummyClassToReproduceError",
  "namespace" : "com.fasterxml.jackson.dataformat.avro.Issue422Test",
  "fields" : [ {
    "name" : "contr",
    "type" : [ "null", {
      "type" : "string",
      "java-class" : "com.fasterxml.jackson.dataformat.avro.Issue422Test$EnumTypeContrat"
    } ]
  } ]
}

@cowtowncoder
Copy link
Member

@MichalFoksa excellent! Going forward it might be good to have a way to target specific classes (new annotation?), but for now I think this should be fine.

@cowtowncoder
Copy link
Member

May close in near future as there is a work-around, but will first see if there's a way to handle error in better way.

cowtowncoder added a commit that referenced this issue Jan 6, 2025
cowtowncoder added a commit that referenced this issue Jan 6, 2025
@cowtowncoder
Copy link
Member

Improved error handling; will close this issue. New issue(s) may be filed for suggesting further improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.19 avro has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue
Projects
None yet
Development

No branches or pull requests

3 participants