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

[Protobuf] Fix generation of "optional" labels in proto3 schema outW #5662

Open
wants to merge 1 commit into
base: 2.6.x
Choose a base branch
from

Conversation

jeroenvandisseldorp
Copy link

When parsing a Protobuf schema, and then generating it back, fields marked with "optional" don't get the same label in the generated output. Instead, they become required. This patch checks per field if it needs to set the "proto3Optional" flag (ignored for oneOf fields). The flag is set if the field is optional, rendering the expected "optional" keyword in fileElement.toSchema().

When parsing a Protobuf schema, and then generating it back, fields marked with "optional" become required. This patch checks per field if it needs to set the "proto3Optional" flag (ignored for oneOf fields). The flag is set if the field is optional, rendering the expected "optional" keyword in fileElement.toSchema().
@apicurio-bot
Copy link

apicurio-bot bot commented Dec 3, 2024

Thank you for creating a pull request!

Pinging @carlesarnal to respond or triage.

@jeroenvandisseldorp
Copy link
Author

I'm working on Protobuf support for KSML, see https://github.com/Axual/ksml

The following code converts an incoming Protobuf schema to KSML's internal data structure, and then back into a Protobuf schema. The compatibility checker then verifies the schema is similar, ie. no differences. The supplied patch is required to force "optional" fields to receive the "optional" label in the generated output from FileElement.toSchema().

    private void runCompatibilityTest(String name, String schemaIn) {
        // This method checks for code completeness of KSML's ProtobufSchemaMapper. It will warn if schema
        // translation to KSML's internal DataSchema and back gives deltas between input and output ProtobufSchemas.
        final var proto = new ProtobufSchemaParser<>().parseSchema(schemaIn.getBytes(), Collections.emptyMap());
        final var internalSchema = MAPPER.toDataSchema(name, proto);
        final var out = MAPPER.fromDataSchema(internalSchema);
        final var outFd = out.getFileDescriptor();
        final var outFe = new ProtobufSchemaParser<>().toProtoFileElement(outFd);
        final var schemaOut = outFe.toSchema();
        final var checker = new ProtobufCompatibilityCheckerLibrary(new ProtobufFile(schemaIn), new ProtobufFile(schemaOut));
        final var diffs = checker.findDifferences();
        final var compatible = diffs.isEmpty();
        if (!compatible) {
            log.warn("PROTOBUF schema {} in/out is not compatible: {}", name, diffs);
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant