From 43040ddfbdc2f6b3dc286949d0df233266638e36 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Tue, 12 Sep 2023 15:47:40 +0200 Subject: [PATCH 1/4] Improve tests and documentation of annotation access from PropertyInfo --- .../src/main/java/io/vertx/codegen/PropertyInfo.java | 5 +++++ .../src/test/java/io/vertx/test/codegen/DataObjectTest.java | 3 +++ .../testdataobject/DataObjectWithAnnotatedField.java | 6 +++--- .../vertx/test/codegen/testdataobject/SomeAnnotation.java | 1 + 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/vertx-codegen-processor/src/main/java/io/vertx/codegen/PropertyInfo.java b/vertx-codegen-processor/src/main/java/io/vertx/codegen/PropertyInfo.java index 1711e8c10..4fc37ad71 100644 --- a/vertx-codegen-processor/src/main/java/io/vertx/codegen/PropertyInfo.java +++ b/vertx-codegen-processor/src/main/java/io/vertx/codegen/PropertyInfo.java @@ -108,6 +108,11 @@ public List getAnnotations() { return new ArrayList<>(annotations.values()); } + /** + * @param annotationName fully qualified name of an annotation type + * @return {@link AnnotationValueInfo} for an annotation of given type present on this property, + * or {@code null} if an annotation of given type is not present on this property + */ public AnnotationValueInfo getAnnotation(String annotationName) { return annotations.get(annotationName); } diff --git a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java index 765a667bd..221289e20 100644 --- a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java +++ b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java @@ -893,9 +893,12 @@ public void testDataObjectWithAnnotations() throws Exception { PropertyInfo idModel = model.getPropertyMap().get("id"); assertEquals(1, idModel.getAnnotations().size()); assertNotNull(idModel.getAnnotation(SomeAnnotation.class.getName()).getName()); + assertEquals(SomeAnnotation.class.getName(), idModel.getAnnotation(SomeAnnotation.class.getName()).getName()); + assertEquals(2, idModel.getAnnotation(SomeAnnotation.class.getName()).getMember("value")); PropertyInfo fieldWithMethodAnnotationModel = model.getPropertyMap().get("fieldWithMethodAnnotation"); assertEquals(2, fieldWithMethodAnnotationModel.getAnnotations().size()); assertNotNull(fieldWithMethodAnnotationModel.getAnnotation(SomeAnnotation.class.getName()).getName()); + assertEquals(3, fieldWithMethodAnnotationModel.getAnnotation(SomeAnnotation.class.getName()).getMember("value")); assertNotNull(fieldWithMethodAnnotationModel.getAnnotation(SomeMethodAnnotation.class.getName()).getName()); } diff --git a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/DataObjectWithAnnotatedField.java b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/DataObjectWithAnnotatedField.java index fe1fdfa93..2853d8269 100644 --- a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/DataObjectWithAnnotatedField.java +++ b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/DataObjectWithAnnotatedField.java @@ -3,18 +3,18 @@ import io.vertx.codegen.annotations.DataObject; import io.vertx.core.json.JsonObject; -@SomeAnnotation +@SomeAnnotation(1) @DataObject(generateConverter = true) public class DataObjectWithAnnotatedField { - @SomeAnnotation + @SomeAnnotation(2) private Long id; private String name; private String author; - @SomeAnnotation + @SomeAnnotation(3) private String fieldWithMethodAnnotation; // Mandatory for JPA entities diff --git a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/SomeAnnotation.java b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/SomeAnnotation.java index 579b9eed8..e7d9930dd 100644 --- a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/SomeAnnotation.java +++ b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testdataobject/SomeAnnotation.java @@ -8,4 +8,5 @@ */ @Target({ElementType.CONSTRUCTOR, ElementType.FIELD, ElementType.TYPE}) public @interface SomeAnnotation { + int value(); } From 9f3b93906c2acbf92e540e445f69c8923c69828b Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 18 Sep 2023 11:41:23 +0200 Subject: [PATCH 2/4] Add convenient method Model.getAnnotation() --- .../src/main/java/io/vertx/codegen/Model.java | 12 ++++++++++++ .../java/io/vertx/test/codegen/DataObjectTest.java | 8 +++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/vertx-codegen-processor/src/main/java/io/vertx/codegen/Model.java b/vertx-codegen-processor/src/main/java/io/vertx/codegen/Model.java index e0f1daeb4..1387e398c 100644 --- a/vertx-codegen-processor/src/main/java/io/vertx/codegen/Model.java +++ b/vertx-codegen-processor/src/main/java/io/vertx/codegen/Model.java @@ -4,10 +4,12 @@ import io.vertx.codegen.type.TypeInfo; import javax.lang.model.element.Element; +import java.lang.annotation.Annotation; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; /** * @author Julien Viet @@ -26,6 +28,16 @@ default List getAnnotations() { return Collections.emptyList(); } + default Optional getAnnotation(Class annotationType) { + String annotationName = annotationType.getName(); + for (AnnotationValueInfo annotation : getAnnotations()) { + if (annotation.getName().equals(annotationName)) { + return Optional.of(annotation); + } + } + return Optional.empty(); + } + default Map getVars() { HashMap vars = new HashMap<>(); vars.put("helper", new Helper()); diff --git a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java index 221289e20..ec1795829 100644 --- a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java +++ b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/DataObjectTest.java @@ -4,6 +4,8 @@ import io.vertx.codegen.GenException; import io.vertx.codegen.PropertyInfo; import io.vertx.codegen.PropertyKind; +import io.vertx.codegen.annotations.DataObject; +import io.vertx.codegen.annotations.VertxGen; import io.vertx.codegen.doc.Doc; import io.vertx.codegen.type.*; import io.vertx.core.json.JsonArray; @@ -672,7 +674,11 @@ public void testToJson() throws Exception { public void testAnnotatedObject() throws Exception { DataObjectModel model = new GeneratorHelper().generateDataObject(AnnotatedDataObject.class); assertEquals(2, model.getAnnotations().size()); - assertEquals(EmptyAnnotation.class.getSimpleName(),model.getAnnotations().get(1).getSimpleName()); + assertEquals(DataObject.class.getSimpleName(), model.getAnnotations().get(0).getSimpleName()); + assertEquals(EmptyAnnotation.class.getSimpleName(), model.getAnnotations().get(1).getSimpleName()); + assertTrue(model.getAnnotation(DataObject.class).isPresent()); + assertTrue(model.getAnnotation(EmptyAnnotation.class).isPresent()); + assertFalse(model.getAnnotation(Deprecated.class).isPresent()); } @Test From bdcbc3ffed08239b45583171b00ab49504897bc9 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Fri, 29 Sep 2023 17:23:43 +0200 Subject: [PATCH 3/4] Add support for annotation access from EnumValueInfo --- .../main/java/io/vertx/codegen/EnumModel.java | 9 ++++++- .../java/io/vertx/codegen/EnumValueInfo.java | 26 ++++++++++++++++++- .../java/io/vertx/test/codegen/EnumTest.java | 4 +++ .../test/codegen/testenum/SomeAnnotation.java | 5 ++++ .../test/codegen/testenum/ValidEnum.java | 3 +++ 5 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/SomeAnnotation.java diff --git a/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumModel.java b/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumModel.java index ac7bb1c04..37e90db93 100644 --- a/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumModel.java +++ b/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumModel.java @@ -10,6 +10,7 @@ import io.vertx.codegen.type.TypeMirrorFactory; import javax.annotation.processing.ProcessingEnvironment; +import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.TypeElement; @@ -17,6 +18,7 @@ import javax.lang.model.type.TypeMirror; import javax.lang.model.util.Elements; import javax.lang.model.util.Types; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Optional; @@ -85,7 +87,12 @@ public boolean process() { enumItemDeprecatedDesc = new Text(Helper.normalizeWhitespaces(methodDeprecatedTag.get().getValue())).map(Token.tagMapper(elementUtils, typeUtils, modelElt)); } } - return new EnumValueInfo(elt.getSimpleName().toString(), doc, elt.getAnnotation(Deprecated.class) != null, enumItemDeprecatedDesc); + List annotationMirrors = elt.getAnnotationMirrors(); + List annotationValueInfos = new ArrayList<>(); + if (annotationMirrors != null) { + annotationMirrors.stream().map(annotationValueInfoFactory::processAnnotation).forEach(annotationValueInfos::add); + } + return new EnumValueInfo(elt.getSimpleName().toString(), doc, elt.getAnnotation(Deprecated.class) != null, enumItemDeprecatedDesc, annotationValueInfos); }). collect(Collectors.toList()); if (values.isEmpty()) { diff --git a/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumValueInfo.java b/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumValueInfo.java index 823b7ab37..8f758fc24 100644 --- a/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumValueInfo.java +++ b/vertx-codegen-processor/src/main/java/io/vertx/codegen/EnumValueInfo.java @@ -2,6 +2,12 @@ import io.vertx.codegen.doc.Doc; import io.vertx.codegen.doc.Text; +import io.vertx.codegen.type.AnnotationValueInfo; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; /** * The value (member) of an enumeration model. @@ -14,12 +20,14 @@ public class EnumValueInfo { private final Doc doc; private final boolean deprecated; private final Text deprecatedDesc; + private final Map annotations; - public EnumValueInfo(String identifier, Doc doc, boolean deprecated, Text deprecatedDesc) { + public EnumValueInfo(String identifier, Doc doc, boolean deprecated, Text deprecatedDesc, List annotations) { this.identifier = identifier; this.doc = doc; this.deprecated = deprecated; this.deprecatedDesc = deprecatedDesc; + this.annotations = annotations.stream().collect(HashMap::new, (m, a) -> m.put(a.getName(), a), HashMap::putAll); } /** @@ -48,4 +56,20 @@ public boolean isDeprecated() { public Text getDeprecatedDesc() { return deprecatedDesc; } + + /** + * @return the list of {@link AnnotationValueInfo} for this enum value + */ + public List getAnnotations() { + return new ArrayList<>(annotations.values()); + } + + /** + * @param annotationName fully qualified name of an annotation type + * @return {@link AnnotationValueInfo} for an annotation of given type present on this enum value, + * or {@code null} if an annotation of given type is not present on this enum value + */ + public AnnotationValueInfo getAnnotation(String annotationName) { + return annotations.get(annotationName); + } } diff --git a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/EnumTest.java b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/EnumTest.java index 553dab6c8..ed398a52e 100644 --- a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/EnumTest.java +++ b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/EnumTest.java @@ -9,6 +9,7 @@ import io.vertx.test.codegen.testapi.jsonmapper.WithMyCustomEnumWithMapper; import io.vertx.test.codegen.testenum.EnumAsParam; import io.vertx.test.codegen.testenum.InvalidEmptyEnum; +import io.vertx.test.codegen.testenum.SomeAnnotation; import io.vertx.test.codegen.testenum.ValidEnum; import org.junit.Test; @@ -31,6 +32,9 @@ public void testEnum() throws Exception { assertEquals(Arrays.asList("RED doc", "GREEN doc", "BLUE doc"), model.getValues().stream(). map(e -> e.getDoc().toString()). collect(Collectors.toList())); + assertEquals(Arrays.asList("red", "green", "blue"), model.getValues().stream() + .map(e -> e.getAnnotation(SomeAnnotation.class.getName()).getMember("value")) + .collect(Collectors.toList())); assertEquals("enum", model.getKind()); assertEquals("ValidEnum doc", model.getDoc().toString()); assertEquals(ValidEnum.class.getName(), model.getFqn()); diff --git a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/SomeAnnotation.java b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/SomeAnnotation.java new file mode 100644 index 000000000..bfeb9da95 --- /dev/null +++ b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/SomeAnnotation.java @@ -0,0 +1,5 @@ +package io.vertx.test.codegen.testenum; + +public @interface SomeAnnotation { + String value(); +} diff --git a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/ValidEnum.java b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/ValidEnum.java index be1f97059..fa3108e75 100644 --- a/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/ValidEnum.java +++ b/vertx-codegen-processor/src/test/java/io/vertx/test/codegen/testenum/ValidEnum.java @@ -7,12 +7,15 @@ public enum ValidEnum { /**RED doc*/ + @SomeAnnotation("red") RED, /**GREEN doc*/ + @SomeAnnotation("green") GREEN, /**BLUE doc*/ + @SomeAnnotation("blue") BLUE } From df2886df5e4c47b27fc394702359fc61abaead62 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Mon, 18 Sep 2023 12:10:54 +0200 Subject: [PATCH 4/4] Introduce mechanism for specifying protobuf field numbers Previously, field numbers were always assigned automatically during protobuf schema generation, based on declaration order of properties in a data class. It is very easy to modify the data class in a way that changes field numbers unintentionally, which introduces a breaking change to the protobuf schema. This commit introduces the annotation `@ProtobufField` as a mechanism for explicitly providing field numbers, as well as a `FieldNumberStrategy` enum (used in `@ProtobufGen.fieldNumberStrategy`) to select how field numbers should be assigned. There's a manual strategy and two automatic strategies. Further, `@ProtobufGen.reservedFieldNumbers` and `reservedFieldNames` allow specifying reserved field numbers / names, similarly to the protobuf language itself. Reserved field numbers and names are verified at compile time. The fields in the protobuf schema are generated in the field number order, not in the property declaration order. The serialization/deserialization code in generated converters also uses field number order. This is to make sure that a Vert.x-generated and `protoc`-generated serializers produce outputs that are not just mutually _compatible_, but also binary _identical_. --- .../converters/generated/dataobjects.proto | 14 ++ .../codegen/converter/BookProtoConverter.java | 91 +++++++++ .../converter/PersonProtoConverter.java | 71 +++++++ .../vertx/test/codegen/converter/Address.java | 3 +- .../io/vertx/test/codegen/converter/Book.java | 64 +++++++ .../test/codegen/converter/EnumType.java | 3 +- .../vertx/test/codegen/converter/Person.java | 46 +++++ .../test/codegen/converter/RecursiveItem.java | 3 +- .../io/vertx/test/codegen/converter/User.java | 3 +- .../annotations/FieldNumberStrategy.java | 45 +++++ .../protobuf/annotations/ProtobufField.java | 16 ++ .../protobuf/annotations/ProtobufGen.java | 27 +++ .../generator/DataObjectProtobufGen.java | 28 ++- .../generator/JsonProtoEncodingSelector.java | 22 --- .../protobuf/generator/ProtoFileGen.java | 40 +++- .../protobuf/generator/ProtoProperty.java | 12 ++ .../protobuf/generator/ProtobufFields.java | 149 +++++++++++++++ .../generator/ProtobufGenAnnotation.java | 44 +++++ .../generator/ProtobufFieldsTest.java | 179 ++++++++++++++++++ 19 files changed, 817 insertions(+), 43 deletions(-) create mode 100644 vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/BookProtoConverter.java create mode 100644 vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/PersonProtoConverter.java create mode 100644 vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Book.java create mode 100644 vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Person.java create mode 100644 vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/FieldNumberStrategy.java create mode 100644 vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufField.java delete mode 100644 vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/JsonProtoEncodingSelector.java create mode 100644 vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufFields.java create mode 100644 vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufGenAnnotation.java create mode 100644 vertx-codegen-protobuf/src/test/java/io/vertx/codegen/protobuf/generator/ProtobufFieldsTest.java diff --git a/vertx-codegen-protobuf/src/converters/generated/dataobjects.proto b/vertx-codegen-protobuf/src/converters/generated/dataobjects.proto index 2bddf194c..b210ad988 100644 --- a/vertx-codegen-protobuf/src/converters/generated/dataobjects.proto +++ b/vertx-codegen-protobuf/src/converters/generated/dataobjects.proto @@ -16,12 +16,26 @@ message Address { float latitude = 3; } +message Book { + reserved 2; + reserved "title"; + string name = 1; + string author = 3; + string isbn = 10; + string genre = 20; +} + enum EnumType { A = 0; B = 1; C = 2; } +message Person { + string name = 2; + int32 age = 4; +} + message RecursiveItem { string id = 1; RecursiveItem childA = 2; diff --git a/vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/BookProtoConverter.java b/vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/BookProtoConverter.java new file mode 100644 index 000000000..0b617d0c9 --- /dev/null +++ b/vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/BookProtoConverter.java @@ -0,0 +1,91 @@ +package io.vertx.test.codegen.converter; + +import com.google.protobuf.CodedOutputStream; +import com.google.protobuf.CodedInputStream; +import java.io.IOException; +import java.time.Instant; +import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.List; +import java.util.HashMap; +import java.util.Map; +import java.util.Arrays; +import io.vertx.core.json.JsonObject; +import io.vertx.codegen.protobuf.utils.ExpandableIntArray; +import io.vertx.codegen.protobuf.converters.*; + +public class BookProtoConverter { + + public static void fromProto(CodedInputStream input, Book obj) throws IOException { + int tag; + while ((tag = input.readTag()) != 0) { + switch (tag) { + case 10: { + obj.setName(input.readString()); + break; + } + case 26: { + obj.setAuthor(input.readString()); + break; + } + case 82: { + obj.setIsbn(input.readString()); + break; + } + case 162: { + obj.setGenre(input.readString()); + break; + } + } + } + } + + public static void toProto(Book obj, CodedOutputStream output) throws IOException { + ExpandableIntArray cache = new ExpandableIntArray(16); + BookProtoConverter.computeSize(obj, cache, 0); + BookProtoConverter.toProto(obj, output, cache, 0); + } + + public static int toProto(Book obj, CodedOutputStream output, ExpandableIntArray cache, int index) throws IOException { + index = index + 1; + if (obj.getName() != null) { + output.writeString(1, obj.getName()); + } + if (obj.getAuthor() != null) { + output.writeString(3, obj.getAuthor()); + } + if (obj.getIsbn() != null) { + output.writeString(10, obj.getIsbn()); + } + if (obj.getGenre() != null) { + output.writeString(20, obj.getGenre()); + } + return index; + } + + public static int computeSize(Book obj) { + ExpandableIntArray cache = new ExpandableIntArray(16); + BookProtoConverter.computeSize(obj, cache, 0); + return cache.get(0); + } + + public static int computeSize(Book obj, ExpandableIntArray cache, final int baseIndex) { + int size = 0; + int index = baseIndex + 1; + if (obj.getName() != null) { + size += CodedOutputStream.computeStringSize(1, obj.getName()); + } + if (obj.getAuthor() != null) { + size += CodedOutputStream.computeStringSize(3, obj.getAuthor()); + } + if (obj.getIsbn() != null) { + size += CodedOutputStream.computeStringSize(10, obj.getIsbn()); + } + if (obj.getGenre() != null) { + size += CodedOutputStream.computeStringSize(20, obj.getGenre()); + } + cache.set(baseIndex, size); + return index; + } + +} diff --git a/vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/PersonProtoConverter.java b/vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/PersonProtoConverter.java new file mode 100644 index 000000000..46002f198 --- /dev/null +++ b/vertx-codegen-protobuf/src/converters/generated/io/vertx/test/codegen/converter/PersonProtoConverter.java @@ -0,0 +1,71 @@ +package io.vertx.test.codegen.converter; + +import com.google.protobuf.CodedOutputStream; +import com.google.protobuf.CodedInputStream; +import java.io.IOException; +import java.time.Instant; +import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.List; +import java.util.HashMap; +import java.util.Map; +import java.util.Arrays; +import io.vertx.core.json.JsonObject; +import io.vertx.codegen.protobuf.utils.ExpandableIntArray; +import io.vertx.codegen.protobuf.converters.*; + +public class PersonProtoConverter { + + public static void fromProto(CodedInputStream input, Person obj) throws IOException { + int tag; + while ((tag = input.readTag()) != 0) { + switch (tag) { + case 18: { + obj.setName(input.readString()); + break; + } + case 32: { + obj.setAge(input.readInt32()); + break; + } + } + } + } + + public static void toProto(Person obj, CodedOutputStream output) throws IOException { + ExpandableIntArray cache = new ExpandableIntArray(16); + PersonProtoConverter.computeSize(obj, cache, 0); + PersonProtoConverter.toProto(obj, output, cache, 0); + } + + public static int toProto(Person obj, CodedOutputStream output, ExpandableIntArray cache, int index) throws IOException { + index = index + 1; + if (obj.getName() != null) { + output.writeString(2, obj.getName()); + } + if (obj.getAge() != 0) { + output.writeInt32(4, obj.getAge()); + } + return index; + } + + public static int computeSize(Person obj) { + ExpandableIntArray cache = new ExpandableIntArray(16); + PersonProtoConverter.computeSize(obj, cache, 0); + return cache.get(0); + } + + public static int computeSize(Person obj, ExpandableIntArray cache, final int baseIndex) { + int size = 0; + int index = baseIndex + 1; + if (obj.getName() != null) { + size += CodedOutputStream.computeStringSize(2, obj.getName()); + } + if (obj.getAge() != 0) { + size += CodedOutputStream.computeInt32Size(4, obj.getAge()); + } + cache.set(baseIndex, size); + return index; + } + +} diff --git a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Address.java b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Address.java index 602df7b72..c67a2a2f9 100644 --- a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Address.java +++ b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Address.java @@ -1,12 +1,13 @@ package io.vertx.test.codegen.converter; import io.vertx.codegen.annotations.DataObject; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; import io.vertx.codegen.protobuf.annotations.ProtobufGen; import java.util.Objects; @DataObject -@ProtobufGen +@ProtobufGen(fieldNumberStrategy = FieldNumberStrategy.COMPACT) public class Address { private String name; private Float longitude; diff --git a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Book.java b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Book.java new file mode 100644 index 000000000..1303500d7 --- /dev/null +++ b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Book.java @@ -0,0 +1,64 @@ +package io.vertx.test.codegen.converter; + +import io.vertx.codegen.annotations.DataObject; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; +import io.vertx.codegen.protobuf.annotations.ProtobufField; +import io.vertx.codegen.protobuf.annotations.ProtobufGen; + +import java.util.Objects; + +@DataObject +@ProtobufGen(fieldNumberStrategy = FieldNumberStrategy.SEGMENTED, reservedFieldNumbers = 2, reservedFieldNames = "title") +public class Book { + private String name; + private String author; + @ProtobufField(10) + private String isbn; + @ProtobufField(20) + private String genre; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getAuthor() { + return author; + } + + public void setAuthor(String author) { + this.author = author; + } + + public String getIsbn() { + return isbn; + } + + public void setIsbn(String isbn) { + this.isbn = isbn; + } + + public String getGenre() { + return genre; + } + + public void setGenre(String genre) { + this.genre = genre; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Book book = (Book) o; + return Objects.equals(name, book.name) && Objects.equals(author, book.author) && Objects.equals(isbn, book.isbn) && Objects.equals(genre, book.genre); + } + + @Override + public int hashCode() { + return Objects.hash(name, author, isbn, genre); + } +} diff --git a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/EnumType.java b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/EnumType.java index 5eb6863eb..871bd052b 100644 --- a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/EnumType.java +++ b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/EnumType.java @@ -1,10 +1,11 @@ package io.vertx.test.codegen.converter; import io.vertx.codegen.annotations.VertxGen; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; import io.vertx.codegen.protobuf.annotations.ProtobufGen; @VertxGen -@ProtobufGen +@ProtobufGen(fieldNumberStrategy = FieldNumberStrategy.COMPACT) public enum EnumType { A, B, diff --git a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Person.java b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Person.java new file mode 100644 index 000000000..4ad8e260b --- /dev/null +++ b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/Person.java @@ -0,0 +1,46 @@ +package io.vertx.test.codegen.converter; + +import io.vertx.codegen.annotations.DataObject; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; +import io.vertx.codegen.protobuf.annotations.ProtobufField; +import io.vertx.codegen.protobuf.annotations.ProtobufGen; + +import java.util.Objects; + +@DataObject +@ProtobufGen(fieldNumberStrategy = FieldNumberStrategy.MANUAL) +public class Person { + @ProtobufField(2) + private String name; + @ProtobufField(4) + private int age; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public int getAge() { + return age; + } + + public void setAge(int age) { + this.age = age; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Person person = (Person) o; + return Objects.equals(name, person.name) && Objects.equals(age, person.age); + } + + @Override + public int hashCode() { + return Objects.hash(name, age); + } +} diff --git a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/RecursiveItem.java b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/RecursiveItem.java index dd1ac5197..6c7484beb 100644 --- a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/RecursiveItem.java +++ b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/RecursiveItem.java @@ -1,12 +1,13 @@ package io.vertx.test.codegen.converter; import io.vertx.codegen.annotations.DataObject; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; import io.vertx.codegen.protobuf.annotations.ProtobufGen; import java.util.Objects; @DataObject -@ProtobufGen +@ProtobufGen(fieldNumberStrategy = FieldNumberStrategy.COMPACT) public class RecursiveItem { private String id; diff --git a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/User.java b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/User.java index 6efbcf7a8..421572091 100644 --- a/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/User.java +++ b/vertx-codegen-protobuf/src/converters/java/io/vertx/test/codegen/converter/User.java @@ -1,6 +1,7 @@ package io.vertx.test.codegen.converter; import io.vertx.codegen.annotations.DataObject; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; import io.vertx.codegen.protobuf.annotations.ProtobufGen; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; @@ -12,7 +13,7 @@ import java.util.Objects; @DataObject -@ProtobufGen +@ProtobufGen(fieldNumberStrategy = FieldNumberStrategy.COMPACT) public class User { private String userName; private Integer age; diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/FieldNumberStrategy.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/FieldNumberStrategy.java new file mode 100644 index 000000000..ff9319475 --- /dev/null +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/FieldNumberStrategy.java @@ -0,0 +1,45 @@ +package io.vertx.codegen.protobuf.annotations; + +public enum FieldNumberStrategy { + /** + * Field numbers must be assigned explicitly (using {@link ProtobufField @ProtobufField}) + * for all properties of the data object. It is an error when two properties are assigned + * the same field number. + *

+ * When removing properties, it is recommended to put their field numbers and names into + * the reserved set to prevent accidentally assigning the same field number + * to a different property in the future. + */ + MANUAL, + /** + * Field numbers are assigned automatically for all properties of the data object. + * First, all properties of the data object that have an explicitly assigned field number + * (using {@link ProtobufField @ProtobufField}) are collected and their field numbers + * are remembered. Then, properties of the data object are iterated in declaration order + * and for each of them that does not have an explicitly assigned field number, a field + * number is assigned, starting from {@code 1} for the first property and increasing + * sequentially, skipping the explicitly assigned field numbers. + *

+ * With this strategy, it is crucial to never remove properties (they must + * be either left in place or added to the reserved set), and always add + * properties at the end, even if they logically belong elsewhere. + */ + COMPACT, + /** + * Field numbers are assigned automatically for all properties of the data object. + * The properties, taken in declaration order, are divided into segments where each + * property with an explicitly assigned field number (using {@link ProtobufField @ProtobufField}) + * starts a new segment. The explicitly assigned field number that starts a segment + * is called the initial field number of the segment. There may be an additional + * segment at the very beginning, if the first property does not have an explicitly + * assigned field number; its initial field number is {@code 1}. Field numbers are + * assigned sequentially in each segment, where the first property of the segment + * has the initial field number of the segment. It is an error if two segments + * have overlapping field numbers. + *

+ * With this strategy, it is crucial to never remove properties (they must + * be either left in place or added to the reserved set), and always add + * properties at the end of the segments, even if they logically belong elsewhere. + */ + SEGMENTED, +} diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufField.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufField.java new file mode 100644 index 000000000..ac90443ba --- /dev/null +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufField.java @@ -0,0 +1,16 @@ +package io.vertx.codegen.protobuf.annotations; + +/** + * Specifies a protobuf field number of a property in a data class that uses {@link ProtobufGen}. + * See {@link ProtobufGen#fieldNumberStrategy()} for more information about how field numbers + * may be assigned. + *

+ * Note that field numbers should never be changed or reused. + * See the Protocol Buffers Language Guide + * for more information. + *

+ * Currently, this annotation is ignored on enum values. That may change in the future. + */ +public @interface ProtobufField { + int value(); +} diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufGen.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufGen.java index a98b6b829..0e785e52b 100644 --- a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufGen.java +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/annotations/ProtobufGen.java @@ -32,4 +32,31 @@ */ public @interface ProtobufGen { JsonProtoEncoding jsonProtoEncoding() default JsonProtoEncoding.VERTX_STRUCT; + + /** + * The strategy of field number assignment. Use {@link FieldNumberStrategy#MANUAL}, + * {@link FieldNumberStrategy#COMPACT}, or {@link FieldNumberStrategy#SEGMENTED}. + *

+ * See the Protocol Buffers Language Guide + * for more information about protobuf schema evolution. + *

+ * Note that this setting is ignored for {@code enum}s, where protobuf enum constants are always + * assigned automatically. Coincidentally, the protobuf enum constants are identical to + * {@linkplain Enum#ordinal() enum ordinals}. Extra care must be taken when changing + * {@code enum}s annotated {@code @ProtobufGen}. In the future, this setting may become + * relevant even for enums, together with {@link ProtobufField @ProtobufField}. + */ + FieldNumberStrategy fieldNumberStrategy(); + + /** + * The set of reserved field numbers. It is an error for a data object property to be explicitly assigned + * a field number from the reserved set. Automatically assigned field numbers will skip the reserved numbers. + */ + int[] reservedFieldNumbers() default {}; + + /** + * The set of reserved field names. It is an error for a data object property to have a name that is + * present in the reserved set. + */ + String[] reservedFieldNames() default {}; } diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/DataObjectProtobufGen.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/DataObjectProtobufGen.java index 99bc6db30..876719f7c 100644 --- a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/DataObjectProtobufGen.java +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/DataObjectProtobufGen.java @@ -3,6 +3,7 @@ import io.vertx.codegen.DataObjectModel; import io.vertx.codegen.Generator; import io.vertx.codegen.PropertyInfo; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; import io.vertx.codegen.protobuf.annotations.JsonProtoEncoding; import io.vertx.codegen.protobuf.annotations.ProtobufGen; import io.vertx.codegen.type.ClassKind; @@ -14,7 +15,9 @@ import java.lang.annotation.Annotation; import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.Set; /** * @author Julien Viet @@ -52,7 +55,10 @@ public String renderProto(DataObjectModel model, int index, int size, Map reservedFieldNumbers = ProtobufGenAnnotation.reservedFieldNumbers(model); + Set reservedFieldNames = ProtobufGenAnnotation.reservedFieldNames(model); writer.print("package " + model.getType().getPackageName() + ";\n"); writer.print("\n"); @@ -76,15 +82,20 @@ public String renderProto(DataObjectModel model, int index, int size, Map properties = model.getPropertyMap().values(); + ProtobufFields.verifyFieldNames(properties, reservedFieldNames); + Map fieldNumbers = ProtobufFields.fieldNumbers(properties, fieldNumberStrategy, reservedFieldNumbers); + List orderedProperties = ProtobufFields.inFieldNumberOrder(properties, fieldNumbers); + // fromProto() { writer.print(" " + visibility + " static void fromProto(CodedInputStream input, " + simpleName + " obj) throws IOException {\n"); writer.print(" int tag;\n"); writer.print(" while ((tag = input.readTag()) != 0) {\n"); writer.print(" switch (tag) {\n"); - int fieldNumber = 1; - for (PropertyInfo prop : model.getPropertyMap().values()) { + for (PropertyInfo prop : orderedProperties) { ClassKind propKind = prop.getType().getKind(); + int fieldNumber = fieldNumbers.get(prop.getName()); ProtoProperty protoProperty = ProtoProperty.getProtoProperty(prop, fieldNumber); writer.print(" case " + protoProperty.getTag() + ": {\n"); if (prop.getType().getKind() == ClassKind.ENUM) { @@ -223,7 +234,6 @@ public String renderProto(DataObjectModel model, int index, int size, Map opMember = model.getAnnotations() - .stream() - .filter(ann -> ann.getName().equals(ProtobufGen.class.getName())) - .findFirst() - .map(ann -> ann.getMember("jsonProtoEncoding")); - - return opMember - .map(v -> JsonProtoEncoding.valueOf((String) v)) - .orElse(JsonProtoEncoding.VERTX_STRUCT); // Default to VERTX_STRUCT - } -} diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoFileGen.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoFileGen.java index b0c309da1..5205aaba2 100644 --- a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoFileGen.java +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoFileGen.java @@ -1,9 +1,7 @@ package io.vertx.codegen.protobuf.generator; import io.vertx.codegen.*; -import io.vertx.codegen.annotations.DataObject; -import io.vertx.codegen.annotations.ModuleGen; -import io.vertx.codegen.annotations.VertxGen; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; import io.vertx.codegen.protobuf.annotations.JsonProtoEncoding; import io.vertx.codegen.protobuf.annotations.ProtobufGen; import io.vertx.codegen.type.ClassKind; @@ -14,7 +12,10 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; public class ProtoFileGen extends Generator { @@ -69,7 +70,10 @@ private String renderDataObjectModel(DataObjectModel model, int index) { StringWriter buffer = new StringWriter(); PrintWriter writer = new PrintWriter(buffer); - JsonProtoEncoding jsonProtoEncoding = JsonProtoEncodingSelector.select(model); + JsonProtoEncoding jsonProtoEncoding = ProtobufGenAnnotation.jsonProtoEncoding(model); + FieldNumberStrategy fieldNumberStrategy = ProtobufGenAnnotation.fieldNumberStrategy(model); + Set reservedFieldNumbers = ProtobufGenAnnotation.reservedFieldNumbers(model); + Set reservedFieldNames = ProtobufGenAnnotation.reservedFieldNames(model); if (index == 0) { writer.print("// Automatically generated by vertx-codegen.\n"); @@ -91,11 +95,34 @@ private String renderDataObjectModel(DataObjectModel model, int index) { } String messageName = model.getType().getSimpleName(); - int fieldNumber = 1; + + Collection properties = model.getPropertyMap().values(); + ProtobufFields.verifyFieldNames(properties, reservedFieldNames); + Map fieldNumbers = ProtobufFields.fieldNumbers(properties, fieldNumberStrategy, reservedFieldNumbers); + List orderedProperties = ProtobufFields.inFieldNumberOrder(properties, fieldNumbers); writer.print("message " + messageName + " {\n"); - for (PropertyInfo prop : model.getPropertyMap().values()) { + + if (!reservedFieldNumbers.isEmpty()) { + writer.print(" reserved "); + writer.print(reservedFieldNumbers.stream() + .sorted() + .map(it -> "" + it) + .collect(Collectors.joining(", "))); + writer.print(";\n"); + } + if (!reservedFieldNames.isEmpty()) { + writer.print(" reserved "); + writer.print(reservedFieldNames.stream() + .sorted() + .map(it -> '"' + it + '"') + .collect(Collectors.joining(", "))); + writer.print(";\n"); + } + + for (PropertyInfo prop : orderedProperties) { ClassKind propKind = prop.getType().getKind(); + int fieldNumber = fieldNumbers.get(prop.getName()); ProtoProperty protoProperty = ProtoProperty.getProtoProperty(prop, fieldNumber); String protoFieldType; @@ -120,7 +147,6 @@ private String renderDataObjectModel(DataObjectModel model, int index) { } else { writer.print(" " + protoFieldType + " " + prop.getName() + " = " + fieldNumber + ";\n"); } - fieldNumber++; } writer.print("}\n"); writer.print("\n"); diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoProperty.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoProperty.java index 505c83d59..f8a2f17ae 100644 --- a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoProperty.java +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtoProperty.java @@ -68,6 +68,8 @@ public static ProtoProperty getProtoProperty(PropertyInfo prop, int fieldNumber) wireType = 2; } + checkFieldNumber(fieldNumber); + int tag = (fieldNumber << 3) | wireType; protoProperty.fieldNumber = fieldNumber; @@ -81,6 +83,16 @@ public static ProtoProperty getProtoProperty(PropertyInfo prop, int fieldNumber) return protoProperty; } + private static void checkFieldNumber(int fieldNumber) { + // see https://protobuf.dev/programming-guides/proto3/#assigning + if (fieldNumber < 1 || fieldNumber > 536_870_911) { + throw new IllegalArgumentException("Field number " + fieldNumber + " is invalid"); + } + if (fieldNumber >= 19_000 && fieldNumber <= 19_999) { + throw new IllegalArgumentException("Field number " + fieldNumber + " is reserved for Protobuf implementations"); + } + } + private static ProtoType determinePrimitiveProtoType(String javaDataType) { if ("java.lang.Integer".equals(javaDataType) || "int".equals(javaDataType)) { return ProtoType.INT32; diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufFields.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufFields.java new file mode 100644 index 000000000..7de463978 --- /dev/null +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufFields.java @@ -0,0 +1,149 @@ +package io.vertx.codegen.protobuf.generator; + +import io.vertx.codegen.PropertyInfo; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; +import io.vertx.codegen.protobuf.annotations.ProtobufField; +import io.vertx.codegen.type.AnnotationValueInfo; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +class ProtobufFields { + private static final String PROTOBUF_FIELD_ANNOTATION_NAME = ProtobufField.class.getName(); + + static void verifyFieldNames(Collection props, Set reservedFieldNames) { + for (PropertyInfo prop : props) { + if (reservedFieldNames.contains(prop.getName())) { + throw new IllegalArgumentException("Field name '" + prop.getName() + "' is reserved"); + } + } + } + + static Map fieldNumbers(Collection props, FieldNumberStrategy fieldNumberStrategy, + Set reservedFieldNumbers) { + switch (fieldNumberStrategy) { + case MANUAL: + return manualFieldNumbers(props, reservedFieldNumbers); + case COMPACT: + return compactFieldNumbers(props, reservedFieldNumbers); + case SEGMENTED: + return segmentedFieldNumbers(props, reservedFieldNumbers); + default: + throw new IllegalArgumentException("Unknown field number strategy: " + fieldNumberStrategy); + } + } + + private static Map manualFieldNumbers(Collection props, Set reservedFieldNumbers) { + Map result = new HashMap<>(); + Map alreadyUsed = new HashMap<>(); + for (PropertyInfo prop : props) { + AnnotationValueInfo ann = prop.getAnnotation(PROTOBUF_FIELD_ANNOTATION_NAME); + if (ann != null) { + Integer fieldNumber = (Integer) ann.getMember("value"); + if (reservedFieldNumbers.contains(fieldNumber)) { + throw new IllegalArgumentException("Property '" + prop.getName() + + "' has a reserved field number " + fieldNumber); + } + if (alreadyUsed.containsKey(fieldNumber)) { + String collidingProperty = alreadyUsed.get(fieldNumber); + throw new IllegalArgumentException("Property '" + prop.getName() + "' is assigned field number " + + fieldNumber + ", which collides with property '" + collidingProperty + "'"); + } + result.put(prop.getName(), fieldNumber); + alreadyUsed.put(fieldNumber, prop.getName()); + } else { + throw new IllegalArgumentException("Property '" + prop.getName() + + "' does not declare a field number; use @ProtobufField"); + } + } + return result; + } + + private static Map compactFieldNumbers(Collection props, Set reservedFieldNumbers) { + Map result = new HashMap<>(); + Map alreadyUsed = new HashMap<>(); + // 1. find pre-allocated field numbers + for (PropertyInfo prop : props) { + AnnotationValueInfo ann = prop.getAnnotation(PROTOBUF_FIELD_ANNOTATION_NAME); + if (ann != null) { + Integer fieldNumber = (Integer) ann.getMember("value"); + if (reservedFieldNumbers.contains(fieldNumber)) { + throw new IllegalArgumentException("Property '" + prop.getName() + + "' has a reserved field number " + fieldNumber); + } + if (alreadyUsed.containsKey(fieldNumber)) { + String collidingProperty = alreadyUsed.get(fieldNumber); + throw new IllegalArgumentException("Property '" + prop.getName() + "' is assigned field number " + + fieldNumber + ", which collides with property '" + collidingProperty + "'"); + } + result.put(prop.getName(), fieldNumber); + alreadyUsed.put(fieldNumber, prop.getName()); + } + } + // 2. allocate field numbers to properties that don't have one pre-allocated + int lastFieldNumber = 1; + for (PropertyInfo prop : props) { + if (result.containsKey(prop.getName())) { + continue; + } + while (alreadyUsed.containsKey(lastFieldNumber) || reservedFieldNumbers.contains(lastFieldNumber)) { + lastFieldNumber++; + } + result.put(prop.getName(), lastFieldNumber++); + } + return result; + } + + private static Map segmentedFieldNumbers(Collection props, Set reservedFieldNumbers) { + Map result = new HashMap<>(); + Map alreadyUsed = new HashMap<>(); + int lastFieldNumber = 0; + for (PropertyInfo prop : props) { + AnnotationValueInfo ann = prop.getAnnotation(PROTOBUF_FIELD_ANNOTATION_NAME); + if (ann != null) { + lastFieldNumber = (Integer) ann.getMember("value"); + if (reservedFieldNumbers.contains(lastFieldNumber)) { + throw new IllegalArgumentException("Property '" + prop.getName() + + "' has a reserved field number " + lastFieldNumber); + } + } else { + lastFieldNumber++; + while (reservedFieldNumbers.contains(lastFieldNumber)) { + lastFieldNumber++; + } + } + if (alreadyUsed.containsKey(lastFieldNumber)) { + String collidingProperty = alreadyUsed.get(lastFieldNumber); + throw new IllegalArgumentException("Property '" + prop.getName() + "' is assigned field number " + + lastFieldNumber + ", which collides with property '" + collidingProperty + "'"); + } + result.put(prop.getName(), lastFieldNumber); + alreadyUsed.put(lastFieldNumber, prop.getName()); + } + return result; + } + + /** + * Returns a list with the same elements as the given collection of properties, except + * the list is ordered so that properties with lower protobuf field number precede + * properties with higher protobuf field number. + * + * @param props a collection of {@link PropertyInfo}s + * @param fieldNumbers field numbers for all properties + * @return a list of the same {@link PropertyInfo}s, in field number order + */ + static List inFieldNumberOrder(Collection props, Map fieldNumbers) { + List result = new ArrayList<>(props); + result.sort((a, b) -> { + Integer ai = fieldNumbers.get(a.getName()); + Integer bi = fieldNumbers.get(b.getName()); + // there should be no `null` values at the moment + return Integer.compare(ai != null ? ai : 0, bi != null ? bi : 0); + }); + return result; + } +} diff --git a/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufGenAnnotation.java b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufGenAnnotation.java new file mode 100644 index 000000000..43eeb4e3a --- /dev/null +++ b/vertx-codegen-protobuf/src/main/java/io/vertx/codegen/protobuf/generator/ProtobufGenAnnotation.java @@ -0,0 +1,44 @@ +package io.vertx.codegen.protobuf.generator; + +import io.vertx.codegen.DataObjectModel; +import io.vertx.codegen.Model; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; +import io.vertx.codegen.protobuf.annotations.JsonProtoEncoding; +import io.vertx.codegen.protobuf.annotations.ProtobufGen; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.Set; + +class ProtobufGenAnnotation { + static JsonProtoEncoding jsonProtoEncoding(DataObjectModel model) { + return model.getAnnotation(ProtobufGen.class) + .map(ann -> ann.getMember("jsonProtoEncoding")) + .map(v -> JsonProtoEncoding.valueOf((String) v)) + .orElse(JsonProtoEncoding.VERTX_STRUCT); // Default to VERTX_STRUCT + } + + static FieldNumberStrategy fieldNumberStrategy(Model model) { + return model.getAnnotation(ProtobufGen.class) + .map(ann -> (String) ann.getMember("fieldNumberStrategy")) + .map(FieldNumberStrategy::valueOf) + .orElseThrow(NoSuchElementException::new); // the annotation member is mandatory, so this should never happen + } + + static Set reservedFieldNumbers(Model model) { + return model.getAnnotation(ProtobufGen.class) + .map(ann -> (List) ann.getMember("reservedFieldNumbers")) + .map(HashSet::new) + .orElseGet(HashSet::new); + } + + static Set reservedFieldNames(Model model) { + return model.getAnnotation(ProtobufGen.class) + .map(ann -> (List) ann.getMember("reservedFieldNames")) + .map(HashSet::new) + .orElseGet(HashSet::new); + } +} diff --git a/vertx-codegen-protobuf/src/test/java/io/vertx/codegen/protobuf/generator/ProtobufFieldsTest.java b/vertx-codegen-protobuf/src/test/java/io/vertx/codegen/protobuf/generator/ProtobufFieldsTest.java new file mode 100644 index 000000000..5a74e3b43 --- /dev/null +++ b/vertx-codegen-protobuf/src/test/java/io/vertx/codegen/protobuf/generator/ProtobufFieldsTest.java @@ -0,0 +1,179 @@ +package io.vertx.codegen.protobuf.generator; + +import io.vertx.codegen.PropertyInfo; +import io.vertx.codegen.PropertyKind; +import io.vertx.codegen.protobuf.annotations.FieldNumberStrategy; +import io.vertx.codegen.protobuf.annotations.ProtobufField; +import io.vertx.codegen.type.AnnotationValueInfo; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class ProtobufFieldsTest { + private static PropertyInfo prop(String name, Integer fieldNumberAnnotation) { + List anns = new ArrayList<>(); + if (fieldNumberAnnotation != null) { + AnnotationValueInfo ann = new AnnotationValueInfo(ProtobufField.class.getName()); + ann.putMember("value", fieldNumberAnnotation); + anns.add(ann); + } + return new PropertyInfo(true, name, null, null, null, null, null, + anns, PropertyKind.VALUE, true, false, null); + } + + @Test + public void verifyFieldNames() { + List props = Arrays.asList( + prop("foo", null), + prop("bar", null)); + + // doesn't throw + ProtobufFields.verifyFieldNames(props, Collections.singleton("baz")); + + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.verifyFieldNames(props, Collections.singleton("foo")); + }); + assertTrue(error.getMessage().contains("is reserved")); + + assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.verifyFieldNames(props, new HashSet<>(Arrays.asList("foo", "bar"))); + }); + assertTrue(error.getMessage().contains("is reserved")); + + assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.verifyFieldNames(props, new HashSet<>(Arrays.asList("foo", "baz"))); + }); + assertTrue(error.getMessage().contains("is reserved")); + } + + @Test + public void manualFieldNumbers() { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers( + Arrays.asList(prop("foo", null)), + FieldNumberStrategy.MANUAL, Collections.emptySet()); + }); + assertTrue(error.getMessage().contains("does not declare a field number")); + + error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers(Arrays.asList( + prop("foo", 1), + prop("bar", 1)), + FieldNumberStrategy.MANUAL, Collections.emptySet()); + }); + assertTrue(error.getMessage().contains("collides with property")); + + error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers( + Arrays.asList(prop("foo", 1)), + FieldNumberStrategy.MANUAL, Collections.singleton(1)); + }); + assertTrue(error.getMessage().contains("has a reserved field number")); + + Map fieldNumbers = ProtobufFields.fieldNumbers( + Arrays.asList( + prop("foo", 1), + prop("bar", 5), + prop("baz", 7), + prop("quux", 3)), + FieldNumberStrategy.MANUAL, Collections.emptySet()); + assertEquals(1, (int) fieldNumbers.get("foo")); + assertEquals(5, (int) fieldNumbers.get("bar")); + assertEquals(7, (int) fieldNumbers.get("baz")); + assertEquals(3, (int) fieldNumbers.get("quux")); + } + + @Test + public void compactFieldNumbers() { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers( + Arrays.asList(prop("foo", 1)), + FieldNumberStrategy.COMPACT, Collections.singleton(1)); + }); + assertTrue(error.getMessage().contains("has a reserved field number")); + + error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers( + Arrays.asList( + prop("foo", 1), + prop("bar", 1)), + FieldNumberStrategy.COMPACT, Collections.emptySet()); + }); + assertTrue(error.getMessage().contains("collides with property")); + + Map fieldNumbers = ProtobufFields.fieldNumbers( + Arrays.asList( + prop("foo", 1), + prop("bar", null), + prop("baz", 3), + prop("quux", null)), + FieldNumberStrategy.COMPACT, Collections.singleton(2)); + assertEquals(1, (int) fieldNumbers.get("foo")); + assertEquals(4, (int) fieldNumbers.get("bar")); + assertEquals(3, (int) fieldNumbers.get("baz")); + assertEquals(5, (int) fieldNumbers.get("quux")); + } + + @Test + public void segmentedFieldNumbers() { + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers( + Arrays.asList(prop("foo", 1)), + FieldNumberStrategy.SEGMENTED, Collections.singleton(1)); + }); + assertTrue(error.getMessage().contains("has a reserved field number")); + + error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers( + Arrays.asList( + prop("foo", 1), + prop("bar", 1)), + FieldNumberStrategy.SEGMENTED, Collections.emptySet()); + }); + assertTrue(error.getMessage().contains("collides with property")); + + error = assertThrows(IllegalArgumentException.class, () -> { + ProtobufFields.fieldNumbers( + Arrays.asList( + prop("foo", null), + prop("bar", null), + prop("baz", null), + prop("quux", 3)), + FieldNumberStrategy.SEGMENTED, Collections.emptySet()); + }); + assertTrue(error.getMessage().contains("collides with property")); + + Map fieldNumbers = ProtobufFields.fieldNumbers( + Arrays.asList( + prop("foo", null), + prop("bar", 5), + prop("baz", 10), + prop("quux", null)), + FieldNumberStrategy.SEGMENTED, Collections.singleton(2)); + assertEquals(1, (int) fieldNumbers.get("foo")); + assertEquals(5, (int) fieldNumbers.get("bar")); + assertEquals(10, (int) fieldNumbers.get("baz")); + assertEquals(11, (int) fieldNumbers.get("quux")); + + fieldNumbers = ProtobufFields.fieldNumbers( + Arrays.asList( + prop("foo", null), + prop("bar", null), + prop("baz", 3), + prop("quux", null)), + FieldNumberStrategy.SEGMENTED, Collections.singleton(4)); + assertEquals(1, (int) fieldNumbers.get("foo")); + assertEquals(2, (int) fieldNumbers.get("bar")); + assertEquals(3, (int) fieldNumbers.get("baz")); + assertEquals(5, (int) fieldNumbers.get("quux")); + } +}