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.

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

Support enum avro serialization with default value #391

Open
wants to merge 4 commits into
base: 2.16
Choose a base branch
from
Open

Support enum avro serialization with default value #391

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
736999c
Support enum avro serialization with default value
Sonic-Rage Sep 8, 2023
9f799db
Add missing import statements
cowtowncoder Sep 11, 2023
d8c4fea
Revert Avro version upgrade (code not compatible with 1.9+)
cowtowncoder Sep 11, 2023
9790707
Remove use of static AnnotationIntrospector
cowtowncoder Sep 11, 2023
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
2 changes: 1 addition & 1 deletion avro/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ abstractions.
<dependency>
<groupId>org.apache.avro</groupId>
<artifactId>avro</artifactId>
<version>1.8.2</version>
<version>1.9.2</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under impression that this will break about everything... did you run unit test suite?

But even if not, we need this to be in separate PR, with separate issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wary of that upgrade as well, but doing that version bump and running the avro test suite resulted in passing tests.

We're a gradle shop so running stuff with maven was a bit hokey with all our firewall rules. Readying through #167 looks like some stuff has been upgraded but I imagine there must be missing unit tests for some of the underlying avro lib changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that's odd wrt tests: for me code did not compile with missing imports.
And compilation on CI and locally did not work.

I wish I was more up to date with Avro lib changes since there are newer minor versions, some of which might resolve issues -- although from what I remember, things were quite messy wrt backwards-compatibility (lack thereof) for some specific usage.

</dependency>

<!-- and for testing we need logback -->
Expand Down
12 changes: 11 additions & 1 deletion avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/AvroSchemaHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.introspect.AnnotatedClass;
import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor;
import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
import com.fasterxml.jackson.databind.json.JsonMapper;
import com.fasterxml.jackson.databind.jsonFormatVisitors.JsonFormatTypes;
import com.fasterxml.jackson.databind.util.ClassUtil;
Expand Down Expand Up @@ -71,6 +72,14 @@ public abstract class AvroSchemaHelper
String.class
));

/**
*
* Jackson annotation introspector for verifying enum default annotation for Avro Schema generation
*
* @since 2.16
*/
private static final JacksonAnnotationIntrospector JACKSON_ANNOTATION_INTROSPECTOR = new JacksonAnnotationIntrospector();
Copy link
Member

@cowtowncoder cowtowncoder Sep 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, AnnotationIntrospector should be passed from outside; it may be reconfigured by caller.
We should not rely on default instance.

This just means some more plumbing of things to get AnnotationIntrospector passed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. thanks


/**
* Checks if a given type is "Stringable", that is one of the default
* {@code STRINGABLE_CLASSES}, is an {@code Enum},
Expand Down Expand Up @@ -269,10 +278,11 @@ public static Schema parseJsonSchema(String json) {
*/
public static Schema createEnumSchema(BeanDescription bean, List<String> values) {
final JavaType enumType = bean.getType();
Enum<?> defaultEnumValue = JACKSON_ANNOTATION_INTROSPECTOR.findDefaultEnumValue((Class<Enum<?>>)(Class<?>) enumType.getRawClass());
return addAlias(Schema.createEnum(
getName(enumType),
bean.findClassDescription(),
getNamespace(enumType, bean.getClassInfo()), values
getNamespace(enumType, bean.getClassInfo()), values, defaultEnumValue != null ? defaultEnumValue.toString() : null
), bean);
}

Expand Down
15 changes: 15 additions & 0 deletions avro/src/test/java/com/fasterxml/jackson/dataformat/avro/AvroTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import junit.framework.TestCase;

import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.annotation.JsonEnumDefaultValue;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -249,6 +251,19 @@ public Image(String uri, String title, int w, int h, Size s)

public enum Size { SMALL, LARGE; }

public enum ABC {
A,
B,
@JsonEnumDefaultValue
C;
}

public static class ABCDefaultClass {
public String name;
@JsonProperty(required = true)
public ABC abc;
}

/*
/**********************************************************
/* Recycling for commonly needed helper objects
Expand Down
35 changes: 35 additions & 0 deletions avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/SchemaGenerationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.fasterxml.jackson.dataformat.avro.*;

import org.apache.avro.Schema;
import org.apache.avro.Schema.Type;

public class SchemaGenerationTest extends AvroTestBase
{
Expand Down Expand Up @@ -169,4 +170,38 @@ public void testSchemaForUntypedMap() throws Exception
verifyException(e, "Maps with non-stringable keys are not supported (yet?)");
}
}

// Issue 388 Default value for enums with class
public void testClassEnumWithDefault() throws Exception
{
AvroSchemaGenerator gen = new AvroSchemaGenerator();

MAPPER.acceptJsonFormatVisitor(ABCDefaultClass.class, gen);
AvroSchema schema = gen.getGeneratedSchema();
assertNotNull(schema);

String json = schema.getAvroSchema().toString(true);
assertNotNull(json);


// And read it back too just for fun
AvroSchema s2 = MAPPER.schemaFrom(json);
assertNotNull(s2);

Schema avroSchema = s2.getAvroSchema();

// String name, int value
assertEquals(Type.RECORD, avroSchema.getType());
Schema.Field f = avroSchema.getField("abc");
assertNotNull(f);
assertEquals("abc", f.name());

assertEquals(Type.ENUM, f.schema().getType());
assertEquals(ABC.C.toString(), f.schema().getEnumDefault());
assertEquals(Stream.of(ABC.values())
.map(ABC::name)
.collect(Collectors.toList()), f.schema().getEnumSymbols());


}
}
Loading