Codec Interface to Abstract class, made all paths call parseImpl/writeImpl by ldintr · Pull Request #892 · hashgraph/pbj

ldintr · 2026-08-20T21:10:07Z

Description:

I didn't like how classes could have a 5 signature parse call a 3 signature overload ignoring some parameters (idr where I saw that, it was outside of obj). I would rather a consistent entry (which I'll use to call throwOnError in the future), and a consistent impl.

Here's a quick look. I may change (or rebase in the future) better javadoc.

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

Comments

I noticed I couldn't make this final because sometimes a class overloads it and other times there's no impl

    protected final int writeImpl(@NonNull T item, @NonNull byte[] output, final int startOffset) {
        final BufferedData bufferedData = BufferedData.wrap(output, startOffset, output.length - startOffset);
        try {
            write(item, bufferedData);
        } catch (IOException e) {
            throw new UncheckedIOException(e);
        }
        return (int) bufferedData.position();
    }

It seems like there's a chance they cane become inconsistent

    protected int writeImpl(@NonNull MessageInProtoPackage data, @NonNull byte[] output, final int startOffset) {
        int offset = startOffset;
        // [1] - bytesField
        offset += ProtoArrayWriterTools.writeBytes(output, offset, MessageInProtoPackageSchema.BYTES_FIELD,  data.bytesField(), true);

        // Write unknown fields if there are any
        for (final UnknownField uf : data.getUnknownFields()) {
            final int tag = (uf.field() << TAG_FIELD_OFFSET) | uf.wireType().ordinal();
            offset += ProtoArrayWriterTools.writeUnsignedVarInt(output, offset, tag);
            offset += uf.bytes().writeTo(output, offset);
        }
        return offset - startOffset;
    }

    protected void writeImpl(@NonNull MessageInProtoPackage data, @NonNull final WritableSequentialData out) throws IOException {
            // [1] - bytesField
        writeBytes(out, MessageInProtoPackageSchema.BYTES_FIELD, data.bytesField(), true);

        // Check if not-empty to avoid creating a lambda if there's nothing to write.
        if (!data.getUnknownFields().isEmpty()) {
            data.getUnknownFields().forEach(uf -> {
                final int tag = (uf.field() << TAG_FIELD_OFFSET) | uf.wireType().ordinal();
                out.writeVarInt(tag, false);
                uf.bytes().writeTo(out);
            });
        }
    }

github-actions · 2026-08-20T21:11:59Z

JUnit Test Report

523 files + 2 523 suites +2 31s ⏱️ +6s
1 629 tests +92 1 625 ✅ +92 4 💤 ±0 0 ❌ ±0
10 847 runs +92 10 819 ✅ +92 28 💤 ±0 0 ❌ ±0

Results for commit 6ff8dd8. ± Comparison against base commit 6762bb6.

This pull request removes 6 and adds 98 tests. Note that renamed tests count towards both.

com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] FLOAT, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49cd30@6df4d8f1, [0.1, 0.5, 100.0], 12, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49cf58@735b1ad2
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] STRING, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a91b8@230e163b, [string 1, testing here, testing there], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a93e0@7a41f4a7
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] BYTES, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a9608@7164e54, [010203, ff7f0f, 42da07370bff], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e4a9830@20040c6e
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] DOUBLE, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d180@88fc9cb, [0.1, 0.5, 100.0, 1.7653472635472653E240], 32, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d3a8@75693526
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [3] BOOL, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d5d0@603a34e5, [true, false, false, true, true, true], 6, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49d7f8@160d68b8
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [4] ENUM, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49da20@1d77d9c6, [0, 2, 1], 3, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000005e49dc48@2c5b4b91

com.hedera.pbj.runtime.CodecParseMethodTest ‑ fastEqualsLeavesInputPositionedAtEndOfMessage()
com.hedera.pbj.runtime.CodecParseMethodTest ‑ measureIsNotInflatedByTrailingData()
com.hedera.pbj.runtime.CodecParseMethodTest ‑ measureLeavesInputPositionedAtEndOfMessage()
com.hedera.pbj.runtime.CodecParseMethodTest ‑ parseLeavesInputPositionedAtEndOfMessage()
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] FLOAT, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49e6f8@32d3b965, [0.1, 0.5, 100.0], 12, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49e920@7fb2645b
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] STRING, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d4aabd0@436103b1, [string 1, testing here, testing there], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d4aadf8@427128a6
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] BYTES, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d4ab020@7a085d02, [010203, ff7f0f, 42da07370bff], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d4ab248@7842260f
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] DOUBLE, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49eb48@25031fd4, [0.1, 0.5, 100.0, 1.7653472635472653E240], 32, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49ed70@54faa040
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [3] BOOL, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49ef98@22195d06, [true, false, false, true, true, true], 6, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49f1c0@1f591b2e
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [4] ENUM, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49f3e8@c10f7a8, [0, 2, 1], 3, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x000000001d49f610@64096a41
…

♻️ This comment has been updated with latest results.

github-actions · 2026-08-20T21:18:16Z

Integration Test Report

428 files ±0 428 suites ±0 16m 56s ⏱️ - 4m 9s
115 048 tests ±0 115 048 ✅ ±0 0 💤 ±0 0 ❌ ±0
115 292 runs ±0 115 292 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 6ff8dd8. ± Comparison against base commit 6762bb6.

This pull request removes 2 and adds 2 tests. Note that renamed tests count towards both.

com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [1] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000012be6d48@43113fba
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [2] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000012be6f90@76681fc4

com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [1] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x000000004bc53378@25544039
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [2] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x000000004bc535c0@6d2f52c3

♻️ This comment has been updated with latest results.

anthony-swirldslabs

You want to add javadocs to any new protected/public methods. I also left a minor comment. Other than that, the changes look great. Thanks!

anthony-swirldslabs · 2026-08-20T22:29:13Z

-    default T parse(@NonNull ReadableSequentialData input, boolean strictMode, boolean parseUnknownFields, int maxDepth)
+    public final T parse(
+            @NonNull ReadableSequentialData input, boolean strictMode, boolean parseUnknownFields, int maxDepth)


Minor: I'd go as far as to actually nuke all the overloads that supply default argument values. In the current fix where these methods have simply been made final, the benefit of the code simplification is a bit small to my liking.

anthony-swirldslabs · 2026-08-20T22:31:14Z

+        return writeImpl(item, output, startOffset);
+    }
+
+    // some classes generate this overload, but not all


How come? Our codec generator here in PBJ is supposed to generate them (or not) unconditionally.

Also, you want to add javadocs to any public/protected methods that you introduce here.

I looked into it and changed the comment to

CodecWriteByteArrayMethodGenerator.java generates this method for Codecs but not JsonCodecs We provide a default implementation here so hand written codecs don't need to.

Thanks! If that comment in code says exactly this verbatim, then that part would look good.

…onsistent

anthony-swirldslabs reviewed Aug 20, 2026

View reviewed changes

ldintr added 4 commits August 24, 2026 16:55

.Gradle No Fuzz

d23f5d8

Switch Codec/JsonCodec to an abstract class, made parse/write paths c…

a17a7ca

…onsistent

Add PbjReader/PbjWriter and tests

19dc62d

Switched to PbjReader/PbjWriter

4e66f26

ldintr force-pushed the ldintr-wip branch from 6ff1bff to 4e66f26 Compare August 24, 2026 20:56

Inline an addToList, and reserved space for the list

6ff8dd8

Navigation Menu

Uh oh!

Codec Interface to Abstract class, made all paths call parseImpl/writeImpl - #892

Codec Interface to Abstract class, made all paths call parseImpl/writeImpl#892
ldintr wants to merge 5 commits into
mainfrom
ldintr-wip

ldintr commented Aug 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 20, 2026 •

edited

Loading

Uh oh!

anthony-swirldslabs left a comment

Uh oh!

anthony-swirldslabs Aug 20, 2026

Uh oh!

anthony-swirldslabs Aug 20, 2026

Uh oh!

ldintr Aug 21, 2026 •

edited

Loading

Uh oh!

anthony-swirldslabs Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ldintr commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JUnit Test Report

Uh oh!

github-actions Bot commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Integration Test Report

Uh oh!

anthony-swirldslabs left a comment

Choose a reason for hiding this comment

Uh oh!

anthony-swirldslabs Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

anthony-swirldslabs Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

ldintr Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anthony-swirldslabs Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ldintr commented Aug 20, 2026 •

edited

Loading

github-actions Bot commented Aug 20, 2026 •

edited

Loading

github-actions Bot commented Aug 20, 2026 •

edited

Loading

ldintr Aug 21, 2026 •

edited

Loading