refactor(torii): update query logic for them to not block #2397
Conversation
WalkthroughOhayo, sensei! The changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant QueryQueue
participant SQL
participant Broker
Client->>QueryQueue: Enqueue query
QueryQueue->>SQL: Execute queries based on QueryType
SQL->>QueryQueue: Return results
QueryQueue->>SQL: Retrieve updated entity if SetEntity
SQL->>QueryQueue: Return updated entity
QueryQueue->>Broker: Send BrokerMessage(EntityUpdated)
Tip OpenAI O1 model for chat
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
lambda-0x
force-pushed
the
spr/main/1b6c03a5
branch
from
adae7e8 to
68fc21f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
crates/torii/core/src/query_queue.rs (1)
68-75: Well-implemented method for adding publish queries.The
push_publish_querymethod correctly handles the addition of new publish queries to the queue. Consider adding a brief comment above the method to explain its purpose for future maintainability.crates/torii/libp2p/src/server/mod.rs (1)
522-534: Ohayo, sensei! The newset_entityfunction is a masterstroke in modularity.Encapsulating the database interaction logic into this function improves readability and maintainability. It would be beneficial to add detailed error handling documentation to aid in debugging and maintenance.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2397 +/- ##
==========================================
+ Coverage 67.89% 67.93% +0.03%
==========================================
Files 364 364
Lines 47773 47832 +59
==========================================
+ Hits 32435 32494 +59
Misses 15338 15338 ☔ View full report in Codecov by Sentry. |
lambda-0x
changed the title
lambda-0x
force-pushed
the
spr/main/1b6c03a5
branch
3 times, most recently
from
4085f11 to
3871fa8
Compare
commit-id:1b6c03a5
lambda-0x
force-pushed
the
spr/main/1b6c03a5
branch
from
3871fa8 to
cb290a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
crates/torii/core/src/sql.rs (1)
Line range hint
1050-1186: Ohayo sensei! The model query building function looks good, but could use more comments.The function handles various scenarios for different model types and correctly builds the create table and insert queries based on the model type and member details. The logic seems to be sound and there are no major issues.
However, given the complexity of the function, it would be beneficial to add more comments explaining the different scenarios and the reasoning behind the query building logic. This will greatly improve the maintainability and readability of the code.
Consider adding more comments to explain the different scenarios and the query building logic for each model type. This will make it easier for future developers to understand and maintain this complex function.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
spawn-and-move-db.tar.gzis excluded by!**/*.gztypes-test-db.tar.gzis excluded by!**/*.gz
Files selected for processing (3)
- crates/torii/core/src/query_queue.rs (3 hunks)
- crates/torii/core/src/sql.rs (21 hunks)
- crates/torii/libp2p/src/server/mod.rs (2 hunks)
Additional comments not posted (12)
crates/torii/core/src/query_queue.rs (7)
34-34: Ohayo, sensei! The change to thequeuefield looks good.The update to store tuples with the new
QueryTypeparameter will enable more effective handling of different query types during execution. Nice work!
40-44: The newQueryTypeenum is a great addition, sensei!Defining the
SetEntity(Ty)andOthervariants will enable more nuanced handling of different query types. This will definitely improve the control flow and functionality of theQueryQueue. Keep up the good work!
51-57: Updating theenqueuemethod to accept thequery_typeparameter is spot on, sensei!This change is essential to leverage the new
QueryTypeparameter in theexecute_allmethod for determining how to handle each query during execution. You're on the right track!
60-66: The update to thepush_frontmethod looks great, sensei!Accepting the
query_typeparameter here is crucial for leveraging the newQueryTypein theexecute_allmethod. You've got the right idea!
Line range hint
73-116: Ohayo, sensei! The enhancements to theexecute_allmethod are fantastic!The introduction of the match statement to direct the execution path based on the query type significantly improves the control flow and overall functionality of the
QueryQueue. Fetching a single row forSetEntityqueries and publishing anEntityUpdatedmessage is a great addition.Changing the return type to
Result<()>aligns well with the new focus on handling messages rather than counting affected rows.These changes really elevate the functionality of the
QueryQueue. Excellent work, sensei!
111-111: Using thesend_broker_messagehelper function here is a nice touch, sensei!Centralizing the logic for sending broker messages in a separate function definitely improves code organization and readability. Good thinking!
118-123: Thesend_broker_messagehelper function is a great addition, sensei!Centralizing the logic for sending different types of broker messages in this helper function definitely improves code organization and readability. It's a clean and effective way to handle sending broker messages. Well done!
crates/torii/libp2p/src/server/mod.rs (2)
522-534: Ohayo sensei! The refactoring looks great! 👍Moving the database interaction logic into a separate
set_entityfunction improves code modularity and readability. It encapsulates the concerns well and makes the code easier to understand and maintain. The error handling is preserved, and the function returns a result to indicate success or failure.This refactoring aligns with best practices and should facilitate easier testing and future enhancements. Excellent work!
352-368: The refactored function call looks perfect, sensei!You've correctly replaced the inline database call with the new
set_entityfunction. The parameters are passed correctly, and the error handling remains intact.This change completes the refactoring and ensures that the new modular function is utilized properly. Great job!
crates/torii/core/src/sql.rs (3)
184-194: Ohayo sensei! Theset_entityfunction looks great! 👍The changes incorporate the new
QueryType::SetEntityto categorize the insert query based on the entity type. This aligns perfectly with the PR objective of using theQueryTypeenum to manage different query types effectively.
300-311: Skipping comment as the past review is still applicable.The past review comment on this change is still valid:
coderabbitai[bot]: Update query for setting model members.
The update query for setting model members now includes a clearer separation of concerns by explicitly handling the update timestamps and event IDs. This change improves the clarity of the SQL operations and ensures that updates are handled more effectively.
Line range hint
875-900: Ohayo sensei! The recursive delete query building looks solid!The function correctly handles the recursive deletion of an entity and its nested members. By using
push_front, it ensures that the delete queries are executed before any other queries in the queue, which is crucial for maintaining data integrity.
Summary by CodeRabbit
New Features
QueryTypefor better categorization during execution.Bug Fixes
Refactor