Skip to content

Conversation

konstantinabl
Copy link
Contributor

@konstantinabl konstantinabl commented Jun 23, 2025

Description:

Updated the generateCacheKey function to properly serialize nested objects using JSON.stringify()
Also removed type IArgument = Record<string, any> as it is not correctly representing what params could be passed to methods.

Related issue(s):

Fixes #3872

Testing

  • Added comprehensive unit tests covering:
  • Basic nested object serialization
  • Differentiation between different nested values
  • Complex nested structures with arrays
  • Deep nesting scenarios

Notes for reviewers

Previously, the test imports were referencing modules from the dist directory. This caused incorrect behavior when checking for instances of RequestDetails in the generateCacheKey method, as the imported class was not the same as the one used in the source. By updating the imports to point to the original source files, type checks (such as instanceof RequestDetails) now behave correctly.
https://github.com/hiero-ledger/hiero-json-rpc-relay/pull/3873/files#diff-cdce3dcc7c73d10d08f1dccc43860f2b14d0feb8321935967728568ea7f2931eR7

@konstantinabl konstantinabl requested review from a team as code owners June 23, 2025 18:20
@konstantinabl konstantinabl requested a review from acuarica June 23, 2025 18:20
@konstantinabl konstantinabl linked an issue Jun 23, 2025 that may be closed by this pull request
@lfdt-bot
Copy link

lfdt-bot commented Jun 23, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

Copy link

github-actions bot commented Jun 23, 2025

Test Results

 20 files   -  2  271 suites   - 44   18m 13s ⏱️ - 3m 26s
667 tests ± 0  662 ✅ + 1  5 💤 ±0  0 ❌  - 1 
683 runs   - 80  678 ✅  - 78  5 💤 ±0  0 ❌  - 2 

Results for commit f51ee76. ± Comparison against base commit c4b9c0a.

♻️ This comment has been updated with latest results.

@konstantinabl konstantinabl self-assigned this Jun 23, 2025
@konstantinabl konstantinabl added the bug Something isn't working label Jun 23, 2025
@konstantinabl konstantinabl added this to the 0.70.0 milestone Jun 23, 2025
natanasow
natanasow previously approved these changes Jun 24, 2025
Copy link
Member

@natanasow natanasow left a comment

Choose a reason for hiding this comment

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

LGTM.

simzzz
simzzz previously approved these changes Jun 24, 2025
Copy link
Contributor

@simzzz simzzz left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Konstantina Blazhukova <[email protected]>
@konstantinabl konstantinabl dismissed stale reviews from simzzz and natanasow via 310b936 June 26, 2025 14:08
@konstantinabl konstantinabl requested a review from acuarica June 26, 2025 14:09
Copy link
Contributor

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

looking good

konstantinabl and others added 4 commits June 27, 2025 12:17
Signed-off-by: Konstantina Blazhukova <[email protected]>
Co-authored-by: Luis Mastrangelo <[email protected]>
Signed-off-by: konstantinabl <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
@konstantinabl konstantinabl requested a review from acuarica June 27, 2025 09:42
Signed-off-by: Konstantina Blazhukova <[email protected]>
Copy link
Contributor

@Ferparishuertas Ferparishuertas left a comment

Choose a reason for hiding this comment

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

LGTM

@konstantinabl konstantinabl merged commit e915d2d into main Jul 1, 2025
44 checks passed
@konstantinabl konstantinabl deleted the 3872-objects-in-params-are-not-serialized-when-added-to-cache-key branch July 1, 2025 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Objects in params are not serialized when added to cache key

6 participants