Resource update #44
Resource update #44
Conversation
- Implemented tests to verify that local and remote resources are correctly deleted from the resource service. - Ensured that deleted resources are no longer retrievable from all APIs.
WalkthroughThe changes introduce new and updated methods for handling resource uploads and deletions, including the addition of resource identifiers to upload operations. New test cases verify resource deletion behavior. Several constructors and method signatures were updated for consistency, and nullable reference annotations were added to clarify property contracts. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ResourceService
participant RemoteResourceService
participant CrdtRepository
Client->>ResourceService: AddExistingRemoteResource(resourcePath, clientId, resourceId, remoteId)
ResourceService->>RemoteResourceService: UploadResource(resourceId, resourcePath)
RemoteResourceService-->>ResourceService: UploadResult
ResourceService->>CrdtRepository: AddLocalResource(...)
CrdtRepository-->>ResourceService: (confirmation)
ResourceService-->>Client: (completion)
Client->>ResourceService: DeleteResource(clientId, resourceId)
ResourceService->>CrdtRepository: DeleteLocalResource(resourceId)
CrdtRepository-->>ResourceService: (confirmation)
ResourceService-->>Client: (completion)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/SIL.Harmony.Core/IRemoteResourceService.cs(1 hunks)src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs(1 hunks)src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs(1 hunks)src/SIL.Harmony/Db/CrdtRepository.cs(1 hunks)src/SIL.Harmony/Resource/CreateRemoteResourceChange.cs(1 hunks)src/SIL.Harmony/Resource/CreateRemoteResourcePendingUpload.cs(1 hunks)src/SIL.Harmony/Resource/HarmonyResource.cs(1 hunks)src/SIL.Harmony/ResourceService.cs(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/SIL.Harmony/Db/CrdtRepository.cs (1)
src/SIL.Harmony/Resource/LocalResource.cs (1)
LocalResource(6-16)
src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs (3)
src/SIL.Harmony.Core/IRemoteResourceService.cs (2)
Task(17-17)Task(25-25)src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs (2)
Task(18-25)Task(29-43)src/SIL.Harmony/ResourceService.cs (12)
Task(31-46)Task(48-93)Task(95-102)Task(104-122)Task(124-131)Task(133-138)Task(140-149)Task(151-160)Task(162-167)Task(169-183)Task(185-188)Task(190-193)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: Analyze (csharp)
🔇 Additional comments (9)
src/SIL.Harmony/Db/CrdtRepository.cs (1)
393-396: LGTM! Clean implementation following established patterns.The new
DeleteLocalResourcemethod correctly follows the same pattern as other delete operations in the repository, usingExecuteDeleteAsync()for efficient bulk deletion.src/SIL.Harmony/Resource/CreateRemoteResourceChange.cs (1)
6-6: LGTM! Parameter naming standardized for consistency.The rename from
resourceIdtoentityIdaligns with the base class constructor and improves naming consistency across the resource management codebase.src/SIL.Harmony.Core/IRemoteResourceService.cs (1)
22-25: All implementations updated for the new UploadResource signatureVerified that the only concrete implementation (RemoteServiceMock in
src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs) and all callers have been updated to include theresourceIdparameter. No other classes implementIRemoteResourceService, so the interface change is fully reflected.src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs (1)
29-29: Mock implementation updated to match interface signature.The
resourceIdparameter addition correctly aligns with the updatedIRemoteResourceServiceinterface. It's acceptable that the mock doesn't utilize this parameter since it focuses on simulating upload behavior rather than resource tracking.src/SIL.Harmony/Resource/CreateRemoteResourcePendingUpload.cs (1)
6-7: Excellent refactor to primary constructor with consistent naming.The change to primary constructor syntax with
entityIdparameter improves code conciseness and aligns with the naming standardization across resource management classes.src/SIL.Harmony/Resource/HarmonyResource.cs (1)
1-14: LGTM! Good use of nullable reference type annotations.The
MemberNotNullWhenattributes correctly express the null-state contract between the boolean properties and their corresponding nullable fields, helping the compiler provide better null-safety warnings.src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs (1)
211-243: LGTM! Comprehensive test coverage for resource deletion.Both tests properly verify that deleted resources are removed from all retrieval APIs (
GetResource,GetLocalResource, andAllResources). The tests cover both local and remote resource deletion scenarios with clear Arrange-Act-Assert structure.src/SIL.Harmony/ResourceService.cs (2)
31-46: LGTM! Well-structured method for adding existing remote resources.The method properly validates resources setup, checks file existence, and follows the established pattern of creating changes and storing local resources.
69-69: LGTM! Consistent updates to UploadResource calls.All calls to
UploadResourcehave been correctly updated to pass the resource ID as the first parameter, aligning with the new interface signature.Also applies to: 113-113, 136-136
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Improvements