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

06302 - turn NodeId into a record containing a long #6370

Merged
merged 36 commits into from May 12, 2023

Conversation

edward-swirldslabs
Copy link
Contributor

@edward-swirldslabs edward-swirldslabs commented May 2, 2023

Description:

  • removes isMirror() related attributes from NodeId
  • removes EventCreationRule from NodeId
  • turns NodeId into a record wrapping a long.
  • deprecates getIdAsInt()
  • changes equals(long id) to matches(long id)
  • backs create() with a static concurrent hashmap from Long to NodeId to minimize construction of the record.

Related issue(s):

Fixes #6302

Notes for reviewer:

  • This PR contains breaking API changes impacting Services with respect to how NodeId is used
  • Mirror node NodeIds were never really used.
  • The event creation rule was just to prevent creation of mirror nodes.
  • Removing the hashcode implementation causes the app to misbehave, so it is left in.

Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
# Conflicts:
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/components/EventCreator.java
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/event/creation/ChatterEventCreator.java
#	platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/state/SwirldStateManagerImplTests.java
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
@github-actions
Copy link

github-actions bot commented May 3, 2023

Node: Unit Test Results

  1 362 files    1 362 suites   1h 25m 18s ⏱️
97 615 tests 97 608 ✔️ 7 💤 0
99 246 runs  99 239 ✔️ 7 💤 0

Results for commit d8fa9c6.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented May 3, 2023

Node: Integration Test Results

    3 files      3 suites   13m 26s ⏱️
151 tests 151 ✔️ 0 💤 0
152 runs  152 ✔️ 0 💤 0

Results for commit d8fa9c6.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented May 3, 2023

Node: E2E Test Results

    1 files      1 suites   17m 23s ⏱️
310 tests 310 ✔️ 0 💤 0
328 runs  328 ✔️ 0 💤 0

Results for commit d8fa9c6.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented May 3, 2023

Platform: JUnit Test Report

     509 files           1 errors  508 suites   18m 29s ⏱️
13 641 tests 13 605 ✔️ 36 💤 0
15 425 runs  15 389 ✔️ 36 💤 0

Results for commit d8fa9c6.

♻️ This comment has been updated with latest results.

Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
# Conflicts:
#	platform-sdk/platform-apps/tests/PlatformTestingTool/src/test/java/com/swirlds/demo/platform/FCMQueryControllerTest.java
@codecov
Copy link

codecov bot commented May 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (34a84cd) 0.00% compared to head (618a989) 0.00%.

❗ Current head 618a989 differs from pull request most recent head d8fa9c6. Consider uploading reports for the commit d8fa9c6 to get more accurate results

Additional details and impacted files
@@       Coverage Diff       @@
##   develop   #6370   +/-   ##
===============================
===============================

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@edward-swirldslabs edward-swirldslabs marked this pull request as ready for review May 5, 2023 16:48
@edward-swirldslabs edward-swirldslabs requested a review from a team as a code owner May 5, 2023 16:48
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
# Conflicts:
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/SwirldsPlatform.java
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
# Conflicts:
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/SwirldsPlatform.java
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
hendrikebbers
hendrikebbers previously approved these changes May 12, 2023
# Conflicts:
#	platform-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/PlatformMetrics.java
#	platform-sdk/swirlds-unit-tests/core/swirlds-platform-test/src/test/java/com/swirlds/platform/test/components/EventMapperTest.java
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
@sonarcloud
Copy link

sonarcloud bot commented May 12, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented May 12, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 11 Code Smells

22.5% 22.5% Coverage
0.0% 0.0% Duplication

@edward-swirldslabs edward-swirldslabs merged commit 074dd7c into develop May 12, 2023
15 of 18 checks passed
@edward-swirldslabs edward-swirldslabs deleted the 06302-Turn-NodeId-into-record-of-a-long branch May 12, 2023 18:25
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.

Noncontiguous NodeId - Turn NodeId into a record containing a long
4 participants