Skip to content

Conversation

@shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Oct 25, 2025

Relevant issue(s)

Resolves #4110

Description

Gate the following P2P ops:

  • Connect
  • SetReplicator
  • DeleteReplicator
  • GetAllReplicators
  • AddP2PCollections
  • RemoveP2PCollections
  • GetAllP2PCollections
  • AddP2PDocuments
  • RemoveP2PDocuments
  • GetAllP2PDocuments

For Reviewers:

  • Commit by commit should be clean (the first commit is just refactor and splitting of a large testing framework file)

Future Todo(s) (ones with tick boxes will likely be done in this PR):

  • Finish testing p2p peer connect op
  • Finish testing p2p replicator ops
  • Finish testing p2p documents subscription ops
  • Finish testing p2p collection subscription ops
  • Finish testing p2p peer connect op with admin relation
  • Finish testing p2p replicator ops with admin relation
  • Finish testing p2p documents subscription ops with admin relation
  • Finish testing p2p collection subscription ops with admin relation
  • Gate PeerInfo
  • Gate SyncDocuments
  • Test gap for GetAllReplicators: Test ListReplicators/GetAllReplicators Operation with NAC #4109

@shahzadlone shahzadlone added this to the DefraDB v0.20 milestone Oct 25, 2025
@shahzadlone shahzadlone requested a review from a team October 25, 2025 01:27
@shahzadlone shahzadlone self-assigned this Oct 25, 2025
@shahzadlone shahzadlone added feature New feature or request area/p2p Related to the p2p networking system area/acp Related to the acp (access control) system needed for v1 labels Oct 25, 2025
@shahzadlone shahzadlone force-pushed the lone/feat/p2p-replicator branch 2 times, most recently from f3e89a7 to a28120e Compare October 25, 2025 01:43
Copy link
Collaborator

@ChrisBQu ChrisBQu left a comment

Choose a reason for hiding this comment

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

LGTM.

Maybe it makes sense to create/link a second issue that mentions separating out the parts of p2p.go, before merging, since I don't think this PR's title or linked issue implies that at all. Maybe it would be enough to just mention it in the description of this PR before merging. But the code itself looks good.

@shahzadlone
Copy link
Member Author

LGTM.

Maybe it makes sense to create/link a second issue that mentions separating out the parts of p2p.go, before merging, since I don't think this PR's title or linked issue implies that at all. Maybe it would be enough to just mention it in the description of this PR before merging. But the code itself looks good.

I already added to the description a few minutes ago, haha. cheers.

@codecov
Copy link

codecov bot commented Oct 25, 2025

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.25%. Comparing base (c2ecf94) to head (1c801ea).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
internal/db/p2p.go 90.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4111      +/-   ##
===========================================
+ Coverage    77.18%   77.25%   +0.07%     
===========================================
  Files          482      482              
  Lines        37243    37263      +20     
===========================================
+ Hits         28745    28787      +42     
+ Misses        6213     6202      -11     
+ Partials      2285     2274      -11     
Flag Coverage Δ
all-tests 77.25% <90.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
acp/types/types.go 100.00% <ø> (ø)
internal/db/p2p.go 66.18% <90.00%> (+4.11%) ⬆️

... and 11 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2ecf94...1c801ea. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shahzadlone shahzadlone force-pushed the lone/feat/p2p-replicator branch 6 times, most recently from 2e24d9b to 92a500c Compare October 27, 2025 15:17
@shahzadlone shahzadlone force-pushed the lone/feat/p2p-replicator branch from 92a500c to ae62655 Compare October 27, 2025 15:55
@shahzadlone shahzadlone marked this pull request as ready for review October 27, 2025 15:56
@shahzadlone shahzadlone merged commit e874405 into sourcenetwork:develop Oct 27, 2025
80 of 85 checks passed
@shahzadlone shahzadlone deleted the lone/feat/p2p-replicator branch October 27, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/acp Related to the acp (access control) system area/p2p Related to the p2p networking system feature New feature or request needed for v1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gate collection doc ops with NAC

2 participants