[Avro] Generate logicalType switch

[MichalFoksa]

Hi @cowtowncoder

I want to implement a toggle to enable / disable logicalType generation in Avro schema. I am bit lost here, I do not know where to add new feature, whether to AvroGenerator.Feature, AvroMapper.Feature or AvroParser.

When I start backwards, I do know how to check whether the feature is enable. I have in my mind following implementation in VisitorFormatWrapperImpl:

public class VisitorFormatWrapperImpl implements JsonFormatVisitorWrapper {

   @Override
    public JsonIntegerFormatVisitor expectIntegerFormat(JavaType type) {
        ...
        if (GENERATE_LOGICAL_TYPE feature enabled){
            if (_isDateTimeType(type)) {
                DateTimeVisitor v = new DateTimeVisitor(type);
                _builder = v;
                return v;
            }
        }

        IntegerVisitor v = new IntegerVisitor(type);
        _builder = v;
        return v;
    }
}

My Question is: How to make if (GENERATE_LOGICAL_TYPE feature enabled) condition?

Cheers, Michal

[cowtowncoder]

Yeah that visitor system is... not great. Came originally as a contribution for generating JSON Schema. But I think there is a mechanism that typically initializes context, let me dig and see if I can remember how it works.

[cowtowncoder]

Ok, yes. So, there is VisitorFormatWrapperImpl, which AvroSchemaGenerator extends: it has methods setProvider() (called by ObjectMapper to pass SerializerProvider) and getProvider().

SerializerProvider has access to SerializationConfig and thereby SerializationFeatures as well as MapperFeatures. That would be the path through which configuration settings may be found; it might be necessary to change actual Visitors to pass information but possibly not -- looks like visitors are constructed by Avro package defined VisitorFormatWrapperImpl.

It does get bit convoluted but I hope above helps you find access you need.

[MichalFoksa]

Hmmm :(

With SerializerProvider or SerializationConfig one can only read state of MapperFeature or SerializationFeature. Both are Java Enums, thus cannot be extended or inherited from. Both are defined in jackson-databind - which should not be extended just because of some Avro module.

public isEnabled(com.fasterxml.jackson.databind.MapperFeature feature);
public isEnabled(com.fasterxml.jackson.databind.SerializationFeature feature);

One way is to create a toggle method in VisitorFormatWrapperImpl - that, I guess, creates a new patter of working with visitor. Something like:

AvroMapper mapper = AvroMapper.builder()
        .addModule(new AvroJavaTimeModule())
        .build();
AvroSchemaGenerator gen = new AvroSchemaGenerator();
gen.enableLogicalTypes();                             // Enable logicalType suport

mapper.acceptJsonFormatVisitor(testClass, gen);
Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();

I do not know, .... I keep searching ....

[cowtowncoder]

@MichalFoksa there should be a way via SerializerProvider to access "Format [write] features" too. Although... often it'd be only via JsonGenerator. I forget if added access was 3.0-only thing or not; 2.x does have access to JsonParser but I vaguely recall writer-side may be different

If I have time today, I'll see what could be done here.

[cowtowncoder]

Hmmmh. So, SerializationConfig does have format feature flags but they are not accessible as things are (in 2.x), partly since there are base settings from JsonFactory and possible overrides via ObjectWriter -- SerializationConfig only has second set of feature flags.

Given this, your approach may be the way to go; and the only (?) question being whether to enable them by default, or to require specific call. There probably needs to be matching disableLogicalTypes() method too.

I think it also makes sense to extract methods in such a way that sub-classing could be used by developers if necessary to change call logic.

[MichalFoksa]

Thank you,

I thought so. Will look at it later and post a PR next week.

[MichalFoksa]

@cowtowncoder Here you are the PR with the switch implementation. Logical types generation is disabled by default - it more sensitive that way for existing users.

Better start review with #292 - switch is build upon it.