Support SimpleTypes defined as a union of enums as enums#795
Support SimpleTypes defined as a union of enums as enums#795erlendnils1 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the XSD→Proto generator to recognize xsd:simpleType unions whose members are all enumerations and emit a corresponding merged enum in the generated .proto.
Changes:
- Add XSD + expected-proto fixtures for a “union of enums” scenario.
- Add a unit test covering proto generation for the new fixture.
- Add SchemaParser logic to synthesize an enum from
xs:unionmembers when all are enum restrictions.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| schema2proto-lib/src/test/resources/xsd/basic/union-of-enums.xsd | New XSD fixture for a union composed solely of enum simpleTypes. |
| schema2proto-lib/src/test/resources/expectedproto/basic/default/union-of-enums.proto | Expected proto output including merged UnionEnumeration enum. |
| schema2proto-lib/src/test/java/no/entur/schema2proto/generateproto/SchemaParserTest.java | Adds a unit test that compares generated output to the expected proto. |
| schema2proto-lib/src/main/java/no/entur/schema2proto/generateproto/SchemaParser.java | Implements enum generation for union-of-enums simpleTypes. |
| schema2proto-lib/src/main/java/no/entur/schema2proto/compatibility/protolock/ProtolockEnumConstant.java | Makes name field final. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private void createEnumFromUnion(String typeName, XSUnionSimpleType unionType) { | ||
| Type protoType = getType(unionType.getTargetNamespace(), typeName); | ||
| if (protoType == null) { | ||
| Location location = getLocation(unionType); | ||
| List<EnumConstant> constants = new ArrayList<>(); | ||
| int counter = 1; | ||
| Set<String> addedValues = new HashSet<>(); | ||
|
|
||
| for (int i = 0; i < unionType.getMemberSize(); i++) { | ||
| XSRestrictionSimpleType member = unionType.getMember(i).asRestriction(); | ||
| for (XSFacet facet : member.getDeclaredFacets()) { | ||
| String enumValue = facet.getValue().value; | ||
| if (addedValues.add(enumValue)) { | ||
| String doc = resolveDocumentationAnnotation(facet, false); | ||
| List<OptionElement> optionElements = new ArrayList<>(); | ||
| constants.add(new EnumConstant(location, enumValue, counter++, doc, new Options(Options.ENUM_VALUE_OPTIONS, optionElements))); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| List<OptionElement> enumOptionElements = new ArrayList<>(); | ||
| Options enumOptions = new Options(Options.ENUM_OPTIONS, enumOptionElements); | ||
| String doc = resolveDocumentationAnnotation(unionType, false); | ||
|
|
||
| ProtoType definedProtoType = ProtoType.get(typeName); | ||
| EnumType enumType = new EnumType(definedProtoType, location, doc, typeName, constants, new ArrayList<>(), enumOptions); | ||
| addType(unionType.getTargetNamespace(), enumType); | ||
| } |
There was a problem hiding this comment.
createEnumFromUnion always adds the generated enum at file scope. If a union-of-enums occurs as a local/anonymous simpleType (e.g., inline on an element), this will create a global enum with a generated name rather than nesting it under the enclosing message (unlike createEnum(...) which supports enclosingType). If local union-of-enums should be supported, consider threading through the enclosing message (or otherwise aligning scoping rules) to avoid polluting the global namespace and to keep naming consistent.
| @Test | ||
| public void testUnionOfEnums() throws IOException { | ||
| generateProtobufNoOptions("basic/union-of-enums.xsd"); | ||
| compareExpectedAndGenerated(expectedRootFolder, "default/union-of-enums.proto", generatedRootFolder, "default/default.proto"); | ||
| } |
There was a problem hiding this comment.
This test only asserts that a union-of-enums simpleType produces an enum declaration, but it doesn’t cover the key behavior of using that enum when the union type is referenced by an element/attribute/simpleContent. Adding an XSD fixture where UnionEnumeration is used as a field type would help prevent regressions where unions still resolve to string.
| } else if (xs.isUnion()) { | ||
| XSUnionSimpleType unionType = xs.asUnion(); | ||
| boolean allMembersAreEnums = true; | ||
| for (int i = 0; i < unionType.getMemberSize(); i++) { | ||
| XSSimpleType member = unionType.getMember(i); | ||
| if (!member.isRestriction() || member.getFacet(XSFacet.FACET_ENUMERATION) == null) { | ||
| allMembersAreEnums = false; | ||
| break; | ||
| } | ||
| } | ||
| if (allMembersAreEnums) { | ||
| createEnumFromUnion(typeName, unionType); | ||
| } |
There was a problem hiding this comment.
This adds enum generation for union types, but the rest of SchemaParser still resolves unions to string (e.g., findFieldType() returns DEFAULT_PROTO_PRIMITIVE for unions and other call sites treat unions specially). As a result, elements/attributes/simpleContent that use a named union-of-enums type will still end up as string fields even though an enum type is generated. Consider updating the type resolution logic to detect “union of enums” and return/use the union type name (and invoke createEnumFromUnion when needed) so generated fields actually use the enum.
…lity/protolock/ProtolockEnumConstant.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…(generated enum types) and other unions (string)
Types like UnionEnumeration below have been ignored until now:
How should these be converted to proto? Choicewrapper type may be safest, but fare more messy to work with.
This PR proposes creating enums from the combined enum values of all union members if all values are enums. This makes the use as close to the xsd type as possible, but creates a risk for backwards compatibility issues if union type is later changed to allow non enum values 🤔