Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 392: PBJ codecs to provide access to raw field bytes during parsing #396

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

artemananiev
Copy link
Member

Fix summary:

  • A new method is provided in ProtoParserTools: extractRawFieldBytes()
  • This method doesn't depend on any codecs, this is why it's in ProtoParserTools, not in generated codec classes
  • The method can't work with nested fields, e.g. there is no way to read field A in field B in the input. However, it's trivial to implement this in user code by calling the method twice:
final Bytes externalFieldBytes = ProtoParserTools.extractRawFieldBytes(input, fieldB);
final Bytes internalFieldBytes = ProtoParserTools.extractRawFieldBytes(externalFieldBytes.toReadableSequentialData(), fieldA);
  • If needed, a new utility method can be provided to cover this scenario. It will accept a list of field definitions and will chain the calls appropriately
  • Unit tests for the new method are added

Fixes: #392
Signed-off-by: Artem Ananev [email protected]

Copy link

github-actions bot commented Feb 18, 2025

JUnit Test Report

   67 files  ±0     67 suites  ±0   2m 40s ⏱️ - 2m 8s
1 272 tests +7  1 269 ✅ +7   3 💤 ±0  0 ❌ ±0 
7 127 runs  +7  7 108 ✅ +7  19 💤 ±0  0 ❌ ±0 

Results for commit b7bb743. ± Comparison against base commit c189321.

This pull request removes 8 and adds 14 tests. Note that renamed tests count towards both.
, 1
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] FLOAT, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031a870@e7b3e54, [0.1, 0.5, 100.0], 12, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031aa80@78d61f17
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] STRING, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c0327808@36120a8b, [string 1, testing here, testing there], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c0327a18@63d66761
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] BYTES, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c0327c28@2e1ad7de, [010203, ff7f0f, 42da07370bff], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c032c000@7c56c911
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] DOUBLE, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031ac90@c1386b4, [0.1, 0.5, 100.0, 1.7653472635472653E240], 32, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031aea0@53d9af1
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [3] BOOL, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031b0b0@c89e263, [true, false, false, true, true, true], 6, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031b2c0@4d5ea776
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [4] ENUM, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031b4d0@5d68be4f, [Mock for EnumWithProtoMetadata, hashCode: 1836984213, Mock for EnumWithProtoMetadata, hashCode: 1097558044, Mock for EnumWithProtoMetadata, hashCode: 1188469924], 3, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f64c031b6e0@21bf308
com.hedera.pbj.runtime.Utf8ToolsTest ‑ [4] 
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ testExtractBytesBytesField()
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ testExtractBytesMessageField()
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ testExtractBytesNullField()
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ testExtractBytesNullInput()
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ testExtractBytesRepeatedField()
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ testExtractBytesStringField()
com.hedera.pbj.runtime.ProtoParserToolsTest ‑ testExtractBytesUnknownField()
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] FLOAT, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f0f2032ae50@1ed52f44, [0.1, 0.5, 100.0], 12, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f0f2032b060@771afdd5
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [1] STRING, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f0f2033c000@6bfbab1c, [string 1, testing here, testing there], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f0f2033c210@349aeec4
com.hedera.pbj.runtime.ProtoWriterToolsTest ‑ [2] BYTES, com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f0f2033c420@6516181f, [010203, ff7f0f, 42da07370bff], com.hedera.pbj.runtime.ProtoWriterToolsTest$$Lambda/0x00007f0f2033c630@40cb95c1
…

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Feb 18, 2025

Integration Test Report

    383 files  ±0      383 suites  ±0   13m 40s ⏱️ -49s
114 358 tests ±0  114 358 ✅ ±0  0 💤 ±0  0 ❌ ±0 
114 586 runs  ±0  114 586 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b7bb743. ± Comparison against base commit c189321.

This pull request removes 3 and adds 3 tests. Note that renamed tests count towards both.
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [1] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x00007f9bf456a178@43855735
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [2] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x00007f9bf456a3a8@1e271db
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [3] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x00007f9bf456a5d8@24e6e465
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [1] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x00007fd1a05656d8@1f4dbb3f
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [2] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x00007fd1a0565908@244d4131
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [3] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x00007fd1a0565b38@7b90a3df

♻️ This comment has been updated with latest results.

@netopyr
Copy link

netopyr commented Feb 18, 2025

The API looks good to me. Thanks a lot @artemananiev!

Copy link
Contributor

@anthony-swirldslabs anthony-swirldslabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM functionally, but I left a few comments.

Copy link
Contributor

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tyvm @artemananiev !

Copy link
Contributor

@anthony-swirldslabs anthony-swirldslabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you!

@anthony-swirldslabs
Copy link
Contributor

LGTM. Thank you!

Although, you may want to check tests in the PR checks.

@artemananiev
Copy link
Member Author

LGTM. Thank you!

Although, you may want to check tests in the PR checks.

ProtoParserTools.testReadString() failed, because random string length to generate happened to be 0. It has nothing to do with the current fix, but I'll file a ticket about this test failure.

@artemananiev artemananiev merged commit 2ed3662 into main Feb 18, 2025
10 checks passed
@artemananiev artemananiev deleted the 392-M-extract-field-bytes branch February 18, 2025 21:57
@artemananiev artemananiev added this to the 0.9.18 milestone Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need a way to access raw field bytes during parsing
5 participants