From 6becef8e1d7b01b2b5646459254f920155290f19 Mon Sep 17 00:00:00 2001 From: Lubomir Litchev Date: Fri, 15 Sep 2023 11:37:17 -0700 Subject: [PATCH 01/10] Implement stable hashCode for record fields. Cleaned up some code. Add equals method implementation. Added tests and benchmarks for hashCode and equals This is a fix for issue #95. Signed-off-by: Lubomir Litchev Co-authored-by: Ivan Malygin --- .../kotlin/com.hedera.pbj.root.gradle.kts | 1 - .../com/hedera/pbj/compiler/impl/Common.java | 379 +++++++++++++++++- .../impl/generators/ModelGenerator.java | 113 +++++- .../impl/generators/TestGenerator.java | 31 ++ .../hedera/pbj/runtime/ProtoParserTools.java | 3 +- .../runtime/io/ReadableSequentialData.java | 2 - .../pbj/runtime/io/buffer/BufferedData.java | 12 +- .../hedera/pbj/runtime/io/buffer/Bytes.java | 2 +- .../runtime/io/buffer/RandomAccessData.java | 2 - .../runtime/io/buffer/BufferedDataTest.java | 17 - pbj-integration-tests/build.gradle.kts | 3 + .../intergration/jmh/EqualsHashCodeBench.java | 62 +++ .../pbj/intergration/jmh/HashBench.java | 56 +++ .../src/main/proto/hasheval.proto | 34 ++ .../src/main/proto/timestampTest2.proto | 62 +++ .../pbj/intergration/test/HashEqualsTest.java | 90 +++++ .../intergration/test/TestHashFunctions.java | 116 ++++++ 17 files changed, 951 insertions(+), 34 deletions(-) create mode 100644 pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java create mode 100644 pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java create mode 100644 pbj-integration-tests/src/main/proto/hasheval.proto create mode 100644 pbj-integration-tests/src/main/proto/timestampTest2.proto create mode 100644 pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java create mode 100644 pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java diff --git a/pbj-core/buildSrc/src/main/kotlin/com.hedera.pbj.root.gradle.kts b/pbj-core/buildSrc/src/main/kotlin/com.hedera.pbj.root.gradle.kts index 70752bef..ea7f5733 100644 --- a/pbj-core/buildSrc/src/main/kotlin/com.hedera.pbj.root.gradle.kts +++ b/pbj-core/buildSrc/src/main/kotlin/com.hedera.pbj.root.gradle.kts @@ -16,7 +16,6 @@ plugins { id("com.hedera.pbj.repositories") - id("com.hedera.pbj.aggregate-reports") id("com.hedera.pbj.spotless-conventions") id("com.hedera.pbj.spotless-kotlin-conventions") id("com.autonomousapps.dependency-analysis") diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index b8b6be1f..65cabf44 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -1,11 +1,10 @@ package com.hedera.pbj.compiler.impl; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser; +import org.jetbrains.annotations.NotNull; import java.io.File; -import java.util.Arrays; -import java.util.List; -import java.util.Objects; +import java.util.*; import java.util.stream.Collectors; /** @@ -82,7 +81,7 @@ public static String camelToUpperSnake(String name) { // check if already camel upper if (name.chars().allMatch(c -> Character.isUpperCase(c) || Character.isDigit(c) || c == '_')) return name; // check if already has underscores, then just capitalize - if (name.chars().anyMatch(c -> c == '_')) return name.toUpperCase(); + if (name.contains("_")) return name.toUpperCase(); // else convert final StringBuilder buf = new StringBuilder(); for (int i = 0; i < name.length(); i++) { @@ -172,6 +171,378 @@ public static String javaPrimitiveToObjectType(String primitiveFieldType) { }; } + /** + * Recursively calculates the hashcode for a message fields. + * + * @param fields The fields of this object. + * @param generatedCodeSoFar The accumulated hash code so far. + * @return The generated code for getting the hashCode value. + */ + public static String getFieldsHashCode(final List fields, String generatedCodeSoFar) { + for(Field f : fields) { + if (f.parent() != null) { + final OneOfField oneOfField = f.parent(); + generatedCodeSoFar += getFieldsHashCode(oneOfField.fields(), generatedCodeSoFar); + } + + if (f.optionalValueType()) { + generatedCodeSoFar = getOptionalHashCodeGeneration(generatedCodeSoFar, f); + } + else if (f.repeated()) { + generatedCodeSoFar = getRepeatedHashCodeGeneration(generatedCodeSoFar, f); + } + else if (f != null && f.nameCamelFirstLower() != null) { + if (f.type() == Field.FieldType.FIXED32 || + f.type() == Field.FieldType.INT32 || + f.type() == Field.FieldType.SFIXED32 || + f.type() == Field.FieldType.SINT32 || + f.type() == Field.FieldType.UINT32) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Integer.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.FIXED64 || + f.type() == Field.FieldType.INT64 || + f.type() == Field.FieldType.SFIXED64 || + f.type() == Field.FieldType.SINT64 || + f.type() == Field.FieldType.UINT64) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Long.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.ENUM) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.BOOL) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Boolean.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.FLOAT) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Float.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.DOUBLE) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Double.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.STRING) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.BYTES) { + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } + else if (f.parent() == null) { // process sub message + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != null && $fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } + else { + throw new RuntimeException("Unexpected field type for getting HashCode - " + f.type().toString()); + } + } + } + return generatedCodeSoFar; + } + + /** + * Get the hashcode codegen for a optional field. + * @param generatedCodeSoFar The string that the codegen is generated into. + * @param f The field for which to generate the hash code. + * @return Updated codegen string. + */ + @NotNull + private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, Field f) { + switch (f.messageType()) { + case "StringValue" -> generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "BoolValue" -> generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Boolean.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "Int32Value", "UInt32Value" -> generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Integer.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "Int64Value", "UInt64Value" -> generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Long.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "FloatValue" -> generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Float.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "DoubleValue" -> generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Double.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "BytesValue" -> generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + default -> throw new UnsupportedOperationException("Unhandled optional message type:" + f.messageType()); + } + return generatedCodeSoFar; + } + + /** + * Get the hashcode codegen for a repeated field. + * @param generatedCodeSoFar The string that the codegen is generated into. + * @param f The field for which to generate the hash code. + * @return Updated codegen string. + */ + @NotNull + private static String getRepeatedHashCodeGeneration(String generatedCodeSoFar, Field f) { + generatedCodeSoFar += (FIELD_INDENT + """ + java.util.List list$$fieldName = $fieldName; + for (Object o : list$$fieldName) { + if ($fieldName != DEFAULT.$fieldName) { + if (o != null) { + result = 31 * result; + } + else { + result = 31 * result + o.hashCode(); + } + } + } + """).replace("$fieldName", f.nameCamelFirstLower()); + return generatedCodeSoFar; + } + + /** + * Recursively calculates the hashcode for a message fields. + * + * @param fields The fields of this object. + * @param generatedCodeSoFar The accumulated hash code so far. + * @return The generated code for getting the hashCode value. + */ + public static String getFieldsEqualsStatements(final List fields, String generatedCodeSoFar) { + for(Field f : fields) { + if (f.parent() != null) { + final OneOfField oneOfField = f.parent(); + generatedCodeSoFar += getFieldsEqualsStatements(oneOfField.fields(), generatedCodeSoFar); + } + + if (f.optionalValueType()) { + generatedCodeSoFar = getOptionalEqualsGeneration(generatedCodeSoFar, f); + } + else if (f.repeated()) { + generatedCodeSoFar = getRepeatedEqualsGeneration(generatedCodeSoFar, f); + } + else if (f != null && f.nameCamelFirstLower() != null) { + if (f.type() == Field.FieldType.FIXED32 || + f.type() == Field.FieldType.INT32 || + f.type() == Field.FieldType.SFIXED32 || + f.type() == Field.FieldType.SINT32 || + f.type() == Field.FieldType.UINT32) { + generatedCodeSoFar += """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.FIXED64 || + f.type() == Field.FieldType.INT64 || + f.type() == Field.FieldType.SFIXED64 || + f.type() == Field.FieldType.SINT64 || + f.type() == Field.FieldType.UINT64) { + generatedCodeSoFar += """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.BOOL) { + generatedCodeSoFar += """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.DOUBLE) { + } else if (f.type() == Field.FieldType.FLOAT) { + generatedCodeSoFar += """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.DOUBLE) { + generatedCodeSoFar += """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.STRING || + f.type() == Field.FieldType.BYTES || + f.type() == Field.FieldType.ENUM || + f.parent() == null /* Process a sub-message */) { + generatedCodeSoFar += (""" + if ($fieldName == null && thatObj.$fieldName != null) { + return false; + } + if ($fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } + else { + throw new RuntimeException("Unexpected field type for getting HashCode - " + f.type().toString()); + } + } + } + return generatedCodeSoFar; + } + + /** + * Get the equals codegen for a optional field. + * @param generatedCodeSoFar The string that the codegen is generated into. + * @param f The field for which to generate the equals code. + * @return Updated codegen string. + */ + @NotNull + private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Field f) { + switch (f.messageType()) { + case "StringValue": + generatedCodeSoFar += (FIELD_INDENT + """ + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + break; + case "BoolValue": + generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName instanceof Object) { + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (!$fieldName.equals(thatObj.$fieldName)) { + return false; + } + } + else if ($fieldName != thatObj.$fieldName) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + break; + case "Int32Value", "UInt32Value": generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName instanceof Object) { + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + } + else if ($fieldName != thatObj.$fieldName) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + break; + case "Int64Value", "UInt64Value": generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName instanceof Object) { + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + } + else if ($fieldName != thatObj.$fieldName) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + break; + case "FloatValue": generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName instanceof Object) { + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + } + else if ($fieldName != thatObj.$fieldName) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + break; + case "DoubleValue": generatedCodeSoFar += (FIELD_INDENT + """ + if ($fieldName instanceof Object) { + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + } + else if ($fieldName != thatObj.$fieldName) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + break; + case "BytesValue": generatedCodeSoFar += (FIELD_INDENT + """ + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + break; + default: throw new UnsupportedOperationException("Unhandled optional message type:" + f.messageType()); + } + ; + return generatedCodeSoFar; + } + + /** + * Get the equals codegen for a repeated field. + * @param generatedCodeSoFar The string that the codegen is generated into. + * @param f The field for which to generate the equals code. + * @return Updated codegen string. + */ + @NotNull + private static String getRepeatedEqualsGeneration(String generatedCodeSoFar, Field f) { + generatedCodeSoFar += (""" + if (this.$fieldName == null && this.$fieldName != null) { + return false; + } + + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + return generatedCodeSoFar; + } + /** * Remove leading dot from a string so ".a.b.c" becomes "a.b.c" * diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java index a1a27f00..77600151 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java @@ -260,6 +260,7 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, } // Add here hashCode() for object with a single int field. + boolean hashCodeGenerated = true; if (fields.size() == 1) { FieldType fieldType = fields.get(0).type(); switch (fieldType) { @@ -288,12 +289,122 @@ public int hashCode() { .replace("$fieldName", fields.get(0).name()) .replaceAll("\n","\n"+FIELD_INDENT); } + case FLOAT, DOUBLE -> { + bodyContent += FIELD_INDENT + """ + /** + * Override the default hashCode method for + * single field float and double objects. + */ + @Override + public int hashCode() { + // Shifts: 30, 27, 16, 20, 5, 18, 10, 24, 30 + double x = $fieldName; + x += x << 30; + x ^= x >>> 27; + x += x << 16; + x ^= x >>> 20; + x += x << 5; + x ^= x >>> 18; + x += x << 10; + x ^= x >>> 24; + x += x << 30; + return (int)x; + }""" + .replace("$fieldName", fields.get(0).name()) + .replaceAll("\n", "\n" + FIELD_INDENT); + } + case STRING -> { + bodyContent += FIELD_INDENT + """ + /** + * Override the default hashCode method for + * single field String objects. + */ + @Override + public int hashCode() { + // Shifts: 30, 27, 16, 20, 5, 18, 10, 24, 30 + long x = $fieldName.hashCode(); + x += x << 30; + x ^= x >>> 27; + x += x << 16; + x ^= x >>> 20; + x += x << 5; + x ^= x >>> 18; + x += x << 10; + x ^= x >>> 24; + x += x << 30; + return (int)x; + }""" + .replace("$fieldName", fields.get(0).name()) + .replaceAll("\n", "\n" + FIELD_INDENT); + } default -> { - // Do nothing. + hashCodeGenerated = false; } } + } else { + hashCodeGenerated = false; } + String statements = ""; + if (!hashCodeGenerated) { + // Generate a call to private method that iterates through fields + // and calculates the hashcode. + statements = Common.getFieldsHashCode(fields, statements); + + bodyContent += """ + /** + * Override the default hashCode method for + * all other objects to make hashCode + */ + @Override + public int hashCode() { + int result = 1; + """; + + bodyContent += statements.toString(); + + bodyContent += """ + long hashCode = result; + hashCode += hashCode << 30; + hashCode ^= hashCode >>> 27; + hashCode += hashCode << 16; + hashCode ^= hashCode >>> 20; + hashCode += hashCode << 5; + hashCode ^= hashCode >>> 18; + hashCode += hashCode << 10; + hashCode ^= hashCode >>> 24; + hashCode += hashCode << 30; + return (int)hashCode; + } + """; + bodyContent.replaceAll("\n", "\n" + FIELD_INDENT); + } + + String equalsStatements = new String(); + // Generate a call to private method that iterates through fields + // and calculates the hashcode. + equalsStatements = Common.getFieldsEqualsStatements(fields, equalsStatements); + + bodyContent += FIELD_INDENT + """ + /** + * Override the default equals method for + */ + @Override + public boolean equals(Object that) { + if (that == null || this.getClass() != that.getClass()) { + return false; + } + + $javaRecordName thatObj = ($javaRecordName)that; + + """.replace("$javaRecordName", javaRecordName); + + bodyContent += FIELD_INDENT + FIELD_INDENT + equalsStatements.toString();; + bodyContent += """ + return true; + } + """; + // Has methods bodyContent += String.join("\n", hasMethods).replaceAll("\n","\n"+FIELD_INDENT); bodyContent += "\n"; diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java index 57267120..de669bc5 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java @@ -327,6 +327,37 @@ private static String generateTestMethod(final String modelClassName, final Stri final $modelClassName jsonReadPbj = $modelClassName.JSON.parse(JsonTools.parseJson(charBuffer), false); assertEquals(modelObj, jsonReadPbj); } + // Very slow for now. Too much garbage generated to enable in general case. +// // Test hashCode and equals() +// Stream> objects = createModelTestArguments(); +// Object[] objArray = objects.toArray(); +// for (int i = 0; i < objArray.length; i++) { +// for (int j = i; j < objArray.length; j++) { +// if (objArray[i].hashCode() != objArray[i].hashCode()) { +// fail("Same object, different hash."); +// } +// if (objArray[j].hashCode() != objArray[j].hashCode()) { +// fail("Same object, different hash 1."); +// } +// if (objArray[i].hashCode() == objArray[j].hashCode()) { +// if (!objArray[i].equals(objArray[j])) { +// fail("equalsHash, different objects."); +// } +// } +// } +// } +// +// Map map = new HashMap<>(); +// map.put(objArray[0], objArray[0]); +// for (int i = 1; i < objArray.length; i++) { +// Object o = map.put(objArray[i], objArray[i]); +// if (o != null) { +// Object existing = map.get(objArray[i]); +// assertEquals(existing.hashCode(), objArray[i].hashCode()); +// assertEquals(existing, objArray[i]); +// } +// } + """ .replace("$modelClassName",modelClassName) .replace("$protocModelClass",protoCJavaFullQualifiedClass) diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java index 826e039c..5e352034 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/ProtoParserTools.java @@ -225,7 +225,8 @@ public static String readString(final ReadableSequentialData input) throws IOExc * Read a Bytes field from data input * * @param input the input to read from - * @return read Bytes object, this will be a copy of the data + * @return read Bytes object, this can be a copy or a direct reference to inputs data. So it has same life span + * of InputData * @throws IOException If there was a problem reading */ public static Bytes readBytes(final ReadableSequentialData input) throws IOException { diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java index eb0323a4..286cafb1 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/ReadableSequentialData.java @@ -180,8 +180,6 @@ default long readBytes(@NonNull final BufferedData dst) { * the read data. The returned bytes will be immutable. The {@link #position()} of this sequence will be * incremented by {@code length} bytes. * - *

All data returned will always be a copy otherwise the underlying data could be modified. - * *

Bytes are read from the sequence one at a time. If there are not {@code length} bytes remaining in this * sequence, then a {@link BufferUnderflowException} will be thrown. The {@link #position()} will be * incremented by the number of bytes read prior to the exception. diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java index 19bae4bd..65e810f0 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java @@ -372,11 +372,13 @@ public long getBytes(final long offset, @NonNull final BufferedData dst) { @Override public Bytes getBytes(long offset, long length) { final var len = Math.toIntExact(length); - if(len < 0) throw new IllegalArgumentException("Length cannot be negative"); - // It is vital that we always copy here, we can never assume ownership of the underlying buffer - final var copy = new byte[len]; - buffer.get(Math.toIntExact(offset), copy, 0, len); - return Bytes.wrap(copy); + if (direct) { + final var copy = new byte[len]; + buffer.get(Math.toIntExact(offset), copy, 0, len); + return Bytes.wrap(copy); + } else { + return Bytes.wrap(buffer.array(), Math.toIntExact(buffer.arrayOffset() + offset), len); + } } /** {@inheritDoc} */ diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java index 8c03c41f..bf9c03f6 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java @@ -355,7 +355,7 @@ public int read() throws IOException { @Override @NonNull public String toString() { - return HexFormat.of().formatHex(buffer, start, start + length); + return HexFormat.of().formatHex(buffer, start, length); } /** diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java index ec2914fb..f69bd8b9 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/RandomAccessData.java @@ -142,8 +142,6 @@ default long getBytes(final long offset, @NonNull final BufferedData dst) { * Get {@code length} bytes starting at the given {@code offset} from this buffer. The returned bytes will * be immutable. The returned {@link Bytes} will have exactly {@code length} bytes. * - *

All data returned will always be a copy otherwise the underlying data could be modified. - * * @param offset The offset into data to begin reading bytes * @param length The non-negative length in bytes to read * @return new {@link Bytes} containing the read data diff --git a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java index c6f5fb0b..846e26a0 100644 --- a/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java +++ b/pbj-core/pbj-runtime/src/test/java/com/hedera/pbj/runtime/io/buffer/BufferedDataTest.java @@ -1,6 +1,5 @@ package com.hedera.pbj.runtime.io.buffer; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import com.hedera.pbj.runtime.io.ReadableSequentialData; @@ -8,7 +7,6 @@ import com.hedera.pbj.runtime.io.WritableSequentialData; import com.hedera.pbj.runtime.io.WritableTestBase; import edu.umd.cs.findbugs.annotations.NonNull; -import java.util.Arrays; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -95,21 +93,6 @@ void sliceThenReadBytes() { assertEquals((byte) (i + START), slicedBytes.getByte(i)); } } - @Test - @DisplayName("readBytes() does always copy") - void readBytesCopies() { - byte[] bytes = new byte[] {1, 2, 3, 4, 5}; - byte[] bytesOrig = bytes.clone(); - final var buf = BufferedData.wrap(bytes); - final Bytes readBytes = buf.readBytes(5); - assertArrayEquals(bytesOrig, readBytes.toByteArray()); - bytes[0] = 127; - bytes[1] = 127; - bytes[2] = 127; - bytes[3] = 127; - bytes[4] = 127; - assertArrayEquals(bytesOrig, readBytes.toByteArray()); - } @Nested final class ReadableSequentialDataTest extends ReadableTestBase { diff --git a/pbj-integration-tests/build.gradle.kts b/pbj-integration-tests/build.gradle.kts index b56abcab..4132643e 100644 --- a/pbj-integration-tests/build.gradle.kts +++ b/pbj-integration-tests/build.gradle.kts @@ -82,6 +82,9 @@ jmh { // includes.add("AccountDetailsBench") // includes.add("JsonBench") // includes.add("VarIntBench") +// includes.add("HashBench") +// includes.add("EqualsHashCodeBench"); + jmhVersion.set("1.35") includeTests.set(true) // jvmArgsAppend.add("-XX:MaxInlineSize=100 -XX:MaxInlineLevel=20") diff --git a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java new file mode 100644 index 00000000..66584db7 --- /dev/null +++ b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java @@ -0,0 +1,62 @@ +package com.hedera.pbj.intergration.jmh; + +import com.google.protobuf.CodedInputStream; +import com.google.protobuf.CodedOutputStream; +import com.hedera.pbj.intergration.test.TestHashFunctions; +import com.hedera.pbj.runtime.MalformedProtobufException; +import com.hedera.pbj.runtime.io.buffer.BufferedData; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import com.hedera.pbj.runtime.io.stream.ReadableStreamingData; +import com.hedera.pbj.test.proto.pbj.Hasheval; +import com.hedera.pbj.test.proto.pbj.Suit; +import com.hedera.pbj.test.proto.pbj.TimestampTest; +import com.hedera.pbj.test.proto.pbj.tests.HashevalTest; +import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.infra.Blackhole; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.*; +import java.util.Random; +import java.util.concurrent.TimeUnit; + +@SuppressWarnings("unused") +@State(Scope.Benchmark) +@Fork(1) +@Warmup(iterations = 4, time = 2) +@Measurement(iterations = 5, time = 2) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@BenchmarkMode(Mode.AverageTime) +public class EqualsHashCodeBench { + private TimestampTest testStamp; + private TimestampTest testStamp1; + + public EqualsHashCodeBench() { + testStamp = new TimestampTest(987L, 123); + testStamp1 = new TimestampTest(987L, 122); + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchHashCode(Blackhole blackhole) throws IOException { + for (int i = 0; i < 1050; i++) { + testStamp.hashCode(); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchEquals(Blackhole blackhole) throws IOException { + for (int i = 0; i < 1050; i++) { + testStamp.equals(testStamp); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchNotEquals(Blackhole blackhole) throws IOException { + for (int i = 0; i < 1050; i++) { + testStamp.equals(testStamp1); + } + } +} \ No newline at end of file diff --git a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java new file mode 100644 index 00000000..beb3970a --- /dev/null +++ b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java @@ -0,0 +1,56 @@ +package com.hedera.pbj.intergration.jmh; + +import com.google.protobuf.CodedInputStream; +import com.google.protobuf.CodedOutputStream; +import com.hedera.pbj.intergration.test.TestHashFunctions; +import com.hedera.pbj.runtime.MalformedProtobufException; +import com.hedera.pbj.runtime.io.buffer.BufferedData; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import com.hedera.pbj.runtime.io.stream.ReadableStreamingData; +import com.hedera.pbj.test.proto.pbj.Hasheval; +import com.hedera.pbj.test.proto.pbj.Suit; +import com.hedera.pbj.test.proto.pbj.TimestampTest; +import com.hedera.pbj.test.proto.pbj.tests.HashevalTest; +import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.infra.Blackhole; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.*; +import java.util.Random; +import java.util.concurrent.TimeUnit; + +@SuppressWarnings("unused") +@State(Scope.Benchmark) +@Fork(1) +@Warmup(iterations = 4, time = 2) +@Measurement(iterations = 5, time = 2) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@BenchmarkMode(Mode.AverageTime) +public class HashBench { + private Hasheval hasheval; + + public HashBench() { + TimestampTest tst = new TimestampTest(987L, 123); + hasheval = new Hasheval(1, -1, 2, 3, -2, + 123f, 7L, -7L, 123L, 234L, + -345L, 456.789D, true, Suit.ACES, tst, "FooBarKKKKHHHHOIOIOI", + Bytes.wrap(new byte[]{1, 2, 3, 4, 5, 6, 7, (byte)255})); + } + + @Benchmark + @OperationsPerInvocation(1050) + public void hashBenchSHA256(Blackhole blackhole) throws IOException { + for (int i = 0; i < 1050; i++) { + TestHashFunctions.hash1(hasheval); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void hashBenchFieldWise(Blackhole blackhole) throws IOException { + for (int i = 0; i < 1050; i++) { + TestHashFunctions.hash2(hasheval); + } + } +} \ No newline at end of file diff --git a/pbj-integration-tests/src/main/proto/hasheval.proto b/pbj-integration-tests/src/main/proto/hasheval.proto new file mode 100644 index 00000000..0d9b01f7 --- /dev/null +++ b/pbj-integration-tests/src/main/proto/hasheval.proto @@ -0,0 +1,34 @@ +syntax = "proto3"; + +package proto; + +option java_package = "com.hedera.pbj.test.proto.java"; +option java_multiple_files = true; +// <<>> This comment is special code for setting PBJ Compiler java package + +import "timestampTest.proto"; +import "google/protobuf/wrappers.proto"; +import "everything.proto"; + +/** + * Example protobuf containing examples of all types + */ +message Hasheval { + int32 int32Number = 1; + sint32 sint32Number = 2; + uint32 uint32Number = 3; + fixed32 fixed32Number = 4; + sfixed32 sfixed32Number = 5; + float floatNumber = 6; + int64 int64Number = 7; + sint64 sint64Number = 8; + uint64 uint64Number = 9; + fixed64 fixed64Number = 10; + sfixed64 sfixed64Number = 11; + double doubleNumber = 12; + bool booleanField = 13; + Suit enumSuit = 14; + TimestampTest subObject = 15; + string text = 16; + bytes bytesField = 17; +} diff --git a/pbj-integration-tests/src/main/proto/timestampTest2.proto b/pbj-integration-tests/src/main/proto/timestampTest2.proto new file mode 100644 index 00000000..1eb2c3da --- /dev/null +++ b/pbj-integration-tests/src/main/proto/timestampTest2.proto @@ -0,0 +1,62 @@ +syntax = "proto3"; + +package proto; + +/** Test issue 87 */ +/** Test issue 87 */ + +/*- + * ‌ + * Hedera Network Services Protobuf + * ​ + * Copyright (C) 2018 - 2021 Hedera Hashgraph, LLC + * ​ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ‍ + */ + +option java_package = "com.hedera.pbj.test.proto.java"; +option java_multiple_files = true; +// <<>> This comment is special code for setting PBJ Compiler java package + +/** + * An exact date and time. This is the same data structure as the protobuf Timestamp.proto (see the + * comments in https://github.com/google/protobuf/blob/master/src/google/protobuf/timestamp.proto) + */ +message TimestampTest2 { + /** + * Number of complete seconds since the start of the epoch + */ + int64 seconds = 1; + + /** + * Number of nanoseconds since the start of the last second + */ + int32 nanos = 2; + + /** + * Number of picoseconds since the start of the last nanosecond + */ + int32 pico = 3; +} + +/** + * An exact date and time, with a resolution of one second (no nanoseconds). + */ +message TimestampTestSeconds2 { + /** + * Number of complete seconds since the start of the epoch + */ + int64 seconds = 1; +} + diff --git a/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java new file mode 100644 index 00000000..71bf02db --- /dev/null +++ b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java @@ -0,0 +1,90 @@ +package com.hedera.pbj.intergration.test; + +import org.junit.jupiter.api.Test; + +import com.hedera.pbj.test.proto.pbj.TimestampTest; +import com.hedera.pbj.test.proto.pbj.TimestampTest2; + +import static org.junit.jupiter.api.Assertions.*; + +class HasjhEqualsTest { + @Test + void differentObjectsWithDefaulEquals() { + TimestampTest tst = new TimestampTest(1, 2); + TimestampTest2 tst2 = new TimestampTest2(1, 2, 0); + + assertFalse(tst.equals(tst2)); + } + + @Test + void sameObjectsWithNoDefaulEquals() { + TimestampTest tst = new TimestampTest(3, 4); + TimestampTest tst1 = new TimestampTest(3, 4); + + assertEquals(tst, tst1); + } + + @Test + void sameObjectsWithDefaulNoEquals() { + TimestampTest tst = new TimestampTest(3, 4); + TimestampTest tst1 = new TimestampTest(3, 5); + + assertNotEquals(tst, tst1); + } + + @Test + void sameObjectsWithDefaulEquals() { + TimestampTest tst = new TimestampTest(0, 0); + TimestampTest tst1 = new TimestampTest(0, 0); + + assertEquals(tst, tst1); + } + + @Test + void differentObjectsWithDefaulHashCode() { + TimestampTest tst = new TimestampTest(0, 0); + TimestampTest2 tst2 = new TimestampTest2(0, 0, 0); + + assertEquals(tst.hashCode(), tst2.hashCode()); + } + + @Test + void differentObjectsWithNoDefaulHashCode() { + TimestampTest tst = new TimestampTest(1, 0); + TimestampTest2 tst2 = new TimestampTest2(1, 0, 0); + + assertEquals(tst.hashCode(), tst2.hashCode()); + } + + @Test + void differentObjectsWithNoDefaulHashCode1() { + TimestampTest tst = new TimestampTest(0, 0); + TimestampTest2 tst2 = new TimestampTest2(0, 0, 3); + + assertNotEquals(tst.hashCode(), tst2.hashCode()); + } + + @Test + void differentObjectsWithNoDefaulHashCode2() { + TimestampTest2 tst = new TimestampTest2(0, 0, 0); + TimestampTest2 tst2 = new TimestampTest2(0, 0, 0); + + assertEquals(tst.hashCode(), tst2.hashCode()); + } + + @Test + void differentObjectsWithNoDefaulHashCode3() { + TimestampTest2 tst = new TimestampTest2(1, 2, 3); + TimestampTest2 tst2 = new TimestampTest2(1, 2, 3); + + assertEquals(tst.hashCode(), tst2.hashCode()); + } + + @Test + void differentObjectsWithNoDefaulHashCode4() { + TimestampTest2 tst = new TimestampTest2(1, 4, 3); + TimestampTest2 tst2 = new TimestampTest2(1, 2, 3); + + assertNotEquals(tst.hashCode(), tst2.hashCode()); + } +} \ No newline at end of file diff --git a/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java new file mode 100644 index 00000000..3d81b383 --- /dev/null +++ b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java @@ -0,0 +1,116 @@ +package com.hedera.pbj.intergration.test; + +import com.google.protobuf.CodedOutputStream; +import com.hedera.hapi.node.base.Timestamp; +import com.hedera.pbj.runtime.io.buffer.BufferedData; +import com.hedera.pbj.runtime.test.NoToStringWrapper; +import com.hedera.pbj.test.proto.pbj.Hasheval; +import com.hedera.pbj.test.proto.pbj.TimestampTest; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import java.nio.ByteBuffer; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.List; +import java.util.stream.IntStream; +import java.util.stream.Stream; +import static com.hedera.pbj.runtime.ProtoTestTools.INTEGER_TESTS_LIST; +import static com.hedera.pbj.runtime.ProtoTestTools.LONG_TESTS_LIST; +import static com.hedera.pbj.runtime.ProtoTestTools.getThreadLocalByteBuffer; +import static com.hedera.pbj.runtime.ProtoTestTools.getThreadLocalDataBuffer; +import static com.hedera.pbj.runtime.ProtoTestTools.getThreadLocalDataBuffer2; +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Unit Test for TimestampTest model object. Generate based on protobuf schema. + */ +public final class TestHashFunctions { + public static int hash1(Hasheval hashEval) { + try { + byte[] hash = MessageDigest.getInstance("SHA-256").digest( + Hasheval.PROTOBUF.toBytes(hashEval).toByteArray()); + int res = hash[0] << 24 | hash[1] << 16 | hash[2] << 8 | hash[3]; + return processForBetterDistribution(res); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } + } + + public static int hash2(Hasheval hashEval) { + if (hashEval == null) return 0; + + int result = 1; + if (hashEval.int32Number() != Hasheval.DEFAULT.int32Number()) { + result = 31 * result + Integer.hashCode(hashEval.int32Number()); + } + if (hashEval.sint32Number() != Hasheval.DEFAULT.sint32Number()) { + result = 31 * result + Integer.hashCode(hashEval.sint32Number()); + } + if (hashEval.uint32Number() != Hasheval.DEFAULT.uint32Number()) { + result = 31 * result + Integer.hashCode(hashEval.uint32Number()); + } + if (hashEval.fixed32Number() != Hasheval.DEFAULT.fixed32Number()) { + result = 31 * result + Integer.hashCode(hashEval.fixed32Number()); + } + if (hashEval.sfixed32Number() != Hasheval.DEFAULT.sfixed32Number()) { + result = 31 * result + Integer.hashCode(hashEval.sfixed32Number()); + } + if (hashEval.floatNumber() != Hasheval.DEFAULT.floatNumber()) { + result = 31 * result + Float.hashCode(hashEval.floatNumber()); + } + if (hashEval.int64Number() != Hasheval.DEFAULT.int64Number()) { + result = 31 * result + Long.hashCode(hashEval.int64Number()); + } + if (hashEval.sint64Number() != Hasheval.DEFAULT.sint64Number()) { + result = 31 * result + Long.hashCode(hashEval.sint64Number()); + } + if (hashEval.uint64Number() != Hasheval.DEFAULT.uint64Number()) { + result = 31 * result + Long.hashCode(hashEval.uint64Number()); + } + if (hashEval.fixed64Number() != Hasheval.DEFAULT.fixed64Number()) { + result = 31 * result + Long.hashCode(hashEval.fixed64Number()); + } + if (hashEval.sfixed64Number() != Hasheval.DEFAULT.sfixed64Number()) { + result = 31 * result + Long.hashCode(hashEval.sfixed64Number()); + } + if (hashEval.doubleNumber() != Hasheval.DEFAULT.doubleNumber()) { + result = 31 * result + Double.hashCode(hashEval.doubleNumber()); + } + if (hashEval.booleanField() != Hasheval.DEFAULT.booleanField()) { + result = 31 * result + Boolean.hashCode(hashEval.booleanField()); + } + if (hashEval.enumSuit() != Hasheval.DEFAULT.enumSuit()) { + result = 31 * result + hashEval.enumSuit().hashCode(); + } + if (hashEval.subObject() != Hasheval.DEFAULT.subObject()) { + TimestampTest sub = hashEval.subObject(); + if (sub.nanos() != sub.DEFAULT.nanos()) { + result = 31 * result + Integer.hashCode(sub.nanos()); + } + if (sub.seconds() != sub.DEFAULT.seconds()) { + result = 31 * result + Long.hashCode(sub.seconds()); + } + } + if (hashEval.text() != Hasheval.DEFAULT.text()) { + result = 31 * result + hashEval.text().hashCode(); + } + if (hashEval.bytesField() != Hasheval.DEFAULT.bytesField()) { + result = 31 * result + (hashEval.bytesField() == null ? 0 : hashEval.bytesField().hashCode()); + } + + return processForBetterDistribution(result); + } + + private static int processForBetterDistribution(int val) { + val += val << 30; + val ^= val >>> 27; + val += val << 16; + val ^= val >>> 20; + val += val << 5; + val ^= val >>> 18; + val += val << 10; + val ^= val >>> 24; + val += val << 30; + return val; + } +} \ No newline at end of file From ee7e91e019c343c6694e77748a92c26e055e780c Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Tue, 7 Nov 2023 11:50:59 -0500 Subject: [PATCH 02/10] Clean up Signed-off-by: Ivan Malygin --- .../com/hedera/pbj/compiler/impl/Common.java | 22 ++++++++++--------- .../impl/generators/ModelGenerator.java | 16 +++++++------- .../json/JsonCodecWriteMethodGenerator.java | 4 ++-- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index 65cabf44..6768e8d9 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -1,10 +1,12 @@ package com.hedera.pbj.compiler.impl; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser; -import org.jetbrains.annotations.NotNull; +import edu.umd.cs.findbugs.annotations.NonNull; import java.io.File; -import java.util.*; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; /** @@ -46,7 +48,7 @@ public static int getTag(final int wireType, final int fieldNumber) { * @return name with first character converted to upper case */ public static String capitalizeFirstLetter(String name) { - if (name.length() > 0) { + if (!name.isEmpty()) { if (name.chars().allMatch(Character::isUpperCase)) { return Character.toUpperCase(name.charAt(0)) + name.substring(1).toLowerCase(); } else { @@ -191,7 +193,7 @@ public static String getFieldsHashCode(final List fields, String generate else if (f.repeated()) { generatedCodeSoFar = getRepeatedHashCodeGeneration(generatedCodeSoFar, f); } - else if (f != null && f.nameCamelFirstLower() != null) { + else if (f.nameCamelFirstLower() != null) { if (f.type() == Field.FieldType.FIXED32 || f.type() == Field.FieldType.INT32 || f.type() == Field.FieldType.SFIXED32 || @@ -270,7 +272,7 @@ else if (f.parent() == null) { // process sub message * @param f The field for which to generate the hash code. * @return Updated codegen string. */ - @NotNull + @NonNull private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, Field f) { switch (f.messageType()) { case "StringValue" -> generatedCodeSoFar += (FIELD_INDENT + """ @@ -319,7 +321,7 @@ private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, F * @param f The field for which to generate the hash code. * @return Updated codegen string. */ - @NotNull + @NonNull private static String getRepeatedHashCodeGeneration(String generatedCodeSoFar, Field f) { generatedCodeSoFar += (FIELD_INDENT + """ java.util.List list$$fieldName = $fieldName; @@ -357,7 +359,7 @@ public static String getFieldsEqualsStatements(final List fields, String else if (f.repeated()) { generatedCodeSoFar = getRepeatedEqualsGeneration(generatedCodeSoFar, f); } - else if (f != null && f.nameCamelFirstLower() != null) { + else if (f.nameCamelFirstLower() != null) { if (f.type() == Field.FieldType.FIXED32 || f.type() == Field.FieldType.INT32 || f.type() == Field.FieldType.SFIXED32 || @@ -424,7 +426,7 @@ else if (f != null && f.nameCamelFirstLower() != null) { * @param f The field for which to generate the equals code. * @return Updated codegen string. */ - @NotNull + @NonNull private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Field f) { switch (f.messageType()) { case "StringValue": @@ -529,7 +531,7 @@ else if ($fieldName != thatObj.$fieldName) { * @param f The field for which to generate the equals code. * @return Updated codegen string. */ - @NotNull + @NonNull private static String getRepeatedEqualsGeneration(String generatedCodeSoFar, Field f) { generatedCodeSoFar += (""" if (this.$fieldName == null && this.$fieldName != null) { @@ -550,7 +552,7 @@ private static String getRepeatedEqualsGeneration(String generatedCodeSoFar, Fie * @return text without a leading dot */ public static String removingLeadingDot(String text) { - if (text.length() > 0 & text.charAt(0) == '.') { + if (!text.isEmpty() & text.charAt(0) == '.') { return text.substring(1); } return text; diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java index 77600151..f721c90a 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java @@ -3,6 +3,7 @@ import com.hedera.pbj.compiler.impl.*; import com.hedera.pbj.compiler.impl.Field.FieldType; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser; +import edu.umd.cs.findbugs.annotations.SuppressWarnings; import java.io.File; import java.io.FileWriter; @@ -203,7 +204,7 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, // process field java doc and insert into record java doc if (!fields.isEmpty()) { - String recordJavaDoc = javaDocComment.length() > 0 ? + String recordJavaDoc = !javaDocComment.isEmpty() ? javaDocComment.replaceAll("\n\s*\\*/","") : "/**\n * "+javaRecordName; recordJavaDoc += "\n *"; @@ -259,7 +260,6 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, ).replaceAll("\n","\n"+FIELD_INDENT); } - // Add here hashCode() for object with a single int field. boolean hashCodeGenerated = true; if (fields.size() == 1) { FieldType fieldType = fields.get(0).type(); @@ -354,14 +354,14 @@ public int hashCode() { bodyContent += """ /** * Override the default hashCode method for - * all other objects to make hashCode + * all other objects to make hashCode */ @Override public int hashCode() { int result = 1; """; - bodyContent += statements.toString(); + bodyContent += statements; bodyContent += """ long hashCode = result; @@ -377,10 +377,10 @@ public int hashCode() { return (int)hashCode; } """; - bodyContent.replaceAll("\n", "\n" + FIELD_INDENT); + bodyContent = bodyContent.replaceAll("\n", "\n" + FIELD_INDENT); } - String equalsStatements = new String(); + String equalsStatements = ""; // Generate a call to private method that iterates through fields // and calculates the hashcode. equalsStatements = Common.getFieldsEqualsStatements(fields, equalsStatements); @@ -392,14 +392,14 @@ public int hashCode() { @Override public boolean equals(Object that) { if (that == null || this.getClass() != that.getClass()) { - return false; + return false; } $javaRecordName thatObj = ($javaRecordName)that; """.replace("$javaRecordName", javaRecordName); - bodyContent += FIELD_INDENT + FIELD_INDENT + equalsStatements.toString();; + bodyContent += FIELD_INDENT + FIELD_INDENT + equalsStatements; bodyContent += """ return true; } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java index c02df85e..4543be88 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java @@ -4,7 +4,7 @@ import com.hedera.pbj.compiler.impl.Field; import com.hedera.pbj.compiler.impl.OneOfField; import com.hedera.pbj.compiler.impl.SingleField; -import org.jetbrains.annotations.NotNull; +import edu.umd.cs.findbugs.annotations.NonNull; import java.util.Comparator; import java.util.List; @@ -97,7 +97,7 @@ private static String generateFieldWriteLines(final Field field, final String mo } } - @NotNull + @NonNull private static String generateBasicFieldLines(Field field, String getValueCode, String fieldDef, String fieldName) { if(field.optionalValueType()) { return switch (field.messageType()) { From abb45747348bb6e6e78a789e827c97c2650c1357 Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Thu, 9 Nov 2023 17:32:00 -0500 Subject: [PATCH 03/10] Fixed generated code indentation. Removed code duplication. Replaced adding spaced with `.indent` calls. Signed-off-by: Ivan Malygin --- .../com/hedera/pbj/compiler/impl/Common.java | 372 +++++++++--------- .../impl/generators/EnumGenerator.java | 36 +- .../impl/generators/ModelGenerator.java | 348 ++++++++-------- .../impl/generators/TestGenerator.java | 22 +- .../json/JsonCodecParseMethodGenerator.java | 10 +- .../json/JsonCodecWriteMethodGenerator.java | 9 +- .../CodecFastEqualsMethodGenerator.java | 5 +- .../CodecMeasureDataMethodGenerator.java | 5 +- .../CodecMeasureRecordMethodGenerator.java | 11 +- .../protobuf/CodecParseMethodGenerator.java | 21 +- .../protobuf/CodecWriteMethodGenerator.java | 11 +- .../pbj/intergration/test/HashEqualsTest.java | 8 +- 12 files changed, 423 insertions(+), 435 deletions(-) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index 6768e8d9..d6e39131 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -17,6 +17,7 @@ public final class Common { /** The indent for fields, default 4 spaces */ public static final String FIELD_INDENT = " ".repeat(4); + public static final int DEFAULT_INDENT = 4; /** Number of bits used to represent the tag type */ static final int TAG_TYPE_BITS = 3; @@ -199,71 +200,80 @@ else if (f.nameCamelFirstLower() != null) { f.type() == Field.FieldType.SFIXED32 || f.type() == Field.FieldType.SINT32 || f.type() == Field.FieldType.UINT32) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Integer.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Integer.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FIXED64 || f.type() == Field.FieldType.INT64 || f.type() == Field.FieldType.SFIXED64 || f.type() == Field.FieldType.SINT64 || f.type() == Field.FieldType.UINT64) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Long.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Long.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.ENUM) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BOOL) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Boolean.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Boolean.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FLOAT) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Float.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Float.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.DOUBLE) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Double.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Double.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.STRING) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BYTES) { - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.parent() == null) { // process sub message - generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != null && $fieldName != DEFAULT.$fieldName) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName != null && $fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else { throw new RuntimeException("Unexpected field type for getting HashCode - " + f.type().toString()); } } } - return generatedCodeSoFar; + return generatedCodeSoFar.indent(DEFAULT_INDENT * 2); } /** @@ -275,41 +285,48 @@ else if (f.parent() == null) { // process sub message @NonNull private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, Field f) { switch (f.messageType()) { - case "StringValue" -> generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - case "BoolValue" -> generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Boolean.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - case "Int32Value", "UInt32Value" -> generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Integer.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - case "Int64Value", "UInt64Value" -> generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Long.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - case "FloatValue" -> generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Float.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - case "DoubleValue" -> generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Double.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - case "BytesValue" -> generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); - } - """).replace("$fieldName", f.nameCamelFirstLower()); + case "StringValue" -> generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "BoolValue" -> generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Boolean.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "Int32Value", "UInt32Value" -> generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Integer.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "Int64Value", "UInt64Value" -> generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Long.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "FloatValue" -> generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Float.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "DoubleValue" -> generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Double.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + case "BytesValue" -> generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + } + """).replace("$fieldName", f.nameCamelFirstLower()); default -> throw new UnsupportedOperationException("Unhandled optional message type:" + f.messageType()); } return generatedCodeSoFar; @@ -323,19 +340,19 @@ private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, F */ @NonNull private static String getRepeatedHashCodeGeneration(String generatedCodeSoFar, Field f) { - generatedCodeSoFar += (FIELD_INDENT + """ - java.util.List list$$fieldName = $fieldName; - for (Object o : list$$fieldName) { - if ($fieldName != DEFAULT.$fieldName) { - if (o != null) { - result = 31 * result; - } - else { - result = 31 * result + o.hashCode(); - } - } - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + java.util.List list$$fieldName = $fieldName; + for (Object o : list$$fieldName) { + if ($fieldName != DEFAULT.$fieldName) { + if (o != null) { + result = 31 * result; + } else { + result = 31 * result + o.hashCode(); + } + } + } + """).replace("$fieldName", f.nameCamelFirstLower()); return generatedCodeSoFar; } @@ -365,59 +382,65 @@ else if (f.nameCamelFirstLower() != null) { f.type() == Field.FieldType.SFIXED32 || f.type() == Field.FieldType.SINT32 || f.type() == Field.FieldType.UINT32) { - generatedCodeSoFar += """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FIXED64 || f.type() == Field.FieldType.INT64 || f.type() == Field.FieldType.SFIXED64 || f.type() == Field.FieldType.SINT64 || f.type() == Field.FieldType.UINT64) { - generatedCodeSoFar += """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BOOL) { - generatedCodeSoFar += """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.DOUBLE) { } else if (f.type() == Field.FieldType.FLOAT) { - generatedCodeSoFar += """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.DOUBLE) { - generatedCodeSoFar += """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.STRING || f.type() == Field.FieldType.BYTES || f.type() == Field.FieldType.ENUM || f.parent() == null /* Process a sub-message */) { - generatedCodeSoFar += (""" - if ($fieldName == null && thatObj.$fieldName != null) { - return false; - } - if ($fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } - """).replace("$fieldName", f.nameCamelFirstLower()); + generatedCodeSoFar += ( + """ + if ($fieldName == null && thatObj.$fieldName != null) { + return false; + } + if ($fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); } else { throw new RuntimeException("Unexpected field type for getting HashCode - " + f.type().toString()); } } } - return generatedCodeSoFar; + return generatedCodeSoFar.indent(DEFAULT_INDENT); } /** @@ -429,18 +452,19 @@ else if (f.nameCamelFirstLower() != null) { @NonNull private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Field f) { switch (f.messageType()) { - case "StringValue": - generatedCodeSoFar += (FIELD_INDENT + """ + case "StringValue" -> + generatedCodeSoFar += ( + """ if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; + return false; } if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { return false; } """).replace("$fieldName", f.nameCamelFirstLower()); - break; - case "BoolValue": - generatedCodeSoFar += (FIELD_INDENT + """ + case "BoolValue" -> + generatedCodeSoFar += ( + """ if ($fieldName instanceof Object) { if (this.$fieldName == null && thatObj.$fieldName != null) { return false; @@ -448,41 +472,13 @@ private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Fie if (!$fieldName.equals(thatObj.$fieldName)) { return false; } - } - else if ($fieldName != thatObj.$fieldName) { - return false; - } - """).replace("$fieldName", f.nameCamelFirstLower()); - break; - case "Int32Value", "UInt32Value": generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName instanceof Object) { - if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; - } - if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } - } - else if ($fieldName != thatObj.$fieldName) { - return false; - } - """).replace("$fieldName", f.nameCamelFirstLower()); - break; - case "Int64Value", "UInt64Value": generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName instanceof Object) { - if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; - } - if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } - } - else if ($fieldName != thatObj.$fieldName) { + } else if ($fieldName != thatObj.$fieldName) { return false; } """).replace("$fieldName", f.nameCamelFirstLower()); - break; - case "FloatValue": generatedCodeSoFar += (FIELD_INDENT + """ + case "Int32Value", "UInt32Value", "Int64Value", "UInt64Value", "FloatValue", "DoubleValue" -> + generatedCodeSoFar += ( + """ if ($fieldName instanceof Object) { if (this.$fieldName == null && thatObj.$fieldName != null) { return false; @@ -490,36 +486,21 @@ else if ($fieldName != thatObj.$fieldName) { if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { return false; } - } - else if ($fieldName != thatObj.$fieldName) { + } else if ($fieldName != thatObj.$fieldName) { return false; } """).replace("$fieldName", f.nameCamelFirstLower()); - break; - case "DoubleValue": generatedCodeSoFar += (FIELD_INDENT + """ - if ($fieldName instanceof Object) { - if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; - } - if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } - } - else if ($fieldName != thatObj.$fieldName) { - return false; - } - """).replace("$fieldName", f.nameCamelFirstLower()); - break; - case "BytesValue": generatedCodeSoFar += (FIELD_INDENT + """ + case "BytesValue" -> + generatedCodeSoFar += ( + """ if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; + return false; } if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { return false; } """).replace("$fieldName", f.nameCamelFirstLower()); - break; - default: throw new UnsupportedOperationException("Unhandled optional message type:" + f.messageType()); + default -> throw new UnsupportedOperationException("Unhandled optional message type:" + f.messageType()); } ; return generatedCodeSoFar; @@ -533,15 +514,16 @@ else if ($fieldName != thatObj.$fieldName) { */ @NonNull private static String getRepeatedEqualsGeneration(String generatedCodeSoFar, Field f) { - generatedCodeSoFar += (""" - if (this.$fieldName == null && this.$fieldName != null) { - return false; - } + generatedCodeSoFar += ( + """ + if (this.$fieldName == null && this.$fieldName != null) { + return false; + } - if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } - """).replace("$fieldName", f.nameCamelFirstLower()); + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); return generatedCodeSoFar; } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java index a426c32a..662ad0ff 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java @@ -14,6 +14,7 @@ import java.util.stream.Collectors; import static com.hedera.pbj.compiler.impl.Common.*; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; /** * Code for generating enum code @@ -70,7 +71,7 @@ public static void generateEnumFile(Protobuf3Parser.EnumDefContext enumDef, File try (FileWriter javaWriter = new FileWriter(getJavaFile(destinationSrcDir, modelPackage, enumName))) { javaWriter.write( "package "+modelPackage+";\n\n"+ - createEnum("", javaDocComment, deprecated, enumName, maxIndex, enumValues, false) + createEnum(javaDocComment, deprecated, enumName, maxIndex, enumValues, false) ); } } @@ -78,7 +79,6 @@ public static void generateEnumFile(Protobuf3Parser.EnumDefContext enumDef, File /** * Generate code for a enum * - * @param indent extra indent spaces beyond the default 4 * @param javaDocComment either enum javadoc comment or empty string * @param deprecated either @deprecated string or empty string * @param enumName the name for enum @@ -87,34 +87,35 @@ public static void generateEnumFile(Protobuf3Parser.EnumDefContext enumDef, File * @param addUnknown when true we add an enum value for one of * @return string code for enum */ - static String createEnum(String indent, String javaDocComment, String deprecated, String enumName, + static String createEnum(String javaDocComment, String deprecated, String enumName, int maxIndex, Map enumValues, boolean addUnknown) { final List enumValuesCode = new ArrayList<>(maxIndex); if (addUnknown) { - enumValuesCode.add(FIELD_INDENT+""" + enumValuesCode.add( + """ /** * Enum value for a unset OneOf, to avoid null OneOfs */ - UNSET(-1, "UNSET")""" - .replaceAll("\n","\n"+FIELD_INDENT)); + UNSET(-1, "UNSET")"""); } for (int i = 0; i <= maxIndex; i++) { final EnumValue enumValue = enumValues.get(i); if (enumValue != null) { - final String cleanedEnumComment = FIELD_INDENT + "/** \n" - + FIELD_INDENT+" * " - + enumValue.javaDoc.replaceAll("\n[\t\s]*","\n"+FIELD_INDENT+" * ") // clean up doc indenting - + "\n" - + FIELD_INDENT + " */\n"; - final String deprecatedText = enumValue.deprecated ? FIELD_INDENT+"@Deprecated\n" : ""; + final String cleanedEnumComment = + """ + /**$enumJavadoc + */ + """ + .replace("$enumJavadoc", enumValue.javaDoc); + final String deprecatedText = enumValue.deprecated ? "@Deprecated\n" : ""; enumValuesCode.add( cleanedEnumComment - + deprecatedText+FIELD_INDENT+camelToUpperSnake(enumValue.name)+"("+i+", \""+enumValue.name+"\")"); + + deprecatedText+ camelToUpperSnake(enumValue.name) +"("+i+", \""+enumValue.name+"\")"); } } return """ $javaDocComment - $deprecated public enum $enumName implements com.hedera.pbj.runtime.EnumWithProtoMetadata{ + $deprecated$public enum $enumName implements com.hedera.pbj.runtime.EnumWithProtoMetadata { $enumValues; /** The field ordinal in protobuf for this type */ @@ -181,9 +182,9 @@ public String protoName() { } """ .replace("$javaDocComment", javaDocComment) - .replace("$deprecated", deprecated) + .replace("$deprecated$", deprecated) .replace("$enumName", enumName) - .replace("$enumValues", String.join(",\n\n", enumValuesCode)) + .replace("$enumValues", String.join(",\n\n", enumValuesCode).indent(DEFAULT_INDENT)) .replace("$caseStatements", enumValues.entrySet().stream().map((entry) -> " case " + entry.getKey() + " -> " + camelToUpperSnake(entry.getValue().name) + ";").collect(Collectors.joining("\n"))) .replace("$fromStringCaseStatements", enumValues.values().stream().map(enumValue -> { @@ -194,7 +195,6 @@ public String protoName() { return " case \"" + camelToUpperSnake(enumValue.name) + "\", \"" + enumValue.name + "\" -> " + camelToUpperSnake(enumValue.name) + ";"; } - }).collect(Collectors.joining("\n"))) - .replaceAll("\n", "\n" + indent); + }).collect(Collectors.joining("\n"))); } } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java index f721c90a..0663da2d 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java @@ -22,6 +22,20 @@ @SuppressWarnings({"StringConcatenationInLoop", "EscapedSpace", "RedundantLabeledSwitchRuleCodeBlock"}) public final class ModelGenerator implements Generator { + private static final String HASH_CODE_MANIPULATION = + """ + // Shifts: 30, 27, 16, 20, 5, 18, 10, 24, 30 + hashCode += hashCode << 30; + hashCode ^= hashCode >>> 27; + hashCode += hashCode << 16; + hashCode ^= hashCode >>> 20; + hashCode += hashCode << 5; + hashCode ^= hashCode >>> 18; + hashCode += hashCode << 10; + hashCode ^= hashCode >>> 24; + hashCode += hashCode << 30; + """.indent(DEFAULT_INDENT); + /** * {@inheritDoc} * @@ -123,8 +137,8 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, .replace("$oneOfField",oneOfField.nameCamelFirstLower()) .replace("$enumName",enumName) .replace("$enumValue",camelToUpperSnake(field.name())) - .replace("$enumValue",camelToUpperSnake(field.name())) - .replaceAll("\n","\n" + FIELD_INDENT)); + .indent(DEFAULT_INDENT) + ); if (field.type() == Field.FieldType.MESSAGE) { field.addAllNeededImports(imports, true, false, false); } @@ -133,7 +147,8 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, /** * Enum for the type of "%s" oneof value */""".formatted(oneOfField.name()); - final String enumString = createEnum(FIELD_INDENT,enumComment ,"",enumName,maxIndex,enumValues, true); + final String enumString = createEnum(enumComment ,"",enumName,maxIndex,enumValues, true) + .indent(DEFAULT_INDENT); oneofEnums.add(enumString); fields.add(oneOfField); imports.add("com.hedera.pbj.runtime"); @@ -189,7 +204,9 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, """ .replace("$fieldNameUpperFirst", field.nameCamelFirstUpper()) .replace("$javaFieldType", field.javaFieldType()) - .replace("$fieldName", field.nameCamelFirstLower())); + .replace("$fieldName", field.nameCamelFirstLower()) + .indent(DEFAULT_INDENT) + ); } } else if (item.optionStatement() != null){ if ("deprecated".equals(item.optionStatement().optionName().getText())) { @@ -221,8 +238,9 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, String bodyContent = ""; // static codec and default instance - bodyContent += """ - /** Protobuf codec for reading and writing in protobuf format */ + bodyContent += + """ + /** Protobuf codec for reading and writing in protobuf format */ public static final Codec<$modelClass> PROTOBUF = new $qualifiedCodecClass(); /** JSON codec for reading and writing in JSON format */ public static final JsonCodec<$modelClass> JSON = new $qualifiedJsonCodecClass(); @@ -233,7 +251,7 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, .replace("$modelClass",javaRecordName) .replace("$qualifiedCodecClass",lookupHelper.getFullyQualifiedMessageClassname(FileType.CODEC, msgDef)) .replace("$qualifiedJsonCodecClass",lookupHelper.getFullyQualifiedMessageClassname(FileType.JSON_CODEC, msgDef)) - .replaceAll("\n","\n"+FIELD_INDENT); + .indent(DEFAULT_INDENT); // constructor if (fields.stream().anyMatch(f -> f instanceof OneOfField || f.optionalValueType())) { @@ -246,18 +264,19 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, public %s { %s } - - """.formatted( - fields.stream().map(field -> "\n * @param "+field.nameCamelFirstLower()+" "+ - field.comment() - .replaceAll("\n", "\n * "+" ".repeat(field.nameCamelFirstLower().length())) - ).collect(Collectors.joining()), - javaRecordName, - fields.stream() - .filter(f -> f instanceof OneOfField) - .map(ModelGenerator::generateConstructorCode) - .collect(Collectors.joining("\n")) - ).replaceAll("\n","\n"+FIELD_INDENT); + """ + .formatted( + fields.stream().map(field -> "\n * @param "+field.nameCamelFirstLower()+" "+ + field.comment() + .replaceAll("\n", "\n * "+" ".repeat(field.nameCamelFirstLower().length())) + ).collect(Collectors.joining()), + javaRecordName, + fields.stream() + .filter(f -> f instanceof OneOfField) + .map(ModelGenerator::generateConstructorCode) + .collect(Collectors.joining("\n")) + ) + .indent(DEFAULT_INDENT); } boolean hashCodeGenerated = true; @@ -266,76 +285,57 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, switch (fieldType) { case INT32, UINT32, SINT32, FIXED32, SFIXED32, FIXED64, SFIXED64, INT64, UINT64, SINT64 -> { - bodyContent += FIELD_INDENT + """ - /** - * Override the default hashCode method for - * single field int objects. - */ - @Override - public int hashCode() { - // Shifts: 30, 27, 16, 20, 5, 18, 10, 24, 30 - long x = $fieldName; - x += x << 30; - x ^= x >>> 27; - x += x << 16; - x ^= x >>> 20; - x += x << 5; - x ^= x >>> 18; - x += x << 10; - x ^= x >>> 24; - x += x << 30; - return (int)x; - }""" - .replace("$fieldName", fields.get(0).name()) - .replaceAll("\n","\n"+FIELD_INDENT); + bodyContent += + """ + /** + * Override the default hashCode method for + * single field int objects. + */ + @Override + public int hashCode() { + long hashCode = $fieldName; + $hashCodeManipulation + return (int)hashCode; + } + """.replace("$fieldName", fields.get(0).name()) + .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) + .indent(DEFAULT_INDENT); } case FLOAT, DOUBLE -> { - bodyContent += FIELD_INDENT + """ - /** - * Override the default hashCode method for - * single field float and double objects. - */ - @Override - public int hashCode() { - // Shifts: 30, 27, 16, 20, 5, 18, 10, 24, 30 - double x = $fieldName; - x += x << 30; - x ^= x >>> 27; - x += x << 16; - x ^= x >>> 20; - x += x << 5; - x ^= x >>> 18; - x += x << 10; - x ^= x >>> 24; - x += x << 30; - return (int)x; - }""" - .replace("$fieldName", fields.get(0).name()) - .replaceAll("\n", "\n" + FIELD_INDENT); + bodyContent += + """ + /** + * Override the default hashCode method for + * single field float and double objects. + */ + @Override + public int hashCode() { + double hashCode = $fieldName; + $hashCodeManipulation + return (int)hashCode; + } + """.replace("$fieldName", fields.get(0).name()) + .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) + .indent(DEFAULT_INDENT); } case STRING -> { - bodyContent += FIELD_INDENT + """ - /** - * Override the default hashCode method for - * single field String objects. - */ - @Override - public int hashCode() { - // Shifts: 30, 27, 16, 20, 5, 18, 10, 24, 30 - long x = $fieldName.hashCode(); - x += x << 30; - x ^= x >>> 27; - x += x << 16; - x ^= x >>> 20; - x += x << 5; - x ^= x >>> 18; - x += x << 10; - x ^= x >>> 24; - x += x << 30; - return (int)x; - }""" - .replace("$fieldName", fields.get(0).name()) - .replaceAll("\n", "\n" + FIELD_INDENT); + bodyContent += + """ + /** + * Override the default hashCode method for + * single field String objects. + */ + @Override + public int hashCode() { + long hashCode = $fieldName.hashCode(); + $hashCodeManipulation + return (int)hashCode; + } + """ + .replace("$fieldName", fields.get(0).name()) + .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) + .indent(DEFAULT_INDENT) + ; } default -> { hashCodeGenerated = false; @@ -351,33 +351,27 @@ public int hashCode() { // and calculates the hashcode. statements = Common.getFieldsHashCode(fields, statements); - bodyContent += """ - /** - * Override the default hashCode method for - * all other objects to make hashCode - */ - @Override - public int hashCode() { - int result = 1; - """; + bodyContent += + """ + /** + * Override the default hashCode method for + * all other objects to make hashCode + */ + @Override + public int hashCode() { + int result = 1; + """.indent(DEFAULT_INDENT); bodyContent += statements; - bodyContent += """ - long hashCode = result; - hashCode += hashCode << 30; - hashCode ^= hashCode >>> 27; - hashCode += hashCode << 16; - hashCode ^= hashCode >>> 20; - hashCode += hashCode << 5; - hashCode ^= hashCode >>> 18; - hashCode += hashCode << 10; - hashCode ^= hashCode >>> 24; - hashCode += hashCode << 30; - return (int)hashCode; - } - """; - bodyContent = bodyContent.replaceAll("\n", "\n" + FIELD_INDENT); + bodyContent += + """ + long hashCode = result; + $hashCodeManipulation + return (int)hashCode; + } + """.replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) + .indent(DEFAULT_INDENT); } String equalsStatements = ""; @@ -385,28 +379,28 @@ public int hashCode() { // and calculates the hashcode. equalsStatements = Common.getFieldsEqualsStatements(fields, equalsStatements); - bodyContent += FIELD_INDENT + """ - /** - * Override the default equals method for - */ - @Override - public boolean equals(Object that) { - if (that == null || this.getClass() != that.getClass()) { - return false; - } - - $javaRecordName thatObj = ($javaRecordName)that; - - """.replace("$javaRecordName", javaRecordName); + bodyContent += + """ + /** + * Override the default equals method for + */ + @Override + public boolean equals(Object that) { + if (that == null || this.getClass() != that.getClass()) { + return false; + } + $javaRecordName thatObj = ($javaRecordName)that; + """.replace("$javaRecordName", javaRecordName).indent(DEFAULT_INDENT); - bodyContent += FIELD_INDENT + FIELD_INDENT + equalsStatements; - bodyContent += """ - return true; - } - """; + bodyContent += equalsStatements.indent(DEFAULT_INDENT); + bodyContent += + """ + return true; + } + """.indent(DEFAULT_INDENT); // Has methods - bodyContent += String.join("\n", hasMethods).replaceAll("\n","\n"+FIELD_INDENT); + bodyContent += String.join("\n", hasMethods); bodyContent += "\n"; // oneof getters @@ -414,40 +408,41 @@ public boolean equals(Object that) { bodyContent += "\n"; // builder copy & new builder methods - bodyContent += FIELD_INDENT + """ - /** - * Return a builder for building a copy of this model object. It will be pre-populated with all the data from this - * model object. - * - * @return a pre-populated builder - */ - public Builder copyBuilder() { - return new Builder(%s); - } - - /** - * Return a new builder for building a model object. This is just a shortcut for new Model.Builder(). - * - * @return a new builder - */ - public static Builder newBuilder() { - return new Builder(); - } - - """ - .formatted(fields.stream().map(Field::nameCamelFirstLower).collect(Collectors.joining(", "))) - .replaceAll("\n","\n"+FIELD_INDENT); + bodyContent += + """ + /** + * Return a builder for building a copy of this model object. It will be pre-populated with all the data from this + * model object. + * + * @return a pre-populated builder + */ + public Builder copyBuilder() { + return new Builder(%s); + } + + /** + * Return a new builder for building a model object. This is just a shortcut for new Model.Builder(). + * + * @return a new builder + */ + public static Builder newBuilder() { + return new Builder(); + } + """ + .formatted(fields.stream().map(Field::nameCamelFirstLower).collect(Collectors.joining(", "))) + .indent(DEFAULT_INDENT); // generate builder bodyContent += generateBuilder(msgDef, fields, lookupHelper); - bodyContent += "\n"+FIELD_INDENT; + bodyContent += "\n"; // oneof enums bodyContent += String.join("\n ", oneofEnums); // === Build file try (FileWriter javaWriter = new FileWriter(javaFile)) { - javaWriter.write(""" + javaWriter.write( + """ package $package; $imports import com.hedera.pbj.runtime.Codec; @@ -455,22 +450,20 @@ public static Builder newBuilder() { import edu.umd.cs.findbugs.annotations.Nullable; import static java.util.Objects.requireNonNull; - $javaDocComment - $deprecated + $javaDocComment$deprecated public record $javaRecordName( - $fields - ){ - $bodyContent - } + $fields){ + $bodyContent} """ .replace("$package",modelPackage) .replace("$imports",imports.isEmpty() ? "" : imports.stream().collect(Collectors.joining(".*;\nimport ","\nimport ",".*;\n"))) .replace("$javaDocComment",javaDocComment) - .replace("$deprecated",deprecated) + .replace("$deprecated", deprecated) .replace("$javaRecordName",javaRecordName) - .replace("$fields",fields.stream().map(field -> - FIELD_INDENT + (field.type() == FieldType.MESSAGE ? "@Nullable " : "") + field.javaFieldType() + " " + field.nameCamelFirstLower() - ).collect(Collectors.joining(",\n "))) + .replace("$fields", fields.stream().map(field -> + (field.type() == FieldType.MESSAGE ? "@Nullable " : "") + + field.javaFieldType() + " " + field.nameCamelFirstLower() + ).collect(Collectors.joining(",\n")).indent(DEFAULT_INDENT)) .replace("$bodyContent",bodyContent) ); } @@ -497,7 +490,7 @@ private static void generateBuilderMethods(List builderMethods, Field fi * @return builder to continue building with */ public Builder $fieldName($fieldType $fieldName) { - this.$fieldToSet = $prefix $fieldName $postfix; + this.$fieldToSet = $prefix$fieldName$postfix; return this; }""" .replace("$fieldDoc",field.comment() @@ -507,7 +500,8 @@ private static void generateBuilderMethods(List builderMethods, Field fi .replace("$prefix",prefix) .replace("$postfix",postfix) .replace("$fieldType",field.javaFieldType()) - .replaceAll("\n","\n"+FIELD_INDENT)); + .indent(DEFAULT_INDENT) + ); // add nice method for simple message fields so can just set using un-built builder if (field.type() == Field.FieldType.MESSAGE && !field.optionalValueType() && !field.repeated()) { builderMethods.add(""" @@ -529,7 +523,8 @@ private static void generateBuilderMethods(List builderMethods, Field fi .replace("$prefix",prefix) .replace("$postfix",postfix) .replace("$fieldType",field.javaFieldType()) - .replaceAll("\n","\n"+FIELD_INDENT)); + .indent(DEFAULT_INDENT) + ); } // add nice method for message fields with list types for varargs @@ -553,7 +548,8 @@ private static void generateBuilderMethods(List builderMethods, Field fi .replace("$fieldType",field.javaFieldType()) .replace("$prefix",prefix) .replace("$postfix",postfix) - .replaceAll("\n","\n"+FIELD_INDENT)); + .indent(DEFAULT_INDENT) + ); } } @@ -588,8 +584,7 @@ public Builder() {} * $constructorParamDocs */ public Builder($constructorParams) { - $constructorCode; - } + $constructorCode } /** * Build a new model record with data set on builder @@ -600,8 +595,7 @@ public Builder($constructorParams) { return new $javaRecordName($recordParams); } - $builderMethods - }""" + $builderMethods}""" .replace("$fields", fields.stream().map(field -> "private " + field.javaFieldType() + " " + field.nameCamelFirstLower() + " = " + getDefaultValue(field, msgDef, lookupHelper) @@ -614,12 +608,12 @@ public Builder($constructorParams) { field.javaFieldType() + " " + field.nameCamelFirstLower() ).collect(Collectors.joining(", "))) .replace("$constructorCode",fields.stream().map(field -> - "this." + field.nameCamelFirstLower() + " = " + field.nameCamelFirstLower() - ).collect(Collectors.joining(";\n"+FIELD_INDENT+FIELD_INDENT))) + "this.$name = $name;".replace("$name", field.nameCamelFirstLower()) + ).collect(Collectors.joining("\n")).indent(DEFAULT_INDENT * 2)) .replace("$javaRecordName",javaRecordName) .replace("$recordParams",fields.stream().map(Field::nameCamelFirstLower).collect(Collectors.joining(", "))) - .replace("$builderMethods",builderMethods.stream().collect(Collectors.joining("\n\n"+FIELD_INDENT))) - .replaceAll("\n","\n"+FIELD_INDENT); + .replace("$builderMethods", String.join("\n", builderMethods)) + .indent(DEFAULT_INDENT); } private static String getDefaultValue(Field field, final Protobuf3Parser.MessageDefContext msgDef, final ContextualLookupHelper lookupHelper) { @@ -631,7 +625,7 @@ private static String getDefaultValue(Field field, final Protobuf3Parser.Message } private static String generateConstructorCode(final Field f) { - StringBuilder sb = new StringBuilder(FIELD_INDENT+""" + StringBuilder sb = new StringBuilder(""" if ($fieldName == null) { throw new NullPointerException("Parameter '$fieldName' must be supplied and can not be null"); }""".replace("$fieldName", f.nameCamelFirstLower())); @@ -652,6 +646,6 @@ private static String generateConstructorCode(final Field f) { } } } - return sb.toString().replaceAll("\n","\n"+FIELD_INDENT); + return sb.toString().indent(DEFAULT_INDENT); } } \ No newline at end of file diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java index de669bc5..962823f2 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java @@ -1,5 +1,7 @@ package com.hedera.pbj.compiler.impl.generators; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; + import com.hedera.pbj.compiler.impl.*; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser; @@ -94,9 +96,9 @@ public final class %s { modelClassName, testClassName, generateTestMethod(modelClassName, protoCJavaFullQualifiedClass) - .replaceAll("\n","\n"+ Common.FIELD_INDENT), + .indent(DEFAULT_INDENT), generateModelTestArgumentsMethod(modelClassName, fields) - .replaceAll("\n","\n"+ Common.FIELD_INDENT) + .indent(DEFAULT_INDENT) ) ); } @@ -135,16 +137,16 @@ private static String generateModelTestArgumentsMethod(final String modelClassNa modelClassName, fields.stream() .map(f -> "final var "+f.nameCamelFirstLower()+"List = "+generateTestData(modelClassName, f, f.optionalValueType(), f.repeated())+";") - .collect(Collectors.joining("\n"+ Common.FIELD_INDENT)), + .collect(Collectors.joining("\n")).indent(DEFAULT_INDENT), fields.stream() .map(f -> f.nameCamelFirstLower()+"List.size()") - .collect(Collectors.joining(",\n"+ Common.FIELD_INDENT+ Common.FIELD_INDENT)), + .collect(Collectors.joining(",\n")).indent(DEFAULT_INDENT * 2), modelClassName, fields.stream().map(field -> "%sList.get(Math.min(i, %sList.size()-1))".formatted( field.nameCamelFirstLower(), field.nameCamelFirstLower() )) - .collect(Collectors.joining(",\n"+ Common.FIELD_INDENT+ Common.FIELD_INDENT+ Common.FIELD_INDENT+ Common.FIELD_INDENT)), + .collect(Collectors.joining(",\n")).indent(DEFAULT_INDENT * 4), modelClassName, modelClassName ); @@ -157,12 +159,12 @@ private static String generateTestData(String modelClassName, Field field, boole return """ addNull(%s)""" .formatted(getOptionsForFieldType(convertedFieldType, convertedFieldType.javaType)) - .replaceAll("\n","\n"+ Common.FIELD_INDENT+ Common.FIELD_INDENT); + .indent(DEFAULT_INDENT * 2); } else if (repeated) { final String optionsList = generateTestData(modelClassName, field, field.optionalValueType(), false); return """ generateListArguments(%s)""".formatted(optionsList) - .replaceAll("\n","\n"+ Common.FIELD_INDENT+ Common.FIELD_INDENT); + .indent(DEFAULT_INDENT * 2); } else if(field instanceof final OneOfField oneOf) { final List options = new ArrayList<>(); for (var subField: oneOf.fields()) { @@ -184,7 +186,7 @@ private static String generateTestData(String modelClassName, Field field, boole .toList()""".formatted( modelClassName + "." + field.nameCamelFirstUpper(), enumValueName - ).replaceAll("\n", "\n" + Common.FIELD_INDENT + Common.FIELD_INDENT) + ).indent(DEFAULT_INDENT * 2) ); } } else { @@ -198,8 +200,8 @@ private static String generateTestData(String modelClassName, Field field, boole %s ).flatMap(List::stream).toList()""".formatted( modelClassName+"."+field.nameCamelFirstUpper(), - options.stream().collect(Collectors.joining(",\n"+ Common.FIELD_INDENT)) - ).replaceAll("\n","\n"+ Common.FIELD_INDENT+ Common.FIELD_INDENT); + String.join(",\n", options).indent(DEFAULT_INDENT) + ).indent(DEFAULT_INDENT * 2); } else { return getOptionsForFieldType(field.type(), ((SingleField)field).javaFieldTypeForTest()); } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecParseMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecParseMethodGenerator.java index 4ccf07da..e8eaf7a7 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecParseMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecParseMethodGenerator.java @@ -7,7 +7,7 @@ import java.util.List; import java.util.stream.Collectors; -import static com.hedera.pbj.compiler.impl.Common.FIELD_INDENT; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; import static com.hedera.pbj.compiler.impl.generators.json.JsonCodecGenerator.toJsonFieldName; /** @@ -79,7 +79,7 @@ static String generateParseObjectMethod(final String modelClassName, final List< field.name(), field.javaDefault())).collect(Collectors.joining("\n"))) .replace("$fieldsList",fields.stream().map(field -> "temp_"+field.name()).collect(Collectors.joining(", "))) .replace("$caseStatements",generateCaseStatements(fields)) - .replaceAll("\n", "\n" + FIELD_INDENT); + .indent(DEFAULT_INDENT); } /** @@ -96,8 +96,8 @@ private static String generateCaseStatements(final List fields) { for(final Field subField: oneOfField.fields()) { sb.append("case \"" + toJsonFieldName(subField.name()) +"\" /* [" + subField.fieldNumber() + "] */ " + "-> temp_" + oneOfField.name()+" = new OneOf<>(\n"+ - FIELD_INDENT + oneOfField.getEnumClassRef()+"."+Common.camelToUpperSnake(subField.name())+ - ", \n" +FIELD_INDENT); + oneOfField.getEnumClassRef().indent(DEFAULT_INDENT) +"."+Common.camelToUpperSnake(subField.name())+ + ", \n".indent(DEFAULT_INDENT)); generateFieldCaseStatement(sb,subField); sb.append(");\n"); } @@ -108,7 +108,7 @@ private static String generateCaseStatements(final List fields) { sb.append(";\n"); } } - return sb.toString().replaceAll("\n","\n" + FIELD_INDENT.repeat(3)); + return sb.toString().indent(DEFAULT_INDENT * 3); } /** diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java index 4543be88..84321232 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecWriteMethodGenerator.java @@ -11,6 +11,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; import static com.hedera.pbj.compiler.impl.generators.json.JsonCodecGenerator.toJsonFieldName; /** @@ -26,7 +27,7 @@ static String generateWriteMethod(final String modelClassName, final List .toList(); final String fieldWriteLines = fieldsToWrite.stream() .map(field -> generateFieldWriteLines(field, modelClassName, "data.%s()".formatted(field.nameCamelFirstLower()))) - .collect(Collectors.joining("\n"+Common.FIELD_INDENT)); + .collect(Collectors.joining("\n")).indent(DEFAULT_INDENT); return """ /** @@ -59,7 +60,7 @@ public String toJSON(@NonNull $modelClass data, String indent, boolean inline) { """ .replace("$modelClass", modelClassName) .replace("$fieldWriteLines", fieldWriteLines) - .replaceAll("\n", "\n" + Common.FIELD_INDENT); + .indent(DEFAULT_INDENT); } @@ -75,14 +76,14 @@ private static String generateFieldWriteLines(final Field field, final String mo final String fieldDef = Common.camelToUpperSnake(field.name()); final String fieldName = '\"' + toJsonFieldName(field.name()) + '\"'; final String basicFieldCode = generateBasicFieldLines(field, getValueCode, fieldDef, fieldName); - String prefix = "// ["+field.fieldNumber()+"] - "+field.name() + "\n"+ Common.FIELD_INDENT; + String prefix = "// ["+field.fieldNumber()+"] - "+field.name() + "\n"; if (field.parent() != null) { final OneOfField oneOfField = field.parent(); final String oneOfType = modelClassName+"."+oneOfField.nameCamelFirstUpper()+"OneOfType"; prefix += "if(data."+oneOfField.nameCamelFirstLower()+"().kind() == "+ oneOfType +"."+ Common.camelToUpperSnake(field.name())+")"; - prefix += "\n"+ Common.FIELD_INDENT.repeat(2); + prefix += "\n"; return prefix + "fieldLines.add(" + basicFieldCode + ");"; } else { if (field.repeated()) { diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecFastEqualsMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecFastEqualsMethodGenerator.java index fe07aa03..f24d4743 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecFastEqualsMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecFastEqualsMethodGenerator.java @@ -1,6 +1,7 @@ package com.hedera.pbj.compiler.impl.generators.protobuf; -import com.hedera.pbj.compiler.impl.Common; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; + import com.hedera.pbj.compiler.impl.Field; import java.util.List; @@ -32,6 +33,6 @@ public boolean fastEquals(@NonNull $modelClass item, @NonNull final ReadableSequ } """ .replace("$modelClass", modelClassName) - .replaceAll("\n", "\n" + Common.FIELD_INDENT); + .indent(DEFAULT_INDENT); } } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureDataMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureDataMethodGenerator.java index 4cee5087..f3625409 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureDataMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureDataMethodGenerator.java @@ -1,6 +1,7 @@ package com.hedera.pbj.compiler.impl.generators.protobuf; -import com.hedera.pbj.compiler.impl.Common; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; + import com.hedera.pbj.compiler.impl.Field; import java.util.List; @@ -30,6 +31,6 @@ public int measure(@NonNull final ReadableSequentialData input) throws IOExcepti return (int)(end - start); } """ - .replaceAll("\n", "\n" + Common.FIELD_INDENT); + .indent(DEFAULT_INDENT); } } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureRecordMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureRecordMethodGenerator.java index 4bf82181..6c70652d 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureRecordMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecMeasureRecordMethodGenerator.java @@ -1,5 +1,7 @@ package com.hedera.pbj.compiler.impl.generators.protobuf; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; + import com.hedera.pbj.compiler.impl.Common; import com.hedera.pbj.compiler.impl.Field; import com.hedera.pbj.compiler.impl.OneOfField; @@ -21,7 +23,7 @@ static String generateMeasureMethod(final String modelClassName, final List field.type() == Field.FieldType.ONE_OF ? ((OneOfField)field).fields().stream() : Stream.of(field)) .sorted(Comparator.comparingInt(Field::fieldNumber)) .map(field -> generateFieldSizeOfLines(field, modelClassName, "data.%s()".formatted(field.nameCamelFirstLower()))) - .collect(Collectors.joining("\n" + Common.FIELD_INDENT)); + .collect(Collectors.joining("\n")).indent(DEFAULT_INDENT); return """ /** * Compute number of bytes that would be written when calling {@code write()} method. @@ -37,8 +39,7 @@ public int measureRecord($modelClass data) { """ .replace("$modelClass", modelClassName) .replace("$fieldSizeOfLines", fieldSizeOfLines) - .replaceAll("\n", "\n" + Common.FIELD_INDENT) - ; + .indent(DEFAULT_INDENT); } /** @@ -52,7 +53,7 @@ public int measureRecord($modelClass data) { private static String generateFieldSizeOfLines(final Field field, final String modelClassName, String getValueCode) { final String fieldDef = Common.camelToUpperSnake(field.name()); String prefix = "// ["+field.fieldNumber()+"] - "+field.name(); - prefix += "\n"+ Common.FIELD_INDENT.repeat(1); + prefix += "\n"; if (field.parent() != null) { final OneOfField oneOfField = field.parent(); @@ -60,7 +61,7 @@ private static String generateFieldSizeOfLines(final Field field, final String m getValueCode = "data."+oneOfField.nameCamelFirstLower()+"().as()"; prefix += "if(data."+oneOfField.nameCamelFirstLower()+"().kind() == "+ oneOfType +"."+ Common.camelToUpperSnake(field.name())+")"; - prefix += "\n"+ Common.FIELD_INDENT.repeat(2); + prefix += "\n"; } final String writeMethodName = field.methodNameType(); diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java index e44c43aa..f9fbc473 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java @@ -1,5 +1,7 @@ package com.hedera.pbj.compiler.impl.generators.protobuf; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; + import com.hedera.pbj.compiler.impl.Common; import com.hedera.pbj.compiler.impl.Field; import com.hedera.pbj.compiler.impl.OneOfField; @@ -88,7 +90,7 @@ static String generateParseStrictMethod(final String modelClassName, final List< field.name(), field.javaDefault())).collect(Collectors.joining("\n"))) .replace("$fieldsList",fields.stream().map(field -> "temp_"+field.name()).collect(Collectors.joining(", "))) .replace("$caseStatements",generateCaseStatements(fields)) - .replaceAll("\n", "\n" + Common.FIELD_INDENT); + .indent(DEFAULT_INDENT); } static String generateParseInternalMethod(final String modelClassName, final List fields) { @@ -170,7 +172,7 @@ static String generateParseInternalMethod(final String modelClassName, final Lis field.name(), field.javaDefault())).collect(Collectors.joining("\n"))) .replace("$fieldsList",fields.stream().map(field -> "temp_"+field.name()).collect(Collectors.joining(", "))) .replace("$caseStatements",generateCaseStatements(fields)) - .replaceAll("\n", "\n" + Common.FIELD_INDENT); + .indent(DEFAULT_INDENT); } /** @@ -196,7 +198,7 @@ private static String generateCaseStatements(final List fields) { generateFieldCaseStatement(sb, field); } } - return sb.toString().replaceAll("\n","\n" + Common.FIELD_INDENT.repeat(3)); + return sb.toString().indent(DEFAULT_INDENT * 3); } /** @@ -212,7 +214,6 @@ private static void generateFieldCaseStatementPacked(final StringBuilder sb, fin final int tag = Common.getTag(wireType, fieldNum); sb.append("case " + tag +" /* type=" + wireType + " [" + field.type() + "] packed-repeated " + "field=" + fieldNum + " [" + field.name() + "] */ -> {\n"); - sb.append(Common.FIELD_INDENT); sb.append(""" // Read the length of packed repeated field data final var length = input.readVarInt(false); @@ -224,7 +225,7 @@ private static void generateFieldCaseStatementPacked(final StringBuilder sb, fin input.limit(beforeLimit);""" .replace("$tempFieldName", "temp_" + field.name()) .replace("$readMethod", readMethod(field)) - .replaceAll("\n","\n" + Common.FIELD_INDENT) + .indent(DEFAULT_INDENT) ); sb.append("\n}\n"); } @@ -242,7 +243,7 @@ private static void generateFieldCaseStatement(final StringBuilder sb, final Fie sb.append("case " + tag +" /* type=" + wireType + " [" + field.type() + "] " + "field=" + fieldNum + " [" + field.name() + "] */ -> {\n"); if (field.optionalValueType()) { - sb.append(Common.FIELD_INDENT + """ + sb.append(""" // Read the message size, it is not needed final var valueTypeMessageSize = input.readVarInt(false); final $fieldType value; @@ -282,21 +283,21 @@ private static void generateFieldCaseStatement(final StringBuilder sb, final Fie case "DoubleValue" -> Common.TYPE_FIXED64; default -> throw new PbjCompilerException("Unexpected and unknown field type " + field.type() + " cannot be parsed"); })) - .replaceAll("\n","\n" + Common.FIELD_INDENT) + .indent(DEFAULT_INDENT) ); sb.append('\n'); } else if (field.type() == Field.FieldType.MESSAGE){ - sb.append(Common.FIELD_INDENT + """ + sb.append(""" final var messageLength = input.readVarInt(false); final var limitBefore = input.limit(); input.limit(input.position() + messageLength); final var value = $readMethod; input.limit(limitBefore);""" .replace("$readMethod", readMethod(field)) - .replaceAll("\n", "\n" + Common.FIELD_INDENT) + .indent(DEFAULT_INDENT) ); } else { - sb.append(Common.FIELD_INDENT + "final var value = " + readMethod(field) + ";\n"); + sb.append(("final var value = " + readMethod(field) + ";\n").indent(DEFAULT_INDENT)); } // set value to temp var sb.append(Common.FIELD_INDENT); diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecWriteMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecWriteMethodGenerator.java index 1a3b402c..bae54fac 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecWriteMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecWriteMethodGenerator.java @@ -1,5 +1,7 @@ package com.hedera.pbj.compiler.impl.generators.protobuf; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; + import com.hedera.pbj.compiler.impl.Common; import com.hedera.pbj.compiler.impl.Field; import com.hedera.pbj.compiler.impl.OneOfField; @@ -20,7 +22,8 @@ static String generateWriteMethod(final String modelClassName, final List .flatMap(field -> field.type() == Field.FieldType.ONE_OF ? ((OneOfField)field).fields().stream() : Stream.of(field)) .sorted(Comparator.comparingInt(Field::fieldNumber)) .map(field -> generateFieldWriteLines(field, modelClassName, "data.%s()".formatted(field.nameCamelFirstLower()))) - .collect(Collectors.joining("\n" + Common.FIELD_INDENT)); + .collect(Collectors.joining("\n")) + .indent(DEFAULT_INDENT); return """ /** @@ -36,7 +39,7 @@ public void write(@NonNull $modelClass data,@NonNull final WritableSequentialDat """ .replace("$modelClass", modelClassName) .replace("$fieldWriteLines", fieldWriteLines) - .replaceAll("\n", "\n" + Common.FIELD_INDENT); + .indent(DEFAULT_INDENT); } @@ -51,7 +54,7 @@ public void write(@NonNull $modelClass data,@NonNull final WritableSequentialDat private static String generateFieldWriteLines(final Field field, final String modelClassName, String getValueCode) { final String fieldDef = Common.camelToUpperSnake(field.name()); String prefix = "// ["+field.fieldNumber()+"] - "+field.name(); - prefix += "\n"+ Common.FIELD_INDENT.repeat(1); + prefix += "\n"; if (field.parent() != null) { final OneOfField oneOfField = field.parent(); @@ -59,7 +62,7 @@ private static String generateFieldWriteLines(final Field field, final String mo getValueCode = "data."+oneOfField.nameCamelFirstLower()+"().as()"; prefix += "if(data."+oneOfField.nameCamelFirstLower()+"().kind() == "+ oneOfType +"."+ Common.camelToUpperSnake(field.name())+")"; - prefix += "\n"+ Common.FIELD_INDENT.repeat(2); + prefix += "\n"; } final String writeMethodName = field.methodNameType(); diff --git a/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java index 71bf02db..46933674 100644 --- a/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java +++ b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/HashEqualsTest.java @@ -1,13 +1,15 @@ package com.hedera.pbj.intergration.test; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + import org.junit.jupiter.api.Test; import com.hedera.pbj.test.proto.pbj.TimestampTest; import com.hedera.pbj.test.proto.pbj.TimestampTest2; -import static org.junit.jupiter.api.Assertions.*; - -class HasjhEqualsTest { +class HashEqualsTest { @Test void differentObjectsWithDefaulEquals() { TimestampTest tst = new TimestampTest(1, 2); From dfa400b2799363a78650b385800a87461bdd8413 Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Mon, 13 Nov 2023 11:56:51 -0500 Subject: [PATCH 04/10] Addressed Artem's comment. Signed-off-by: Ivan Malygin --- .../hedera/pbj/runtime/io/buffer/BufferedData.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java index 65e810f0..19bae4bd 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/BufferedData.java @@ -372,13 +372,11 @@ public long getBytes(final long offset, @NonNull final BufferedData dst) { @Override public Bytes getBytes(long offset, long length) { final var len = Math.toIntExact(length); - if (direct) { - final var copy = new byte[len]; - buffer.get(Math.toIntExact(offset), copy, 0, len); - return Bytes.wrap(copy); - } else { - return Bytes.wrap(buffer.array(), Math.toIntExact(buffer.arrayOffset() + offset), len); - } + if(len < 0) throw new IllegalArgumentException("Length cannot be negative"); + // It is vital that we always copy here, we can never assume ownership of the underlying buffer + final var copy = new byte[len]; + buffer.get(Math.toIntExact(offset), copy, 0, len); + return Bytes.wrap(copy); } /** {@inheritDoc} */ From 7c69529149d1135afbffa5f7b682edf64d9a3666 Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Mon, 13 Nov 2023 13:23:35 -0500 Subject: [PATCH 05/10] Apply suggestions from code review Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com> Signed-off-by: Ivan Malygin --- .../com/hedera/pbj/compiler/impl/Common.java | 2 +- .../impl/generators/EnumGenerator.java | 9 ++++-- .../impl/generators/ModelGenerator.java | 29 ++++++++++++++----- .../intergration/jmh/EqualsHashCodeBench.java | 26 ++++++++--------- .../pbj/intergration/jmh/HashBench.java | 20 ++++++------- .../intergration/test/TestHashFunctions.java | 14 --------- 6 files changed, 52 insertions(+), 48 deletions(-) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index d6e39131..ce11f24f 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -436,7 +436,7 @@ else if (f.nameCamelFirstLower() != null) { """).replace("$fieldName", f.nameCamelFirstLower()); } else { - throw new RuntimeException("Unexpected field type for getting HashCode - " + f.type().toString()); + throw new IllegalArgumentException("Unexpected field type for getting HashCode - " + f.type().toString()); } } } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java index 662ad0ff..b47867bd 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java @@ -71,7 +71,8 @@ public static void generateEnumFile(Protobuf3Parser.EnumDefContext enumDef, File try (FileWriter javaWriter = new FileWriter(getJavaFile(destinationSrcDir, modelPackage, enumName))) { javaWriter.write( "package "+modelPackage+";\n\n"+ - createEnum(javaDocComment, deprecated, enumName, maxIndex, enumValues, false) + createEnum(javaDocComment, deprecated, enumName, + maxIndex, enumValues, false) ); } } @@ -110,12 +111,14 @@ static String createEnum(String javaDocComment, String deprecated, String enumNa final String deprecatedText = enumValue.deprecated ? "@Deprecated\n" : ""; enumValuesCode.add( cleanedEnumComment - + deprecatedText+ camelToUpperSnake(enumValue.name) +"("+i+", \""+enumValue.name+"\")"); + + deprecatedText+ camelToUpperSnake(enumValue.name) + + "("+i+", \""+enumValue.name+"\")"); } } return """ $javaDocComment - $deprecated$public enum $enumName implements com.hedera.pbj.runtime.EnumWithProtoMetadata { + $deprecated$public enum $enumName + implements com.hedera.pbj.runtime.EnumWithProtoMetadata { $enumValues; /** The field ordinal in protobuf for this type */ diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java index 0663da2d..4855a6d7 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java @@ -1,20 +1,35 @@ package com.hedera.pbj.compiler.impl.generators; -import com.hedera.pbj.compiler.impl.*; +import static com.hedera.pbj.compiler.impl.Common.DEFAULT_INDENT; +import static com.hedera.pbj.compiler.impl.Common.camelToUpperSnake; +import static com.hedera.pbj.compiler.impl.Common.cleanDocStr; +import static com.hedera.pbj.compiler.impl.Common.getFieldsHashCode; +import static com.hedera.pbj.compiler.impl.Common.getJavaFile; +import static com.hedera.pbj.compiler.impl.Common.javaPrimitiveToObjectType; +import static com.hedera.pbj.compiler.impl.generators.EnumGenerator.EnumValue; +import static com.hedera.pbj.compiler.impl.generators.EnumGenerator.createEnum; + +import com.hedera.pbj.compiler.impl.Common; +import com.hedera.pbj.compiler.impl.ContextualLookupHelper; +import com.hedera.pbj.compiler.impl.Field; import com.hedera.pbj.compiler.impl.Field.FieldType; +import com.hedera.pbj.compiler.impl.FileType; +import com.hedera.pbj.compiler.impl.OneOfField; +import com.hedera.pbj.compiler.impl.SingleField; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser; import edu.umd.cs.findbugs.annotations.SuppressWarnings; import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; -import static com.hedera.pbj.compiler.impl.Common.*; -import static com.hedera.pbj.compiler.impl.generators.EnumGenerator.EnumValue; -import static com.hedera.pbj.compiler.impl.generators.EnumGenerator.createEnum; - /** * Code generator that parses protobuf files and generates nice Java source for record files for each message type and * enum. @@ -349,7 +364,7 @@ public int hashCode() { if (!hashCodeGenerated) { // Generate a call to private method that iterates through fields // and calculates the hashcode. - statements = Common.getFieldsHashCode(fields, statements); + statements = getFieldsHashCode(fields, statements); bodyContent += """ diff --git a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java index 66584db7..7ec979b8 100644 --- a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java +++ b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java @@ -1,23 +1,21 @@ package com.hedera.pbj.intergration.jmh; -import com.google.protobuf.CodedInputStream; -import com.google.protobuf.CodedOutputStream; -import com.hedera.pbj.intergration.test.TestHashFunctions; -import com.hedera.pbj.runtime.MalformedProtobufException; -import com.hedera.pbj.runtime.io.buffer.BufferedData; -import com.hedera.pbj.runtime.io.buffer.Bytes; -import com.hedera.pbj.runtime.io.stream.ReadableStreamingData; -import com.hedera.pbj.test.proto.pbj.Hasheval; import com.hedera.pbj.test.proto.pbj.Suit; import com.hedera.pbj.test.proto.pbj.TimestampTest; -import com.hedera.pbj.test.proto.pbj.tests.HashevalTest; -import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OperationsPerInvocation; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Scope; +// Add any other JMH annotation imports you use +import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.infra.Blackhole; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.nio.*; -import java.util.Random; import java.util.concurrent.TimeUnit; @SuppressWarnings("unused") @@ -29,6 +27,8 @@ @BenchmarkMode(Mode.AverageTime) public class EqualsHashCodeBench { private TimestampTest testStamp; + private TimestampTest testStamp; + private TimestampTest testStamp1; public EqualsHashCodeBench() { diff --git a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java index beb3970a..f2fa35f4 100644 --- a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java +++ b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/HashBench.java @@ -1,23 +1,23 @@ package com.hedera.pbj.intergration.jmh; -import com.google.protobuf.CodedInputStream; -import com.google.protobuf.CodedOutputStream; import com.hedera.pbj.intergration.test.TestHashFunctions; -import com.hedera.pbj.runtime.MalformedProtobufException; -import com.hedera.pbj.runtime.io.buffer.BufferedData; import com.hedera.pbj.runtime.io.buffer.Bytes; -import com.hedera.pbj.runtime.io.stream.ReadableStreamingData; import com.hedera.pbj.test.proto.pbj.Hasheval; import com.hedera.pbj.test.proto.pbj.Suit; import com.hedera.pbj.test.proto.pbj.TimestampTest; -import com.hedera.pbj.test.proto.pbj.tests.HashevalTest; -import org.openjdk.jmh.annotations.*; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OperationsPerInvocation; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.infra.Blackhole; -import java.io.ByteArrayInputStream; import java.io.IOException; -import java.nio.*; -import java.util.Random; import java.util.concurrent.TimeUnit; @SuppressWarnings("unused") diff --git a/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java index 3d81b383..46751c57 100644 --- a/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java +++ b/pbj-integration-tests/src/test/java/com/hedera/pbj/intergration/test/TestHashFunctions.java @@ -1,25 +1,11 @@ package com.hedera.pbj.intergration.test; -import com.google.protobuf.CodedOutputStream; -import com.hedera.hapi.node.base.Timestamp; -import com.hedera.pbj.runtime.io.buffer.BufferedData; -import com.hedera.pbj.runtime.test.NoToStringWrapper; import com.hedera.pbj.test.proto.pbj.Hasheval; import com.hedera.pbj.test.proto.pbj.TimestampTest; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; import java.nio.ByteBuffer; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.util.List; -import java.util.stream.IntStream; -import java.util.stream.Stream; -import static com.hedera.pbj.runtime.ProtoTestTools.INTEGER_TESTS_LIST; -import static com.hedera.pbj.runtime.ProtoTestTools.LONG_TESTS_LIST; -import static com.hedera.pbj.runtime.ProtoTestTools.getThreadLocalByteBuffer; -import static com.hedera.pbj.runtime.ProtoTestTools.getThreadLocalDataBuffer; -import static com.hedera.pbj.runtime.ProtoTestTools.getThreadLocalDataBuffer2; -import static org.junit.jupiter.api.Assertions.assertEquals; /** * Unit Test for TimestampTest model object. Generate based on protobuf schema. From 921e6c672407027f2772cb925449b921011ac027 Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Tue, 14 Nov 2023 16:02:27 -0500 Subject: [PATCH 06/10] Addressed review comments. Signed-off-by: Ivan Malygin --- .../com/hedera/pbj/compiler/impl/Common.java | 24 +++++++-------- .../impl/generators/ModelGenerator.java | 30 ++++++++++++------- .../hedera/pbj/runtime/io/buffer/Bytes.java | 2 +- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index ce11f24f..5bf3d339 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -287,43 +287,43 @@ private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, F switch (f.messageType()) { case "StringValue" -> generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { + if (!DEFAULT.$fieldName.equals($fieldName)) { result = 31 * result + $fieldName.hashCode(); } """).replace("$fieldName", f.nameCamelFirstLower()); case "BoolValue" -> generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { + if (!DEFAULT.$fieldName.equals($fieldName)) { result = 31 * result + Boolean.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "Int32Value", "UInt32Value" -> generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { + if (DEFAULT.$fieldName.equals($fieldName)) { result = 31 * result + Integer.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "Int64Value", "UInt64Value" -> generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { + if (DEFAULT.$fieldName.equals($fieldName)) { result = 31 * result + Long.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "FloatValue" -> generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { + if (DEFAULT.$fieldName.equals($fieldName)) { result = 31 * result + Float.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "DoubleValue" -> generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { + if (DEFAULT.$fieldName.equals($fieldName)) { result = 31 * result + Double.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "BytesValue" -> generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { + if (DEFAULT.$fieldName.equals($fieldName)){ result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); } """).replace("$fieldName", f.nameCamelFirstLower()); @@ -344,12 +344,10 @@ private static String getRepeatedHashCodeGeneration(String generatedCodeSoFar, F """ java.util.List list$$fieldName = $fieldName; for (Object o : list$$fieldName) { - if ($fieldName != DEFAULT.$fieldName) { - if (o != null) { - result = 31 * result; - } else { - result = 31 * result + o.hashCode(); - } + if (o != null) { + result = 31 * result + o.hashCode(); + } else { + result = 31 * result; } } """).replace("$fieldName", f.nameCamelFirstLower()); diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java index 4855a6d7..1c1f8eec 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java @@ -299,7 +299,7 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, FieldType fieldType = fields.get(0).type(); switch (fieldType) { case INT32, UINT32, SINT32, FIXED32, SFIXED32, - FIXED64, SFIXED64, INT64, UINT64, SINT64 -> { + FIXED64, SFIXED64, INT64, UINT64, SINT64 -> bodyContent += """ /** @@ -315,8 +315,7 @@ public int hashCode() { """.replace("$fieldName", fields.get(0).name()) .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) .indent(DEFAULT_INDENT); - } - case FLOAT, DOUBLE -> { + case FLOAT, DOUBLE -> bodyContent += """ /** @@ -332,8 +331,7 @@ public int hashCode() { """.replace("$fieldName", fields.get(0).name()) .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) .indent(DEFAULT_INDENT); - } - case STRING -> { + case STRING -> bodyContent += """ /** @@ -349,12 +347,24 @@ public int hashCode() { """ .replace("$fieldName", fields.get(0).name()) .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) - .indent(DEFAULT_INDENT) - ; - } - default -> { + .indent(DEFAULT_INDENT); + case BOOL -> + bodyContent += + """ + /** + * Override the default hashCode method for + * single field boolean objects. + */ + @Override + public int hashCode() { + return $fieldName ? 1 : 0; + } + """ + .replace("$fieldName", fields.get(0).name()) + .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) + .indent(DEFAULT_INDENT); + default -> hashCodeGenerated = false; - } } } else { hashCodeGenerated = false; diff --git a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java index bf9c03f6..8c03c41f 100644 --- a/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java +++ b/pbj-core/pbj-runtime/src/main/java/com/hedera/pbj/runtime/io/buffer/Bytes.java @@ -355,7 +355,7 @@ public int read() throws IOException { @Override @NonNull public String toString() { - return HexFormat.of().formatHex(buffer, start, length); + return HexFormat.of().formatHex(buffer, start, start + length); } /** From e21de9668a6e0d6edcc4cda12d4033a2f6998fbb Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Mon, 20 Nov 2023 16:56:36 -0500 Subject: [PATCH 07/10] Added patch from Jasper. Addressed Jasper's comments. Changed generated code directory layout to match Maven. Signed-off-by: Ivan Malygin --- .../pbj/compiler/PbjCompilerPlugin.java | 6 +- .../com/hedera/pbj/compiler/impl/Common.java | 122 ++++---- .../impl/generators/EnumGenerator.java | 147 ++++----- .../impl/generators/ModelGenerator.java | 132 ++++---- .../impl/generators/SchemaGenerator.java | 38 +-- .../impl/generators/TestGenerator.java | 296 +++++++++--------- .../generators/json/JsonCodecGenerator.java | 6 +- .../generators/protobuf/CodecGenerator.java | 24 +- .../protobuf/CodecParseMethodGenerator.java | 29 +- .../jmh/ComplexEqualsHashCodeBench.java | 155 +++++++++ .../intergration/jmh/EqualsHashCodeBench.java | 69 +++- 11 files changed, 608 insertions(+), 416 deletions(-) create mode 100644 pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/ComplexEqualsHashCodeBench.java diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/PbjCompilerPlugin.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/PbjCompilerPlugin.java index c641b20c..caa48d2a 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/PbjCompilerPlugin.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/PbjCompilerPlugin.java @@ -49,11 +49,11 @@ public void apply(Project project) { // get java src sets final var mainSrcSet = javaPlugin.getSourceSets().getByName(SourceSet.MAIN_SOURCE_SET_NAME); final var testSrcSet = javaPlugin.getSourceSets().getByName(SourceSet.TEST_SOURCE_SET_NAME); - final String outputDirectory = "generated/source/pbj-proto/main/"; + final String outputDirectory = "generated/source/pbj-proto/"; final Provider outputDirectoryMain = - project.getLayout().getBuildDirectory().dir(outputDirectory + "java"); + project.getLayout().getBuildDirectory().dir(outputDirectory + "main/java"); final Provider outputDirectoryTest = - project.getLayout().getBuildDirectory().dir(outputDirectory + "test"); + project.getLayout().getBuildDirectory().dir(outputDirectory + "test/java"); // for the 'main' source set we: // 1) Add a new 'pbj' virtual directory mapping diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index 5bf3d339..e2d6d415 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -203,7 +203,7 @@ else if (f.nameCamelFirstLower() != null) { generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Integer.hashCode($fieldName); + result = 31 * result + Integer.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FIXED64 || @@ -214,57 +214,57 @@ else if (f.nameCamelFirstLower() != null) { generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Long.hashCode($fieldName); + result = 31 * result + Long.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.ENUM) { generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + $fieldName.hashCode(); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + $fieldName.hashCode(); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BOOL) { generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Boolean.hashCode($fieldName); + result = 31 * result + Boolean.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FLOAT) { generatedCodeSoFar += ( - """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Float.hashCode($fieldName); + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Float.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.DOUBLE) { generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Double.hashCode($fieldName); + result = 31 * result + Double.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.STRING) { generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + $fieldName.hashCode(); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + $fieldName.hashCode(); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BYTES) { generatedCodeSoFar += ( """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.parent() == null) { // process sub message generatedCodeSoFar += ( """ - if ($fieldName != null && $fieldName != DEFAULT.$fieldName) { - result = 31 * result + $fieldName.hashCode(); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + $fieldName.hashCode(); } """).replace("$fieldName", f.nameCamelFirstLower()); } @@ -287,44 +287,44 @@ private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, F switch (f.messageType()) { case "StringValue" -> generatedCodeSoFar += ( """ - if (!DEFAULT.$fieldName.equals($fieldName)) { - result = 31 * result + $fieldName.hashCode(); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + $fieldName.hashCode(); } """).replace("$fieldName", f.nameCamelFirstLower()); case "BoolValue" -> generatedCodeSoFar += ( """ - if (!DEFAULT.$fieldName.equals($fieldName)) { - result = 31 * result + Boolean.hashCode($fieldName); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + Boolean.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "Int32Value", "UInt32Value" -> generatedCodeSoFar += ( """ - if (DEFAULT.$fieldName.equals($fieldName)) { - result = 31 * result + Integer.hashCode($fieldName); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + Integer.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "Int64Value", "UInt64Value" -> generatedCodeSoFar += ( """ - if (DEFAULT.$fieldName.equals($fieldName)) { - result = 31 * result + Long.hashCode($fieldName); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + Long.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "FloatValue" -> generatedCodeSoFar += ( """ - if (DEFAULT.$fieldName.equals($fieldName)) { - result = 31 * result + Float.hashCode($fieldName); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + Float.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "DoubleValue" -> generatedCodeSoFar += ( """ - if (DEFAULT.$fieldName.equals($fieldName)) { - result = 31 * result + Double.hashCode($fieldName); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + Double.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); case "BytesValue" -> generatedCodeSoFar += ( """ - if (DEFAULT.$fieldName.equals($fieldName)){ - result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); } """).replace("$fieldName", f.nameCamelFirstLower()); default -> throw new UnsupportedOperationException("Unhandled optional message type:" + f.messageType()); @@ -344,11 +344,11 @@ private static String getRepeatedHashCodeGeneration(String generatedCodeSoFar, F """ java.util.List list$$fieldName = $fieldName; for (Object o : list$$fieldName) { - if (o != null) { - result = 31 * result + o.hashCode(); - } else { - result = 31 * result; - } + if (o != null) { + result = 31 * result + o.hashCode(); + } else { + result = 31 * result; + } } """).replace("$fieldName", f.nameCamelFirstLower()); return generatedCodeSoFar; @@ -383,7 +383,7 @@ else if (f.nameCamelFirstLower() != null) { generatedCodeSoFar += """ if ($fieldName != thatObj.$fieldName) { - return false; + return false; } """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FIXED64 || @@ -394,14 +394,14 @@ else if (f.nameCamelFirstLower() != null) { generatedCodeSoFar += """ if ($fieldName != thatObj.$fieldName) { - return false; + return false; } """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BOOL) { generatedCodeSoFar += """ if ($fieldName != thatObj.$fieldName) { - return false; + return false; } """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.DOUBLE) { @@ -409,14 +409,14 @@ else if (f.nameCamelFirstLower() != null) { generatedCodeSoFar += """ if ($fieldName != thatObj.$fieldName) { - return false; + return false; } """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.DOUBLE) { generatedCodeSoFar += """ if ($fieldName != thatObj.$fieldName) { - return false; + return false; } """.replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.STRING || @@ -426,10 +426,10 @@ else if (f.nameCamelFirstLower() != null) { generatedCodeSoFar += ( """ if ($fieldName == null && thatObj.$fieldName != null) { - return false; + return false; } if ($fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; + return false; } """).replace("$fieldName", f.nameCamelFirstLower()); } @@ -454,48 +454,48 @@ private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Fie generatedCodeSoFar += ( """ if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; + return false; } if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; + return false; } """).replace("$fieldName", f.nameCamelFirstLower()); case "BoolValue" -> generatedCodeSoFar += ( """ if ($fieldName instanceof Object) { - if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; - } - if (!$fieldName.equals(thatObj.$fieldName)) { - return false; - } + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (!$fieldName.equals(thatObj.$fieldName)) { + return false; + } } else if ($fieldName != thatObj.$fieldName) { - return false; + return false; } """).replace("$fieldName", f.nameCamelFirstLower()); case "Int32Value", "UInt32Value", "Int64Value", "UInt64Value", "FloatValue", "DoubleValue" -> generatedCodeSoFar += ( """ if ($fieldName instanceof Object) { - if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; - } - if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } } else if ($fieldName != thatObj.$fieldName) { - return false; + return false; } """).replace("$fieldName", f.nameCamelFirstLower()); case "BytesValue" -> generatedCodeSoFar += ( """ if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; + return false; } if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; + return false; } """).replace("$fieldName", f.nameCamelFirstLower()); default -> throw new UnsupportedOperationException("Unhandled optional message type:" + f.messageType()); @@ -515,11 +515,11 @@ private static String getRepeatedEqualsGeneration(String generatedCodeSoFar, Fie generatedCodeSoFar += ( """ if (this.$fieldName == null && this.$fieldName != null) { - return false; + return false; } if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; + return false; } """).replace("$fieldName", f.nameCamelFirstLower()); return generatedCodeSoFar; diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java index b47867bd..473bdac7 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/EnumGenerator.java @@ -94,10 +94,10 @@ static String createEnum(String javaDocComment, String deprecated, String enumNa if (addUnknown) { enumValuesCode.add( """ - /** - * Enum value for a unset OneOf, to avoid null OneOfs - */ - UNSET(-1, "UNSET")"""); + /** + * Enum value for a unset OneOf, to avoid null OneOfs + */ + UNSET(-1, "UNSET")"""); } for (int i = 0; i <= maxIndex; i++) { final EnumValue enumValue = enumValues.get(i); @@ -118,85 +118,90 @@ static String createEnum(String javaDocComment, String deprecated, String enumNa return """ $javaDocComment $deprecated$public enum $enumName - implements com.hedera.pbj.runtime.EnumWithProtoMetadata { + implements com.hedera.pbj.runtime.EnumWithProtoMetadata { $enumValues; - - /** The field ordinal in protobuf for this type */ - private final int protoOrdinal; - - /** The original field name in protobuf for this type */ - private final String protoName; - - /** - * OneOf Type Enum Constructor - * - * @param protoOrdinal The oneof field ordinal in protobuf for this type - * @param protoName The original field name in protobuf for this type - */ - $enumName(final int protoOrdinal, String protoName) { - this.protoOrdinal = protoOrdinal; - this.protoName = protoName; - } - - /** - * Get the oneof field ordinal in protobuf for this type - * - * @return The oneof field ordinal in protobuf for this type - */ - public int protoOrdinal() { - return protoOrdinal; - } - - /** - * Get the original field name in protobuf for this type - * - * @return The original field name in protobuf for this type - */ - public String protoName() { - return protoName; - } - - /** - * Get enum from protobuf ordinal - * - * @param ordinal the protobuf ordinal number - * @return enum for matching ordinal - * @throws IllegalArgumentException if ordinal doesn't exist - */ - public static $enumName fromProtobufOrdinal(int ordinal) { - return switch(ordinal) { + + /** The field ordinal in protobuf for this type */ + private final int protoOrdinal; + + /** The original field name in protobuf for this type */ + private final String protoName; + + /** + * OneOf Type Enum Constructor + * + * @param protoOrdinal The oneof field ordinal in protobuf for this type + * @param protoName The original field name in protobuf for this type + */ + $enumName(final int protoOrdinal, String protoName) { + this.protoOrdinal = protoOrdinal; + this.protoName = protoName; + } + + /** + * Get the oneof field ordinal in protobuf for this type + * + * @return The oneof field ordinal in protobuf for this type + */ + public int protoOrdinal() { + return protoOrdinal; + } + + /** + * Get the original field name in protobuf for this type + * + * @return The original field name in protobuf for this type + */ + public String protoName() { + return protoName; + } + + /** + * Get enum from protobuf ordinal + * + * @param ordinal the protobuf ordinal number + * @return enum for matching ordinal + * @throws IllegalArgumentException if ordinal doesn't exist + */ + public static $enumName fromProtobufOrdinal(int ordinal) { + return switch(ordinal) { $caseStatements - default -> throw new IllegalArgumentException("Unknown protobuf ordinal "+ordinal); - }; - } - - /** - * Get enum from string name, supports the enum or protobuf format name - * - * @param name the enum or protobuf format name - * @return enum for matching name - */ - public static $enumName fromString(String name) { - return switch(name) { + default -> throw new IllegalArgumentException("Unknown protobuf ordinal "+ordinal); + }; + } + + /** + * Get enum from string name, supports the enum or protobuf format name + * + * @param name the enum or protobuf format name + * @return enum for matching name + */ + public static $enumName fromString(String name) { + return switch(name) { $fromStringCaseStatements - default -> throw new IllegalArgumentException("Unknown token kyc status "+name); - }; - } + default -> throw new IllegalArgumentException("Unknown token kyc status "+name); + }; + } } """ .replace("$javaDocComment", javaDocComment) .replace("$deprecated$", deprecated) .replace("$enumName", enumName) .replace("$enumValues", String.join(",\n\n", enumValuesCode).indent(DEFAULT_INDENT)) - .replace("$caseStatements", enumValues.entrySet().stream().map((entry) -> " case " + entry.getKey() + " -> " + - camelToUpperSnake(entry.getValue().name) + ";").collect(Collectors.joining("\n"))) + .replace("$caseStatements", enumValues.entrySet() + .stream() + .map((entry) -> "case %s -> %s;".formatted(entry.getKey(), camelToUpperSnake(entry.getValue().name)) + .indent(DEFAULT_INDENT * 3)) + .collect(Collectors.joining("\n"))) .replace("$fromStringCaseStatements", enumValues.values().stream().map(enumValue -> { if (camelToUpperSnake(enumValue.name).equals(enumValue.name)) { - return " case \"" + enumValue.name + "\" -> " + - camelToUpperSnake(enumValue.name) + ";"; + return "case \"%s\" -> %s;" + .formatted(enumValue.name, camelToUpperSnake(enumValue.name)) + .indent(DEFAULT_INDENT * 3); } else { - return " case \"" + camelToUpperSnake(enumValue.name) + "\", \"" + enumValue.name + "\" -> " + - camelToUpperSnake(enumValue.name) + ";"; + return "case \"%s\", \"%s\" -> %s;" + .formatted(enumValue.name, camelToUpperSnake(enumValue.name), camelToUpperSnake(enumValue.name)) + .indent(DEFAULT_INDENT * 3); } }).collect(Collectors.joining("\n"))); } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java index 1c1f8eec..74f1b720 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/ModelGenerator.java @@ -163,7 +163,7 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, * Enum for the type of "%s" oneof value */""".formatted(oneOfField.name()); final String enumString = createEnum(enumComment ,"",enumName,maxIndex,enumValues, true) - .indent(DEFAULT_INDENT); + .indent(DEFAULT_INDENT * 2); oneofEnums.add(enumString); fields.add(oneOfField); imports.add("com.hedera.pbj.runtime"); @@ -302,16 +302,16 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, FIXED64, SFIXED64, INT64, UINT64, SINT64 -> bodyContent += """ - /** - * Override the default hashCode method for - * single field int objects. - */ - @Override - public int hashCode() { - long hashCode = $fieldName; - $hashCodeManipulation - return (int)hashCode; - } + /** + * Override the default hashCode method for + * single field int objects. + */ + @Override + public int hashCode() { + long hashCode = $fieldName; + $hashCodeManipulation + return (int)hashCode; + } """.replace("$fieldName", fields.get(0).name()) .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) .indent(DEFAULT_INDENT); @@ -323,10 +323,10 @@ public int hashCode() { * single field float and double objects. */ @Override - public int hashCode() { - double hashCode = $fieldName; + public int hashCode() { + double hashCode = $fieldName; $hashCodeManipulation - return (int)hashCode; + return (int)hashCode; } """.replace("$fieldName", fields.get(0).name()) .replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) @@ -339,10 +339,10 @@ public int hashCode() { * single field String objects. */ @Override - public int hashCode() { - long hashCode = $fieldName.hashCode(); + public int hashCode() { + long hashCode = $fieldName.hashCode(); $hashCodeManipulation - return (int)hashCode; + return (int)hashCode; } """ .replace("$fieldName", fields.get(0).name()) @@ -356,8 +356,8 @@ public int hashCode() { * single field boolean objects. */ @Override - public int hashCode() { - return $fieldName ? 1 : 0; + public int hashCode() { + return $fieldName ? 1 : 0; } """ .replace("$fieldName", fields.get(0).name()) @@ -379,21 +379,21 @@ public int hashCode() { bodyContent += """ /** - * Override the default hashCode method for - * all other objects to make hashCode - */ - @Override - public int hashCode() { - int result = 1; + * Override the default hashCode method for + * all other objects to make hashCode + */ + @Override + public int hashCode() { + int result = 1; """.indent(DEFAULT_INDENT); bodyContent += statements; bodyContent += """ - long hashCode = result; + long hashCode = result; $hashCodeManipulation - return (int)hashCode; + return (int)hashCode; } """.replace("$hashCodeManipulation", HASH_CODE_MANIPULATION) .indent(DEFAULT_INDENT); @@ -411,17 +411,17 @@ public int hashCode() { */ @Override public boolean equals(Object that) { - if (that == null || this.getClass() != that.getClass()) { - return false; - } - $javaRecordName thatObj = ($javaRecordName)that; + if (that == null || this.getClass() != that.getClass()) { + return false; + } + $javaRecordName thatObj = ($javaRecordName)that; """.replace("$javaRecordName", javaRecordName).indent(DEFAULT_INDENT); bodyContent += equalsStatements.indent(DEFAULT_INDENT); bodyContent += """ - return true; - } + return true; + } """.indent(DEFAULT_INDENT); // Has methods @@ -442,7 +442,7 @@ public boolean equals(Object that) { * @return a pre-populated builder */ public Builder copyBuilder() { - return new Builder(%s); + return new Builder(%s); } /** @@ -451,7 +451,7 @@ public Builder copyBuilder() { * @return a new builder */ public static Builder newBuilder() { - return new Builder(); + return new Builder(); } """ .formatted(fields.stream().map(Field::nameCamelFirstLower).collect(Collectors.joining(", "))) @@ -499,7 +499,7 @@ private static void generateBuilderMethods(List builderMethods, Field fi final OneOfField parentOneOfField = field.parent(); if (parentOneOfField != null) { final String oneOfEnumValue = parentOneOfField.getEnumClassRef()+"."+camelToUpperSnake(field.name()); - prefix = "new OneOf<>("+oneOfEnumValue+","; + prefix = " new OneOf<>("+oneOfEnumValue+","; postfix = ")"; fieldToSet = parentOneOfField.nameCamelFirstLower(); } else { @@ -515,8 +515,8 @@ private static void generateBuilderMethods(List builderMethods, Field fi * @return builder to continue building with */ public Builder $fieldName($fieldType $fieldName) { - this.$fieldToSet = $prefix$fieldName$postfix; - return this; + this.$fieldToSet = $prefix$fieldName$postfix; + return this; }""" .replace("$fieldDoc",field.comment() .replaceAll("\n", "\n * ")) @@ -537,8 +537,8 @@ private static void generateBuilderMethods(List builderMethods, Field fi * @return builder to continue building with */ public Builder $fieldName($messageClass.Builder builder) { - this.$fieldToSet = $prefix builder.build() $postfix; - return this; + this.$fieldToSet =$prefix builder.build() $postfix; + return this; }""" .replace("$messageClass",field.messageType()) .replace("$fieldDoc",field.comment() @@ -562,8 +562,8 @@ private static void generateBuilderMethods(List builderMethods, Field fi * @return builder to continue building with */ public Builder $fieldName($baseType ... values) { - this.$fieldToSet = $prefix List.of(values) $postfix; - return this; + this.$fieldToSet = $prefix List.of(values) $postfix; + return this; }""" .replace("$baseType",field.javaFieldType().substring("List<".length(),field.javaFieldType().length()-1)) .replace("$fieldDoc",field.comment() @@ -597,30 +597,30 @@ private static String generateBuilder(final Protobuf3Parser.MessageDefContext ms * paths use the constructor directly. */ public static final class Builder { - $fields; + $fields; - /** - * Create an empty builder - */ - public Builder() {} + /** + * Create an empty builder + */ + public Builder() {} - /** - * Create a pre-populated builder - * $constructorParamDocs - */ - public Builder($constructorParams) { - $constructorCode } + /** + * Create a pre-populated builder + * $constructorParamDocs + */ + public Builder($constructorParams) { + $constructorCode } - /** - * Build a new model record with data set on builder - * - * @return new model record with data set - */ - public $javaRecordName build() { - return new $javaRecordName($recordParams); - } + /** + * Build a new model record with data set on builder + * + * @return new model record with data set + */ + public $javaRecordName build() { + return new $javaRecordName($recordParams); + } - $builderMethods}""" + $builderMethods}""" .replace("$fields", fields.stream().map(field -> "private " + field.javaFieldType() + " " + field.nameCamelFirstLower() + " = " + getDefaultValue(field, msgDef, lookupHelper) @@ -651,9 +651,9 @@ private static String getDefaultValue(Field field, final Protobuf3Parser.Message private static String generateConstructorCode(final Field f) { StringBuilder sb = new StringBuilder(""" - if ($fieldName == null) { - throw new NullPointerException("Parameter '$fieldName' must be supplied and can not be null"); - }""".replace("$fieldName", f.nameCamelFirstLower())); + if ($fieldName == null) { + throw new NullPointerException("Parameter '$fieldName' must be supplied and can not be null"); + }""".replace("$fieldName", f.nameCamelFirstLower())); if (f instanceof final OneOfField oof) { for (Field subField: oof.fields()) { if(subField.optionalValueType()) { @@ -662,7 +662,7 @@ private static String generateConstructorCode(final Field f) { // handle special case where protobuf does not have destination between a OneOf with optional // value of empty vs an unset OneOf. if($fieldName.kind() == $fieldUpperNameOneOfType.$subFieldNameUpper && $fieldName.value() == null) { - $fieldName = new OneOf<>($fieldUpperNameOneOfType.UNSET, null); + $fieldName = new OneOf<>($fieldUpperNameOneOfType.UNSET, null); }""" .replace("$fieldName", f.nameCamelFirstLower()) .replace("$fieldUpperName", f.nameCamelFirstUpper()) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/SchemaGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/SchemaGenerator.java index a89e88cd..93dc70cb 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/SchemaGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/SchemaGenerator.java @@ -66,22 +66,22 @@ public void generate(final Protobuf3Parser.MessageDefContext msgDef, final File */ public final class $schemaClassName implements Schema { - // -- FIELD DEFINITIONS --------------------------------------------- - + // -- FIELD DEFINITIONS --------------------------------------------- + $fields - - // -- OTHER METHODS ------------------------------------------------- - - /** - * Check if a field definition belongs to this schema. - * - * @param f field def to check - * @return true if it belongs to this schema - */ - public static boolean valid(FieldDefinition f) { - return f != null && getField(f.number()) == f; - } - + + // -- OTHER METHODS ------------------------------------------------- + + /** + * Check if a field definition belongs to this schema. + * + * @param f field def to check + * @return true if it belongs to this schema + */ + public static boolean valid(FieldDefinition f) { + return f != null && getField(f.number()) == f; + } + $getMethods } """ @@ -114,10 +114,10 @@ private static String generateGetField(final List flattenedFields) { * @return field def or null if field number does not exist */ public static FieldDefinition getField(final int fieldNumber) { - return switch(fieldNumber) { - %s - default -> null; - }; + return switch(fieldNumber) { + %s + default -> null; + }; } """.formatted(flattenedFields.stream() .map(Field::schemaGetFieldsDefCase) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java index 962823f2..b9884069 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java @@ -4,6 +4,7 @@ import com.hedera.pbj.compiler.impl.*; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser; +import kotlin.reflect.jvm.internal.impl.protobuf.CodedOutputStream; import java.io.File; import java.io.FileWriter; @@ -13,6 +14,7 @@ import java.util.Set; import java.util.TreeSet; import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Code generator that parses protobuf files and generates unit tests for each message type. @@ -58,37 +60,38 @@ public void generate(Protobuf3Parser.MessageDefContext msgDef, File destinationS imports.add("java.util"); try (FileWriter javaWriter = new FileWriter(javaFile)) { javaWriter.write(""" - package %s; - - import com.google.protobuf.util.JsonFormat; - import com.google.protobuf.CodedOutputStream; - import com.hedera.pbj.runtime.io.buffer.BufferedData; - import com.hedera.pbj.runtime.JsonTools; - import org.junit.jupiter.params.ParameterizedTest; - import org.junit.jupiter.params.provider.MethodSource; - import com.hedera.pbj.runtime.test.*; - import java.util.stream.IntStream; - import java.util.stream.Stream; - import java.nio.ByteBuffer; - import java.nio.CharBuffer; - %s - - import com.google.protobuf.CodedInputStream; - import com.google.protobuf.WireFormat; - import java.io.IOException; - import java.nio.charset.StandardCharsets; - - import static com.hedera.pbj.runtime.ProtoTestTools.*; - import static org.junit.jupiter.api.Assertions.*; + package %s; + + import com.google.protobuf.util.JsonFormat; + import com.google.protobuf.CodedOutputStream; + import com.hedera.pbj.runtime.io.buffer.BufferedData; + import com.hedera.pbj.runtime.JsonTools; + import org.junit.jupiter.api.Test; + import org.junit.jupiter.params.ParameterizedTest; + import org.junit.jupiter.params.provider.MethodSource; + import com.hedera.pbj.runtime.test.*; + import java.util.stream.IntStream; + import java.util.stream.Stream; + import java.nio.ByteBuffer; + import java.nio.CharBuffer; + %s - /** - * Unit Test for %s model object. Generate based on protobuf schema. - */ - public final class %s { - %s - %s - } - """.formatted( + import com.google.protobuf.CodedInputStream; + import com.google.protobuf.WireFormat; + import java.io.IOException; + import java.nio.charset.StandardCharsets; + + import static com.hedera.pbj.runtime.ProtoTestTools.*; + import static org.junit.jupiter.api.Assertions.*; + + /** + * Unit Test for %s model object. Generate based on protobuf schema. + */ + public final class %s { + %s + %s + } + """.formatted( testPackage, imports.isEmpty() ? "" : imports.stream() .filter(input -> !input.equals(testPackage)) @@ -112,16 +115,16 @@ private static String generateModelTestArgumentsMethod(final String modelClassNa public static final List<%s> ARGUMENTS; static { - %s - // work out the longest of all the lists of args as that is how many test cases we need - final int maxValues = IntStream.of( - %s - ).max().getAsInt(); - // create new stream of model objects using lists above as constructor params - ARGUMENTS = IntStream.range(0,maxValues) - .mapToObj(i -> new %s( - %s - )).toList(); + %s + // work out the longest of all the lists of args as that is how many test cases we need + final int maxValues = IntStream.of( + %s + ).max().getAsInt(); + // create new stream of model objects using lists above as constructor params + ARGUMENTS = IntStream.range(0,maxValues) + .mapToObj(i -> new %s( + %s + )).toList(); } /** @@ -136,7 +139,7 @@ private static String generateModelTestArgumentsMethod(final String modelClassNa """.formatted( modelClassName, fields.stream() - .map(f -> "final var "+f.nameCamelFirstLower()+"List = "+generateTestData(modelClassName, f, f.optionalValueType(), f.repeated())+";") + .map(f -> "final var %sList = %s;".formatted(f.nameCamelFirstLower(), generateTestData(modelClassName, f, f.optionalValueType(), f.repeated()))) .collect(Collectors.joining("\n")).indent(DEFAULT_INDENT), fields.stream() .map(f -> f.nameCamelFirstLower()+"List.size()") @@ -158,13 +161,11 @@ private static String generateTestData(String modelClassName, Field field, boole Field.FieldType convertedFieldType = getOptionalConvertedFieldType(field); return """ addNull(%s)""" - .formatted(getOptionsForFieldType(convertedFieldType, convertedFieldType.javaType)) - .indent(DEFAULT_INDENT * 2); + .formatted(getOptionsForFieldType(convertedFieldType, convertedFieldType.javaType)); } else if (repeated) { final String optionsList = generateTestData(modelClassName, field, field.optionalValueType(), false); return """ - generateListArguments(%s)""".formatted(optionsList) - .indent(DEFAULT_INDENT * 2); + generateListArguments(%s)""".formatted(optionsList); } else if(field instanceof final OneOfField oneOf) { final List options = new ArrayList<>(); for (var subField: oneOf.fields()) { @@ -180,13 +181,13 @@ private static String generateTestData(String modelClassName, Field field, boole } else { listStr = getOptionsForFieldType(subField.type(), ((SingleField) subField).javaFieldTypeForTest()); } - options.add(listStr + """ - .stream() + options.add(listStr + ("\n.stream()\n"+ + """ .map(value -> new OneOf<>(%sOneOfType.%s, value)) .toList()""".formatted( modelClassName + "." + field.nameCamelFirstUpper(), enumValueName - ).indent(DEFAULT_INDENT * 2) + )).indent(DEFAULT_INDENT ) ); } } else { @@ -196,8 +197,8 @@ private static String generateTestData(String modelClassName, Field field, boole } return """ Stream.of( - List.of(new OneOf<>(%sOneOfType.UNSET, null)), - %s + List.of(new OneOf<>(%sOneOfType.UNSET, null)), + %s ).flatMap(List::stream).toList()""".formatted( modelClassName+"."+field.nameCamelFirstUpper(), String.join(",\n", options).indent(DEFAULT_INDENT) @@ -252,114 +253,103 @@ private static String generateTestMethod(final String modelClassName, final Stri @ParameterizedTest @MethodSource("createModelTestArguments") public void test$modelClassNameAgainstProtoC(final NoToStringWrapper<$modelClassName> modelObjWrapper) throws Exception { - final $modelClassName modelObj = modelObjWrapper.getValue(); - // get reusable thread buffers - final var dataBuffer = getThreadLocalDataBuffer(); - final var dataBuffer2 = getThreadLocalDataBuffer2(); - final var byteBuffer = getThreadLocalByteBuffer(); - final var charBuffer = getThreadLocalCharBuffer(); - final var charBuffer2 = getThreadLocalCharBuffer2(); - - // model to bytes with PBJ - $modelClassName.PROTOBUF.write(modelObj, dataBuffer); - // clamp limit to bytes written - dataBuffer.limit(dataBuffer.position()); - - // copy bytes to ByteBuffer - dataBuffer.resetPosition(); - final int protoBufByteCount = (int)dataBuffer.remaining(); - dataBuffer.readBytes(byteBuffer); - byteBuffer.flip(); - - // read proto bytes with ProtoC to make sure it is readable and no parse exceptions are thrown - final $protocModelClass protoCModelObj = $protocModelClass.parseFrom(byteBuffer); - - // read proto bytes with PBJ parser - dataBuffer.resetPosition(); - final $modelClassName modelObj2 = $modelClassName.PROTOBUF.parse(dataBuffer); - - // check the read back object is equal to written original one - //assertEquals(modelObj.toString(), modelObj2.toString()); - assertEquals(modelObj, modelObj2); - - // model to bytes with ProtoC writer - byteBuffer.clear(); - final CodedOutputStream codedOutput = CodedOutputStream.newInstance(byteBuffer); - protoCModelObj.writeTo(codedOutput); - codedOutput.flush(); - byteBuffer.flip(); - // copy to a data buffer - dataBuffer2.writeBytes(byteBuffer); - dataBuffer2.flip(); - - // compare written bytes - assertEquals(dataBuffer, dataBuffer2); + final $modelClassName modelObj = modelObjWrapper.getValue(); + // get reusable thread buffers + final var dataBuffer = getThreadLocalDataBuffer(); + final var dataBuffer2 = getThreadLocalDataBuffer2(); + final var byteBuffer = getThreadLocalByteBuffer(); + final var charBuffer = getThreadLocalCharBuffer(); + final var charBuffer2 = getThreadLocalCharBuffer2(); + + // model to bytes with PBJ + $modelClassName.PROTOBUF.write(modelObj, dataBuffer); + // clamp limit to bytes written + dataBuffer.limit(dataBuffer.position()); + + // copy bytes to ByteBuffer + dataBuffer.resetPosition(); + final int protoBufByteCount = (int)dataBuffer.remaining(); + dataBuffer.readBytes(byteBuffer); + byteBuffer.flip(); + + // read proto bytes with ProtoC to make sure it is readable and no parse exceptions are thrown + final $protocModelClass protoCModelObj = $protocModelClass.parseFrom(byteBuffer); + + // read proto bytes with PBJ parser + dataBuffer.resetPosition(); + final $modelClassName modelObj2 = $modelClassName.PROTOBUF.parse(dataBuffer); + + // check the read back object is equal to written original one + //assertEquals(modelObj.toString(), modelObj2.toString()); + assertEquals(modelObj, modelObj2); + + // model to bytes with ProtoC writer + byteBuffer.clear(); + final CodedOutputStream codedOutput = CodedOutputStream.newInstance(byteBuffer); + protoCModelObj.writeTo(codedOutput); + codedOutput.flush(); + byteBuffer.flip(); + // copy to a data buffer + dataBuffer2.writeBytes(byteBuffer); + dataBuffer2.flip(); + + // compare written bytes + assertEquals(dataBuffer, dataBuffer2); - // parse those bytes again with PBJ - dataBuffer2.resetPosition(); - final $modelClassName modelObj3 = $modelClassName.PROTOBUF.parse(dataBuffer2); - assertEquals(modelObj, modelObj3); + // parse those bytes again with PBJ + dataBuffer2.resetPosition(); + final $modelClassName modelObj3 = $modelClassName.PROTOBUF.parse(dataBuffer2); + assertEquals(modelObj, modelObj3); - // check measure methods - dataBuffer2.resetPosition(); - assertEquals(protoBufByteCount, $modelClassName.PROTOBUF.measure(dataBuffer2)); - assertEquals(protoBufByteCount, $modelClassName.PROTOBUF.measureRecord(modelObj)); - - // check fast equals - dataBuffer2.resetPosition(); - assertTrue($modelClassName.PROTOBUF.fastEquals(modelObj, dataBuffer2)); + // check measure methods + dataBuffer2.resetPosition(); + assertEquals(protoBufByteCount, $modelClassName.PROTOBUF.measure(dataBuffer2)); + assertEquals(protoBufByteCount, $modelClassName.PROTOBUF.measureRecord(modelObj)); + + // check fast equals + dataBuffer2.resetPosition(); + assertTrue($modelClassName.PROTOBUF.fastEquals(modelObj, dataBuffer2)); - // Test toBytes() - Bytes bytes = $modelClassName.PROTOBUF.toBytes(modelObj); - final var dataBuffer3 = getThreadLocalDataBuffer(); - bytes.toReadableSequentialData().readBytes(dataBuffer3); - byte[] readBytes = new byte[(int)dataBuffer3.length()]; - dataBuffer3.getBytes(0, readBytes); - assertArrayEquals(bytes.toByteArray(), readBytes); + // Test toBytes() + Bytes bytes = $modelClassName.PROTOBUF.toBytes(modelObj); + final var dataBuffer3 = getThreadLocalDataBuffer(); + bytes.toReadableSequentialData().readBytes(dataBuffer3); + byte[] readBytes = new byte[(int)dataBuffer3.length()]; + dataBuffer3.getBytes(0, readBytes); + assertArrayEquals(bytes.toByteArray(), readBytes); - // Test JSON Writing - final CharBufferToWritableSequentialData charBufferToWritableSequentialData = new CharBufferToWritableSequentialData(charBuffer); - $modelClassName.JSON.write(modelObj,charBufferToWritableSequentialData); - charBuffer.flip(); - JsonFormat.printer().appendTo(protoCModelObj, charBuffer2); - charBuffer2.flip(); - assertEquals(charBuffer2, charBuffer); - - // Test JSON Reading - final $modelClassName jsonReadPbj = $modelClassName.JSON.parse(JsonTools.parseJson(charBuffer), false); - assertEquals(modelObj, jsonReadPbj); + // Test JSON Writing + final CharBufferToWritableSequentialData charBufferToWritableSequentialData = new CharBufferToWritableSequentialData(charBuffer); + $modelClassName.JSON.write(modelObj,charBufferToWritableSequentialData); + charBuffer.flip(); + JsonFormat.printer().appendTo(protoCModelObj, charBuffer2); + charBuffer2.flip(); + assertEquals(charBuffer2, charBuffer); + + // Test JSON Reading + final $modelClassName jsonReadPbj = $modelClassName.JSON.parse(JsonTools.parseJson(charBuffer), false); + assertEquals(modelObj, jsonReadPbj); + } + + @SuppressWarnings("EqualsWithItself") + @Test + public void testTimestampTestEqualsAndHashCode() throws Exception { + if (ARGUMENTS.size() >= 3) { + final var item1 = ARGUMENTS.get(0); + final var item2 = ARGUMENTS.get(1); + final var item3 = ARGUMENTS.get(2); + assertEquals(item1, item1); + assertEquals(item2, item2); + assertEquals(item3, item3); + assertNotEquals(item1, item2); + assertNotEquals(item2, item3); + final var item1HashCode = item1.hashCode(); + final var item2HashCode = item2.hashCode(); + final var item3HashCode = item3.hashCode(); + assertNotEquals(item1HashCode, item2HashCode); + assertNotEquals(item2HashCode, item3HashCode); + } } - // Very slow for now. Too much garbage generated to enable in general case. -// // Test hashCode and equals() -// Stream> objects = createModelTestArguments(); -// Object[] objArray = objects.toArray(); -// for (int i = 0; i < objArray.length; i++) { -// for (int j = i; j < objArray.length; j++) { -// if (objArray[i].hashCode() != objArray[i].hashCode()) { -// fail("Same object, different hash."); -// } -// if (objArray[j].hashCode() != objArray[j].hashCode()) { -// fail("Same object, different hash 1."); -// } -// if (objArray[i].hashCode() == objArray[j].hashCode()) { -// if (!objArray[i].equals(objArray[j])) { -// fail("equalsHash, different objects."); -// } -// } -// } -// } -// -// Map map = new HashMap<>(); -// map.put(objArray[0], objArray[0]); -// for (int i = 1; i < objArray.length; i++) { -// Object o = map.put(objArray[i], objArray[i]); -// if (o != null) { -// Object existing = map.get(objArray[i]); -// assertEquals(existing.hashCode(), objArray[i].hashCode()); -// assertEquals(existing, objArray[i]); -// } -// } - """ .replace("$modelClassName",modelClassName) .replace("$protocModelClass",protoCJavaFullQualifiedClass) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecGenerator.java index 011baef7..7fee0a23 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/json/JsonCodecGenerator.java @@ -76,9 +76,9 @@ public void generate(Protobuf3Parser.MessageDefContext msgDef, final File destin * JSON Codec for $modelClass model object. Generated based on protobuf schema. */ public final class $codecClass implements JsonCodec<$modelClass> { - $unsetOneOfConstants - $parseObject - $writeMethod + $unsetOneOfConstants + $parseObject + $writeMethod } """ .replace("$package", codecPackage) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecGenerator.java index 6a738329..ecdaf6fa 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecGenerator.java @@ -70,22 +70,22 @@ public void generate(Protobuf3Parser.MessageDefContext msgDef, final File destin import static $schemaClass.*; import static com.hedera.pbj.runtime.ProtoWriterTools.*; import static com.hedera.pbj.runtime.ProtoParserTools.*; - + /** * Protobuf Codec for $modelClass model object. Generated based on protobuf schema. */ public final class $codecClass implements Codec<$modelClass> { - $unsetOneOfConstants - $parseMethod - $parseStrictMethod - $writeMethod - $measureDataMethod - $measureRecordMethod - $fastEqualsMethod - - // ------ Private Implementation - - $parseInternal + $unsetOneOfConstants + $parseMethod + $parseStrictMethod + $writeMethod + $measureDataMethod + $measureRecordMethod + $fastEqualsMethod + + // ------ Private Implementation + + $parseInternal } """ .replace("$package", codecPackage) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java index f9fbc473..a8b53a17 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/protobuf/CodecParseMethodGenerator.java @@ -50,8 +50,7 @@ static String generateParseMethod(final String modelClassName, final List * method. If null, the method returns immediately. If there are no bytes remaining in the data input, * then the method also returns immediately. * @return Parsed $modelClassName model object or null if data input was null or empty - * @throws IOException If the protobuf stream is not empty and has malformed - * protobuf bytes (i.e. isn't valid protobuf). + * @throws IOException If the protobuf stream is not empty and has malformed protobuf bytes (i.e. isn't valid protobuf). */ public @NonNull $modelClassName parse(@NonNull final ReadableSequentialData input) throws IOException { return parseInternal(input, false); @@ -220,7 +219,7 @@ private static void generateFieldCaseStatementPacked(final StringBuilder sb, fin final var beforeLimit = input.limit(); input.limit(input.position() + length); while (input.hasRemaining()) { - $tempFieldName = addToList($tempFieldName,$readMethod); + $tempFieldName = addToList($tempFieldName,$readMethod); } input.limit(beforeLimit);""" .replace("$tempFieldName", "temp_" + field.name()) @@ -248,19 +247,19 @@ private static void generateFieldCaseStatement(final StringBuilder sb, final Fie final var valueTypeMessageSize = input.readVarInt(false); final $fieldType value; if (valueTypeMessageSize > 0) { - final var beforeLimit = input.limit(); - input.limit(input.position() + valueTypeMessageSize); - // read inner tag - final int valueFieldTag = input.readVarInt(false); - // assert tag is as expected - assert (valueFieldTag >>> TAG_FIELD_OFFSET) == 1; - assert (valueFieldTag & TAG_WRITE_TYPE_MASK) == $valueTypeWireType; - // read value - value = $readMethod; - input.limit(beforeLimit); + final var beforeLimit = input.limit(); + input.limit(input.position() + valueTypeMessageSize); + // read inner tag + final int valueFieldTag = input.readVarInt(false); + // assert tag is as expected + assert (valueFieldTag >>> TAG_FIELD_OFFSET) == 1; + assert (valueFieldTag & TAG_WRITE_TYPE_MASK) == $valueTypeWireType; + // read value + value = $readMethod; + input.limit(beforeLimit); } else { - // means optional is default value - value = $defaultValue; + // means optional is default value + value = $defaultValue; }""" .replace("$fieldType", field.javaFieldType()) .replace("$readMethod", readMethod(field)) diff --git a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/ComplexEqualsHashCodeBench.java b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/ComplexEqualsHashCodeBench.java new file mode 100644 index 00000000..a2041d01 --- /dev/null +++ b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/ComplexEqualsHashCodeBench.java @@ -0,0 +1,155 @@ +package com.hedera.pbj.intergration.jmh; + +import com.hedera.pbj.runtime.io.buffer.Bytes; +import com.hedera.pbj.test.proto.pbj.Hasheval; +import com.hedera.pbj.test.proto.pbj.Suit; +import com.hedera.pbj.test.proto.pbj.TimestampTest; +import edu.umd.cs.findbugs.annotations.Nullable; +import java.util.concurrent.TimeUnit; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OperationsPerInvocation; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +/* +Mac Results + +Benchmark Mode Cnt Score Error Units +ComplexEqualsHashCodeBench.benchJavaRecordHashCode avgt 5 16.304 ± 0.155 ns/op +ComplexEqualsHashCodeBench.benchHashCode avgt 5 20.601 ± 0.136 ns/op --- 1.26x slower + +ComplexEqualsHashCodeBench.benchJavaRecordEquals avgt 5 13.153 ± 0.043 ns/op +ComplexEqualsHashCodeBench.benchEquals avgt 5 13.141 ± 0.055 ns/op --- 1.0x slower + +ComplexEqualsHashCodeBench.benchJavaRecordNotEquals avgt 5 8.940 ± 0.052 ns/op +ComplexEqualsHashCodeBench.benchNotEquals avgt 5 7.497 ± 0.057 ns/op --- 1.19x faster + */ + +@SuppressWarnings("unused") +@State(Scope.Benchmark) +@Fork(1) +@Warmup(iterations = 4, time = 2) +@Measurement(iterations = 5, time = 2) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@BenchmarkMode(Mode.AverageTime) +public class ComplexEqualsHashCodeBench { + public record HashevalJavaRecord( + int int32Number, + int sint32Number, + int uint32Number, + int fixed32Number, + int sfixed32Number, + float floatNumber, + long int64Number, + long sint64Number, + long uint64Number, + long fixed64Number, + long sfixed64Number, + double doubleNumber, + boolean booleanField, + Suit enumSuit, + @Nullable TimestampTest subObject, + String text, + Bytes bytesField + ){} + + private final Hasheval hasheval; + private final Hasheval hasheval1; + private final Hasheval hashevalDifferent; + private final HashevalJavaRecord hashevalJavaRecord; + private final HashevalJavaRecord hashevalJavaRecord1; + private final HashevalJavaRecord hashevalJavaRecordDifferent; + + public ComplexEqualsHashCodeBench() { + hasheval = new Hasheval(123, 123, 123, + 123, 123, 1.23f, 123L, 123L, + 123L, 123L, 123L, 1.23D, true, + Suit.ACES, new TimestampTest(987L, 123), + "FooBarKKKKHHHHOIOIOI", + Bytes.wrap(new byte[]{1, 2, 3, 4, 5, 6, 7, (byte)255})); + hasheval1 = new Hasheval(123, 123, 123, + 123, 123, 1.23f, 123L, 123L, + 123L, 123L, 123L, 1.23D, true, + Suit.ACES, new TimestampTest(987L, 123), + "FooBarKKKKHHHHOIOIOI", + Bytes.wrap(new byte[]{1, 2, 3, 4, 5, 6, 7, (byte)255})); + hashevalDifferent = new Hasheval(123, 123, 123, + 123, 123, 1.23f, 123L, 123L, + 123L, 123L, 123L, 1.23D, true, + Suit.ACES, new TimestampTest(987L, 123), + "Different", + Bytes.wrap(new byte[]{1, 2, 3, 4, 5, 6, 7, (byte)255})); + hashevalJavaRecord = new HashevalJavaRecord(123, 123, 123, + 123, 123, 1.23f, 123L, 123L, + 123L, 123L, 123L, 1.23D, true, + Suit.ACES, new TimestampTest(987L, 123), + "FooBarKKKKHHHHOIOIOI", + Bytes.wrap(new byte[]{1, 2, 3, 4, 5, 6, 7, (byte)255})); + hashevalJavaRecord1 = new HashevalJavaRecord(123, 123, 123, + 123, 123, 1.23f, 123L, 123L, + 123L, 123L, 123L, 1.23D, true, + Suit.ACES, new TimestampTest(987L, 123), + "FooBarKKKKHHHHOIOIOI", + Bytes.wrap(new byte[]{1, 2, 3, 4, 5, 6, 7, (byte)255})); + hashevalJavaRecordDifferent = new HashevalJavaRecord(123, 123, 123, + 123, 123, 1.23f, 123L, 123L, + 123L, 123L, 123L, 1.23D, true, + Suit.ACES, new TimestampTest(987L, 123), + "Different", + Bytes.wrap(new byte[]{1, 2, 3, 4, 5, 6, 7, (byte)255})); + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchHashCode(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(hasheval.hashCode()); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchJavaRecordHashCode(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(hashevalJavaRecord.hashCode()); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchEquals(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(hasheval.equals(hasheval1)); + } + } + @Benchmark + @OperationsPerInvocation(1050) + public void benchJavaRecordEquals(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(hashevalJavaRecord.equals(hashevalJavaRecord1)); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchNotEquals(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(hasheval.equals(hashevalDifferent)); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchJavaRecordNotEquals(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(hashevalJavaRecord.equals(hashevalJavaRecordDifferent)); + } + } +} \ No newline at end of file diff --git a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java index 7ec979b8..d5b42b3e 100644 --- a/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java +++ b/pbj-integration-tests/src/jmh/java/com/hedera/pbj/intergration/jmh/EqualsHashCodeBench.java @@ -1,6 +1,5 @@ package com.hedera.pbj.intergration.jmh; -import com.hedera.pbj.test.proto.pbj.Suit; import com.hedera.pbj.test.proto.pbj.TimestampTest; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; @@ -15,9 +14,22 @@ import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.infra.Blackhole; -import java.io.IOException; import java.util.concurrent.TimeUnit; +/* +Mac Results + +Benchmark Mode Cnt Score Error Units +EqualsHashCodeBench.benchJavaRecordHashCode avgt 5 0.461 ± 0.007 ns/op +EqualsHashCodeBench.benchHashCode avgt 5 1.936 ± 0.019 ns/op --- 4.2x slower + +EqualsHashCodeBench.benchJavaRecordEquals avgt 5 0.664 ± 0.006 ns/op +EqualsHashCodeBench.benchEquals avgt 5 0.585 ± 0.009 ns/op --- 1.1x slower + +EqualsHashCodeBench.benchJavaRecordNotEquals avgt 5 0.508 ± 0.004 ns/op +EqualsHashCodeBench.benchNotEquals avgt 5 0.655 ± 0.004 ns/op --- 1.3x slower + */ + @SuppressWarnings("unused") @State(Scope.Benchmark) @Fork(1) @@ -26,37 +38,68 @@ @OutputTimeUnit(TimeUnit.NANOSECONDS) @BenchmarkMode(Mode.AverageTime) public class EqualsHashCodeBench { - private TimestampTest testStamp; - private TimestampTest testStamp; - - private TimestampTest testStamp1; + public record TimestampStandardRecord(long seconds, int nanos){} + + private final TimestampTest testStamp; + private final TimestampTest testStamp1; + private final TimestampTest testStampDifferent; + private final TimestampStandardRecord timestampStandardRecord; + private final TimestampStandardRecord timestampStandardRecord1; + private final TimestampStandardRecord timestampStandardRecordDifferent; public EqualsHashCodeBench() { testStamp = new TimestampTest(987L, 123); - testStamp1 = new TimestampTest(987L, 122); + testStamp1 = new TimestampTest(987L, 123); + testStampDifferent = new TimestampTest(987L, 122); + timestampStandardRecord = new TimestampStandardRecord(987L, 123); + timestampStandardRecord1 = new TimestampStandardRecord(987L, 123); + timestampStandardRecordDifferent = new TimestampStandardRecord(987L, 122); + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchHashCode(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(testStamp.hashCode()); + } } @Benchmark @OperationsPerInvocation(1050) - public void benchHashCode(Blackhole blackhole) throws IOException { + public void benchJavaRecordHashCode(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(timestampStandardRecord.hashCode()); + } + } + + @Benchmark + @OperationsPerInvocation(1050) + public void benchEquals(Blackhole blackhole) { + for (int i = 0; i < 1050; i++) { + blackhole.consume(testStamp.equals(testStamp1)); + } + } + @Benchmark + @OperationsPerInvocation(1050) + public void benchJavaRecordEquals(Blackhole blackhole) { for (int i = 0; i < 1050; i++) { - testStamp.hashCode(); + blackhole.consume(timestampStandardRecord.equals(timestampStandardRecord1)); } } @Benchmark @OperationsPerInvocation(1050) - public void benchEquals(Blackhole blackhole) throws IOException { + public void benchNotEquals(Blackhole blackhole) { for (int i = 0; i < 1050; i++) { - testStamp.equals(testStamp); + blackhole.consume(testStamp.equals(testStampDifferent)); } } @Benchmark @OperationsPerInvocation(1050) - public void benchNotEquals(Blackhole blackhole) throws IOException { + public void benchJavaRecordNotEquals(Blackhole blackhole) { for (int i = 0; i < 1050; i++) { - testStamp.equals(testStamp1); + blackhole.consume(timestampStandardRecord.equals(timestampStandardRecordDifferent)); } } } \ No newline at end of file From cfb554de57e07894c8f17687f8937877968cf6e6 Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Tue, 21 Nov 2023 15:55:24 -0500 Subject: [PATCH 08/10] Addressed review comments. Signed-off-by: Ivan Malygin --- .../com/hedera/pbj/compiler/impl/Common.java | 165 +++++++++--------- .../com/hedera/pbj/compiler/impl/Field.java | 2 + .../impl/generators/TestGenerator.java | 2 +- 3 files changed, 83 insertions(+), 86 deletions(-) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index e2d6d415..8021efa8 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -67,7 +67,8 @@ public static String capitalizeFirstLetter(String name) { * @param firstUpper if true then first char is upper case otherwise it is lower * @return out name in camel case */ - public static String snakeToCamel(String name, boolean firstUpper) { + @NonNull + public static String snakeToCamel(@NonNull String name, boolean firstUpper) { final String out = Arrays.stream(name.split("_")).map(Common::capitalizeFirstLetter).collect( Collectors.joining("")); return (firstUpper ? Character.toUpperCase(out.charAt(0)) : Character.toLowerCase(out.charAt(0)) ) @@ -186,92 +187,86 @@ public static String getFieldsHashCode(final List fields, String generate if (f.parent() != null) { final OneOfField oneOfField = f.parent(); generatedCodeSoFar += getFieldsHashCode(oneOfField.fields(), generatedCodeSoFar); - } - - if (f.optionalValueType()) { + } else if (f.optionalValueType()) { generatedCodeSoFar = getOptionalHashCodeGeneration(generatedCodeSoFar, f); - } - else if (f.repeated()) { + } else if (f.repeated()) { generatedCodeSoFar = getRepeatedHashCodeGeneration(generatedCodeSoFar, f); - } - else if (f.nameCamelFirstLower() != null) { - if (f.type() == Field.FieldType.FIXED32 || - f.type() == Field.FieldType.INT32 || - f.type() == Field.FieldType.SFIXED32 || - f.type() == Field.FieldType.SINT32 || - f.type() == Field.FieldType.UINT32) { - generatedCodeSoFar += ( - """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Integer.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.FIXED64 || - f.type() == Field.FieldType.INT64 || - f.type() == Field.FieldType.SFIXED64 || - f.type() == Field.FieldType.SINT64 || - f.type() == Field.FieldType.UINT64) { - generatedCodeSoFar += ( - """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Long.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.ENUM) { - generatedCodeSoFar += ( - """ - if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.BOOL) { - generatedCodeSoFar += ( - """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Boolean.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.FLOAT) { - generatedCodeSoFar += ( - """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Float.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.DOUBLE) { - generatedCodeSoFar += ( - """ - if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Double.hashCode($fieldName); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.STRING) { - generatedCodeSoFar += ( - """ - if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.BYTES) { - generatedCodeSoFar += ( - """ - if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } - else if (f.parent() == null) { // process sub message - generatedCodeSoFar += ( - """ - if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } - else { - throw new RuntimeException("Unexpected field type for getting HashCode - " + f.type().toString()); - } - } + } else { + if (f.type() == Field.FieldType.FIXED32 || + f.type() == Field.FieldType.INT32 || + f.type() == Field.FieldType.SFIXED32 || + f.type() == Field.FieldType.SINT32 || + f.type() == Field.FieldType.UINT32) { + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Integer.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.FIXED64 || + f.type() == Field.FieldType.INT64 || + f.type() == Field.FieldType.SFIXED64 || + f.type() == Field.FieldType.SINT64 || + f.type() == Field.FieldType.UINT64) { + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Long.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.ENUM) { + generatedCodeSoFar += ( + """ + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.BOOL) { + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Boolean.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.FLOAT) { + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Float.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.DOUBLE) { + generatedCodeSoFar += ( + """ + if ($fieldName != DEFAULT.$fieldName) { + result = 31 * result + Double.hashCode($fieldName); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.STRING) { + generatedCodeSoFar += ( + """ + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.BYTES) { + generatedCodeSoFar += ( + """ + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.parent() == null) { // process sub message + generatedCodeSoFar += ( + """ + if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { + result = 31 * result + $fieldName.hashCode(); + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else { + throw new RuntimeException("Unexpected field type for getting HashCode - " + f.type().toString()); + } + } } return generatedCodeSoFar.indent(DEFAULT_INDENT * 2); } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Field.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Field.java index 550c2131..4cea8a36 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Field.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Field.java @@ -1,6 +1,7 @@ package com.hedera.pbj.compiler.impl; import com.hedera.pbj.compiler.impl.grammar.Protobuf3Parser; +import edu.umd.cs.findbugs.annotations.NonNull; import java.util.Set; @@ -47,6 +48,7 @@ default String nameCamelFirstUpper() { * * @return this fields name converted */ + @NonNull default String nameCamelFirstLower() { return snakeToCamel(name(),false); } diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java index b9884069..d5584dd8 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/generators/TestGenerator.java @@ -333,7 +333,7 @@ private static String generateTestMethod(final String modelClassName, final Stri @SuppressWarnings("EqualsWithItself") @Test - public void testTimestampTestEqualsAndHashCode() throws Exception { + public void testTestEqualsAndHashCode() throws Exception { if (ARGUMENTS.size() >= 3) { final var item1 = ARGUMENTS.get(0); final var item2 = ARGUMENTS.get(1); From 52376d0704f790481c1b6e0668ac2b98aedd48fa Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Tue, 21 Nov 2023 18:44:24 -0500 Subject: [PATCH 09/10] Addressed review comments. Signed-off-by: Ivan Malygin --- .../com/hedera/pbj/compiler/impl/Common.java | 195 ++++++++---------- 1 file changed, 90 insertions(+), 105 deletions(-) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index 8021efa8..48738ad6 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -188,7 +188,7 @@ public static String getFieldsHashCode(final List fields, String generate final OneOfField oneOfField = f.parent(); generatedCodeSoFar += getFieldsHashCode(oneOfField.fields(), generatedCodeSoFar); } else if (f.optionalValueType()) { - generatedCodeSoFar = getOptionalHashCodeGeneration(generatedCodeSoFar, f); + generatedCodeSoFar = getPrimitiveWrapperHashCodeGeneration(generatedCodeSoFar, f); } else if (f.repeated()) { generatedCodeSoFar = getRepeatedHashCodeGeneration(generatedCodeSoFar, f); } else { @@ -200,7 +200,7 @@ public static String getFieldsHashCode(final List fields, String generate generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Integer.hashCode($fieldName); + result = 31 * result + Integer.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FIXED64 || @@ -211,56 +211,44 @@ public static String getFieldsHashCode(final List fields, String generate generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Long.hashCode($fieldName); + result = 31 * result + Long.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.ENUM) { - generatedCodeSoFar += ( - """ - if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BOOL) { generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Boolean.hashCode($fieldName); + result = 31 * result + Boolean.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.FLOAT) { generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Float.hashCode($fieldName); + result = 31 * result + Float.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.DOUBLE) { generatedCodeSoFar += ( """ if ($fieldName != DEFAULT.$fieldName) { - result = 31 * result + Double.hashCode($fieldName); + result = 31 * result + Double.hashCode($fieldName); } """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.STRING) { - generatedCodeSoFar += ( - """ - if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + $fieldName.hashCode(); - } - """).replace("$fieldName", f.nameCamelFirstLower()); } else if (f.type() == Field.FieldType.BYTES) { generatedCodeSoFar += ( """ if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + ($fieldName == null ? 0 : $fieldName.hashCode()); + result = 31 * result + $fieldName.hashCode(); } """).replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.parent() == null) { // process sub message + } else if (f.type() == Field.FieldType.ENUM || + f.type() == Field.FieldType.STRING || + f.parent() == null) { // process sub message generatedCodeSoFar += ( """ if ($fieldName != null && !$fieldName.equals(DEFAULT.$fieldName)) { - result = 31 * result + $fieldName.hashCode(); + result = 31 * result + $fieldName.hashCode(); } """).replace("$fieldName", f.nameCamelFirstLower()); } else { @@ -278,7 +266,7 @@ public static String getFieldsHashCode(final List fields, String generate * @return Updated codegen string. */ @NonNull - private static String getOptionalHashCodeGeneration(String generatedCodeSoFar, Field f) { + private static String getPrimitiveWrapperHashCodeGeneration(String generatedCodeSoFar, Field f) { switch (f.messageType()) { case "StringValue" -> generatedCodeSoFar += ( """ @@ -338,12 +326,14 @@ private static String getRepeatedHashCodeGeneration(String generatedCodeSoFar, F generatedCodeSoFar += ( """ java.util.List list$$fieldName = $fieldName; - for (Object o : list$$fieldName) { - if (o != null) { - result = 31 * result + o.hashCode(); - } else { - result = 31 * result; - } + if(list$$fieldName != null) { + for (Object o : list$$fieldName) { + if (o != null) { + result = 31 * result + o.hashCode(); + } else { + result = 31 * result; + } + } } """).replace("$fieldName", f.nameCamelFirstLower()); return generatedCodeSoFar; @@ -364,74 +354,72 @@ public static String getFieldsEqualsStatements(final List fields, String } if (f.optionalValueType()) { - generatedCodeSoFar = getOptionalEqualsGeneration(generatedCodeSoFar, f); + generatedCodeSoFar = getPrimitiveWrapperEqualsGeneration(generatedCodeSoFar, f); } else if (f.repeated()) { generatedCodeSoFar = getRepeatedEqualsGeneration(generatedCodeSoFar, f); - } - else if (f.nameCamelFirstLower() != null) { - if (f.type() == Field.FieldType.FIXED32 || - f.type() == Field.FieldType.INT32 || - f.type() == Field.FieldType.SFIXED32 || - f.type() == Field.FieldType.SINT32 || - f.type() == Field.FieldType.UINT32) { - generatedCodeSoFar += - """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.FIXED64 || - f.type() == Field.FieldType.INT64 || - f.type() == Field.FieldType.SFIXED64 || - f.type() == Field.FieldType.SINT64 || - f.type() == Field.FieldType.UINT64) { - generatedCodeSoFar += - """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.BOOL) { - generatedCodeSoFar += - """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.DOUBLE) { - } else if (f.type() == Field.FieldType.FLOAT) { - generatedCodeSoFar += - """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.DOUBLE) { - generatedCodeSoFar += - """ - if ($fieldName != thatObj.$fieldName) { - return false; - } - """.replace("$fieldName", f.nameCamelFirstLower()); - } else if (f.type() == Field.FieldType.STRING || - f.type() == Field.FieldType.BYTES || - f.type() == Field.FieldType.ENUM || - f.parent() == null /* Process a sub-message */) { - generatedCodeSoFar += ( - """ - if ($fieldName == null && thatObj.$fieldName != null) { - return false; - } - if ($fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } - """).replace("$fieldName", f.nameCamelFirstLower()); - } - else { - throw new IllegalArgumentException("Unexpected field type for getting HashCode - " + f.type().toString()); - } - } + } else { + f.nameCamelFirstLower(); + if (f.type() == Field.FieldType.FIXED32 || + f.type() == Field.FieldType.INT32 || + f.type() == Field.FieldType.SFIXED32 || + f.type() == Field.FieldType.SINT32 || + f.type() == Field.FieldType.UINT32) { + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.FIXED64 || + f.type() == Field.FieldType.INT64 || + f.type() == Field.FieldType.SFIXED64 || + f.type() == Field.FieldType.SINT64 || + f.type() == Field.FieldType.UINT64) { + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.BOOL) { + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.FLOAT) { + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.DOUBLE) { + generatedCodeSoFar += + """ + if ($fieldName != thatObj.$fieldName) { + return false; + } + """.replace("$fieldName", f.nameCamelFirstLower()); + } else if (f.type() == Field.FieldType.STRING || + f.type() == Field.FieldType.BYTES || + f.type() == Field.FieldType.ENUM || + f.parent() == null /* Process a sub-message */) { + generatedCodeSoFar += ( + """ + if ($fieldName == null && thatObj.$fieldName != null) { + return false; + } + if ($fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { + return false; + } + """).replace("$fieldName", f.nameCamelFirstLower()); + } else { + throw new IllegalArgumentException("Unexpected field type for getting HashCode - " + f.type().toString()); + } + } } return generatedCodeSoFar.indent(DEFAULT_INDENT); } @@ -443,7 +431,7 @@ else if (f.nameCamelFirstLower() != null) { * @return Updated codegen string. */ @NonNull - private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Field f) { + private static String getPrimitiveWrapperEqualsGeneration(String generatedCodeSoFar, Field f) { switch (f.messageType()) { case "StringValue" -> generatedCodeSoFar += ( @@ -456,6 +444,7 @@ private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Fie } """).replace("$fieldName", f.nameCamelFirstLower()); case "BoolValue" -> + generatedCodeSoFar += ( """ if ($fieldName instanceof Object) { @@ -472,14 +461,10 @@ private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Fie case "Int32Value", "UInt32Value", "Int64Value", "UInt64Value", "FloatValue", "DoubleValue" -> generatedCodeSoFar += ( """ - if ($fieldName instanceof Object) { - if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; - } - if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { - return false; - } - } else if ($fieldName != thatObj.$fieldName) { + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { return false; } """).replace("$fieldName", f.nameCamelFirstLower()); @@ -509,7 +494,7 @@ private static String getOptionalEqualsGeneration(String generatedCodeSoFar, Fie private static String getRepeatedEqualsGeneration(String generatedCodeSoFar, Field f) { generatedCodeSoFar += ( """ - if (this.$fieldName == null && this.$fieldName != null) { + if (this.$fieldName == null && thatObj.$fieldName != null) { return false; } From df696c86be6d202bcabf35ce2d4ac9b269f0d958 Mon Sep 17 00:00:00 2001 From: Ivan Malygin Date: Tue, 21 Nov 2023 19:26:28 -0500 Subject: [PATCH 10/10] Addressed review comments. Signed-off-by: Ivan Malygin --- .../java/com/hedera/pbj/compiler/impl/Common.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java index 48738ad6..06eb44ed 100644 --- a/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java +++ b/pbj-core/pbj-compiler/src/main/java/com/hedera/pbj/compiler/impl/Common.java @@ -447,14 +447,10 @@ private static String getPrimitiveWrapperEqualsGeneration(String generatedCodeSo generatedCodeSoFar += ( """ - if ($fieldName instanceof Object) { - if (this.$fieldName == null && thatObj.$fieldName != null) { - return false; - } - if (!$fieldName.equals(thatObj.$fieldName)) { - return false; - } - } else if ($fieldName != thatObj.$fieldName) { + if (this.$fieldName == null && thatObj.$fieldName != null) { + return false; + } + if (this.$fieldName != null && !$fieldName.equals(thatObj.$fieldName)) { return false; } """).replace("$fieldName", f.nameCamelFirstLower());