feat: Architectural Refactor of Dependency Injection and Query System #412
Conversation
Updated the IParseCurrentUserController interface to include a new CurrentUser property that returns a ParseUser object. This enhancement allows direct access to the current user while keeping existing method signatures unchanged.
Updated `ParseObjectIdComparer` to be a readonly static field for better efficiency. Enhanced the `Decode` method to support additional operations such as "Add", "AddUnique", "Remove", "AddRelation", "RemoveRelation", and "Batch". Added XML documentation for improved clarity and maintainability.
Updated `ParseDataDecoder` to utilize `IServiceHub` for improved access to `ClassController`. Modified the constructor and enhanced the `Decode` method to support new functionality. Added clarifying comments in `ParseObjectCoder` regarding the `Encode` method and its usage context.
- Introduced a static `Or` method to combine multiple queries using a logical OR operation, ensuring all queries are for the same class. - Implemented `GetHashCode` to generate a unique hash code based on relevant properties, replacing the previous placeholder.
- Added `Services` property to `CustomServiceHub` for better access to service instances. - Replaced `CurrentUser` property with a reference to `Services.CurrentUserController.CurrentUser`. - Updated `Decoder` initialization in `LateInitializedMutableServiceHub` to use `this`. - Changed `Decoder` handling in `MutableServiceHub` and `ServiceHub` for improved encapsulation. - Added new using directives in `ParseClient.cs` and removed unused ones. - Introduced `FinalizeDecoder` method in `ParseClient` for proper decoder initialization. - Modified `FetchAsyncInternal` in `ParseUser` to ensure correct fetching behavior. - Removed commented-out `SessionToken` and added `AuthData` property for authentication management.
Removed the IServiceHub parameter from the Decode method, simplifying its interface to only require the object data parameter. Refactor decoding logic in ParseCloudCodeController and ParseConfiguration Removed the `serviceHub` parameter from the `Decoder.Decode` method calls in both `ParseCloudCodeController.cs` and `ParseConfiguration.cs`. # This is a Breaking Change. At least to simplify method calls.
Removed `serviceHub` parameter from method signatures across multiple classes, replacing it with `this.Services` for improved encapsulation and consistency. Simplified object decoding in `ParseFieldOperations` by eliminating unnecessary parameters. Updated `ParseDataDecoder` and `ParseDataEncoder` to utilize the new service access pattern. Ensured `ParseObjectCoder` aligns with these changes for uniformity across the codebase.
Updated the `ConvertToJSON` method in multiple classes implementing the `IJsonConvertible` interface to return an `IDictionary<string, object>` instead of an `object`. This change improves the method's capability to generate structured JSON-compatible representations, accommodating more complex data structures. A new `ConvertValueToJSON` method was also added to the `ParseSetOperation` class to assist in converting values to JSON format.
Removed the IServiceHub parameter from Decode method calls in CloudTests, ConfigTests, and DecoderTests to simplify method signatures. Updated assertions in tests to ensure expected outcomes are maintained. Improved readability in GeoPointTests by formatting Decode method calls.
- Added necessary using directives in UserTests.cs. - Enhanced `TestLogOut` method with new mock setups for user logout functionality. - Introduced `HeadersContainSessionToken` method for header verification. - Removed unused `Decoder` initialization in MutableServiceHub.cs. - Updated `ConversionTests.cs` to use `Assert.ThrowsExactly` for better exception specificity. - Added `InternalExtensionsTests.cs` with tests for utility methods like `Safe()`, `GetOrDefault()`, and `OnSuccess()`. - Updated `Parse.Tests.csproj` to include `coverlet.collector` for code coverage analysis. - Introduced `ParseQueryExtensionsTests.cs` to validate LINQ operator translations in `ParseQuery`. - Removed a warning directive in `ProgressTests.cs` without functional changes. - Added `TransientCacheControllerTests.cs` to test cache functionality, including add, remove, and clear operations. - Modified `UserTests.cs` to add tests for missing properties during sign-up and ensure password removal from server data.
…ndRunner error handling and JSON support Added new using directives for JSON serialization and Parse namespaces. Improved handling of HTTP response codes, specifically for 100, 201, 400, 401, 403, and 404. Implemented deserialization of error payloads for 400 responses, with appropriate exception handling. Updated 404 response to include more context in the exception message. Retained comment on the removal of the installationFetchTask variable due to async/await simplifications. Updated `ParseFailureException` to include a new error code `BadRequest` (400) with XML documentation. Introduced `ParseErrorPayload` class with properties `code` (int) and `error` (string) for better error handling.
Updated `ParseQueryControllerTests` to remove `mockDecoder` in favor of `serviceHub.Decoder`, with corresponding changes to the setup method and test assertions. Removed several test methods for simplification. Deleted `ParseQueryExtensionsTests` as part of consolidation of tests. Introduced `ParseQueryTests` with comprehensive tests for the `ParseQuery` class, covering various query methods and ensuring proper interaction with `IServiceHub`. This enhances the overall test coverage.
Updated `DecoderTests.cs` to include tests for Increment, Delete, Add, AddUnique, and Remove operations. Reformatted existing primitive decoding tests for consistency. Modified `ParseObject` instantiation in `TestDecodeDictionary` to pass `Client` as a parameter.
- Updated `ACLTests.cs` with new tests for `ParseACL` class. - Removed unnecessary code in `AnalyticsTests.cs` and added warnings. - Introduced `AttributeTests.cs` for testing various attributes. - Refactored `CloudControllerTests.cs` to improve clarity and mock usage. - Enhanced clarity and consistency in `ConfigTests.cs` and `ConversionTests.cs`. - Added new tests in `EncoderTests.cs` for `ParseObject` encoding. - Created `GeoDistanceTest.cs` for testing `ParseGeoDistance` class. - Improved organization and exception handling in `GeoPointTests.cs`, `InternalExtensionsTests.cs`, and `ProgressTests.cs`. - Updated `ObjectCoderTests.cs` and `ObjectControllerTests.cs` for better test clarity. - Added new tests in `ObjectStateTests.cs` for cloning and decoding. - Introduced `ParseOperationsTests.cs` for various operation tests. - Enhanced clarity and exception handling in `ParseQueryTests.cs` and `RelationTests.cs`. - Improved organization and initialization in `UserTests.cs`.
|
🚀 Thanks for opening this pull request! |
📝 WalkthroughWalkthroughThis update introduces extensive changes to both the Parse SDK and its test suite. The core SDK refactors the data decoding pipeline, standardizes JSON conversion return types to Changes
Sequence Diagram(s)sequenceDiagram
participant Client as ParseClient
participant Hub as ServiceHub/MutableServiceHub
participant Decoder as ParseDataDecoder
participant Controller as ParseCloudCodeController
participant Runner as ParseCommandRunner
participant Server as HTTP Server
Client->>Hub: Initialize services
Client->>Decoder: Ensure Decoder is set for each hub
Controller->>Runner: Prepare and send command
Runner->>Server: HTTP request
Server-->>Runner: HTTP response (status, content)
Runner->>Runner: Handle status codes (200, 400, 401, 403, 404, 410, 5xx)
alt Success (200/201)
Runner->>Decoder: Decode response data
Decoder->>Hub: Access ClassController as needed
Decoder-->>Controller: Return decoded result
Controller-->>Client: Return result
else Error (400/401/403/404/410/5xx)
Runner-->>Controller: Throw ParseFailureException (with error code/message)
Controller-->>Client: Propagate exception
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40–60 minutes Complexity:
Estimated review time: Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (2)
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: 13
🔭 Outside diff range comments (2)
Parse/Platform/Location/ParseGeoPoint.cs (1)
61-61: Fix incorrect debug message in longitude setter.The debug message shows "Invalid Latitude" but should be "Invalid Longitude" since this is in the longitude setter.
- Debug.WriteLine($"Invalid Latitude: {value}"); + Debug.WriteLine($"Invalid Longitude: {value}");Parse.Tests/ParseQueryTests.cs (1)
224-224: Fix typo in variable name- var Client= new ParseClient(new ServerConnectionData { Test = true }, hub); + var Client = new ParseClient(new ServerConnectionData { Test = true }, hub);
🧹 Nitpick comments (29)
Parse/Infrastructure/Data/ParseObjectCoder.cs (1)
4-4: Remove unused import.The
System.Diagnosticsimport appears to be unused in this file.-using System.Diagnostics;Parse/Platform/Queries/ParseQuery.cs (1)
8-8: Cosmetic change - unnecessary blank line addition.This blank line addition doesn't add value and may have been added accidentally.
Parse.Tests/CloudControllerTests.cs (1)
16-16: Address the refactoring warning comment.The warning indicates incomplete refactoring work. Ensure this class refactoring is completed before merging to production.
Would you like me to help identify what aspects of the refactoring still need completion?
Parse.Tests/TransientCacheControllerTests.cs (2)
18-32: Consider testing edge cases and improving assertion robustness.The test correctly validates basic add/retrieve functionality, but consider these improvements:
- Test with null values and empty strings
- Use more specific assertions (e.g.,
Assert.AreEqual("testValue", result.ToString())if result could be of different types)- Consider testing with different data types beyond strings
[TestMethod] [Description("Tests that the cache controller can add and then retrieve a value.")] public async Task LoadAsync_AddAsync_CanRetrieveValue() { // Arrange var cacheController = new TransientCacheController(); var initialCache = await cacheController.LoadAsync(); // Act await initialCache.AddAsync("testKey", "testValue"); var finalCache = await cacheController.LoadAsync(); - finalCache.TryGetValue("testKey", out var result); + bool found = finalCache.TryGetValue("testKey", out var result); // Assert + Assert.IsTrue(found, "Key should be found in cache"); Assert.IsNotNull(result); Assert.AreEqual("testValue", result); }
54-68: Consider testing cache state before clearing.While the test correctly validates that Clear empties the cache, it could be more robust by also asserting the initial state.
// Act + // Verify cache has items before clearing + var preCache = await cacheController.LoadAsync(); + Assert.AreEqual(2, preCache.Count, "Cache should contain 2 items before clearing"); + cacheController.Clear(); var finalCache = await cacheController.LoadAsync();Parse.Tests/GeoDistanceTest.cs (2)
22-28: Consider using named constants for Earth radius values.The hardcoded value
3958.8(Earth's radius in miles) should ideally match the actual constant used in the implementation to avoid test brittleness.Consider extracting the conversion constants or verifying they match the implementation:
[TestMethod] [Description("Tests the Miles conversion using a given Radians.")] public void Miles_ReturnsCorrectValue() { double radians = 2.5; ParseGeoDistance distance = new ParseGeoDistance(radians); - double expected = radians * 3958.8; + // Use the same constant as the implementation + const double EARTH_RADIUS_MILES = 3958.8; // Should match ParseGeoDistance.EARTH_RADIUS_MILES + double expected = radians * EARTH_RADIUS_MILES; Assert.AreEqual(expected, distance.Miles); }
32-39: Same constant concern applies to kilometers conversion.Similar to the miles test, ensure the kilometers constant matches the implementation.
double radians = 2.5; ParseGeoDistance distance = new ParseGeoDistance(radians); -double expected = radians * 6371.0; +const double EARTH_RADIUS_KM = 6371.0; // Should match ParseGeoDistance.EARTH_RADIUS_KM +double expected = radians * EARTH_RADIUS_KM;Parse.Tests/AttributeTests.cs (1)
47-47: Remove trailing comment.The comment
// 1at the end of the method name appears to be leftover from development.-public void PreserveWrapperTypes_HasCorrectlyAOTTypesRegistered()// 1 +public void PreserveWrapperTypes_HasCorrectlyAOTTypesRegistered()Parse.Tests/AnalyticsTests.cs (1)
24-36: Consider extracting common mock setup.All three test methods have similar mock setup patterns. Consider extracting this to a helper method to reduce duplication.
+private (MutableServiceHub hub, ParseClient client, Mock<IParseAnalyticsController> mockController) SetupTestEnvironment() +{ + var hub = new MutableServiceHub(); + var client = new ParseClient(new ServerConnectionData { Test = true }, hub); + var mockController = new Mock<IParseAnalyticsController>(); + var mockCurrentUserController = new Mock<IParseCurrentUserController>(); + + mockCurrentUserController + .Setup(controller => controller.GetCurrentSessionTokenAsync(It.IsAny<IServiceHub>(), It.IsAny<CancellationToken>())) + .ReturnsAsync("sessionToken"); + + hub.AnalyticsController = mockController.Object; + hub.CurrentUserController = mockCurrentUserController.Object; + + return (hub, client, mockController); +} [TestMethod] public async Task TestTrackEvent() { // Arrange - var hub = new MutableServiceHub(); - var client = new ParseClient(new ServerConnectionData { Test = true }, hub); - var mockController = new Mock<IParseAnalyticsController>(); - var mockCurrentUserController = new Mock<IParseCurrentUserController>(); - // ... rest of setup + var (hub, client, mockController) = SetupTestEnvironment();Parse/Platform/Configuration/ParseConfiguration.cs (1)
4-4: Remove unnecessary empty using statementThe added empty line in the using statements section doesn't serve any purpose.
-using System.Diagnostics; - -using Parse.Abstractions.Infrastructure; +using System.Diagnostics; +using Parse.Abstractions.Infrastructure;Parse/Infrastructure/Execution/ParseCommandRunner.cs (1)
122-122: Fix error message formattingThe error message has spacing issues around the concatenation.
- throw new ParseFailureException(ParseFailureException.ErrorCode.ERROR404, "Error 404 " +response.Item1+ response.Item2); + throw new ParseFailureException(ParseFailureException.ErrorCode.ERROR404, $"Error 404: {response.Item1} {response.Item2}");Parse.Tests/DecoderTests.cs (1)
3-4: Remove unnecessary empty linesMultiple empty lines have been added unnecessarily.
-using System.Linq; - -using Microsoft.VisualStudio.TestTools.UnitTesting; - -using Parse.Abstractions.Internal; +using System.Linq; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Parse.Abstractions.Internal;Also applies to: 6-6
Parse.Tests/ObjectCoderTests.cs (1)
107-121: Consider making ExecuteBatchRequests_EmptyList test async for consistency.The test correctly validates empty batch request handling, but since other tests in this class are async and this method likely returns a Task, consider making this test async as well for consistency.
[TestMethod] [Description("Tests that ExecuteBatchRequests correctly handles empty list.")] - public void ExecuteBatchRequests_EmptyList() + public async Task ExecuteBatchRequests_EmptyList() { var mockCommandRunner = new Mock<IParseCommandRunner>(); var mockDecoder = new Mock<IParseDataDecoder>(); var mockServiceHub = new Mock<IServiceHub>(); ParseObjectController controller = new ParseObjectController(mockCommandRunner.Object, mockDecoder.Object, new ServerConnectionData()); IList<ParseCommand> emptyList = new List<ParseCommand>(); - var task = controller.ExecuteBatchRequests(emptyList, "session", CancellationToken.None); + var task = await controller.ExecuteBatchRequests(emptyList, "session", CancellationToken.None); Assert.AreEqual(0, task.Count); }Parse.Tests/ParseOperationsTests.cs (8)
3-3: Remove unused importThe
System.Diagnosticsimport is not used in this test file.-using System.Diagnostics;
5-6: Remove unused importsThe
System.TextandSystem.Threading.Tasksimports are not used in this test file.-using System.Text; -using System.Threading.Tasks;
31-33: Remove extra blank lineAssert.IsTrue(new List<object> { 1, 2 }.SequenceEqual(result.Value as List<object>)); - }
54-61: Remove extra blank linesAssert.IsTrue(new List<object> { 1, 4, 5, 2, 3 }.SequenceEqual(result as List<object>)); - - var operation2 = new ParseAddUniqueOperation(new object[] { 4, 6, 7 }); var result2 = operation2.Apply(null, "testKey"); - Assert.IsTrue(new List<object> { 4, 6, 7 }.SequenceEqual(result2 as List<object>)); - }
99-101: Remove extra blank lineAssert.IsTrue(new List<object> { 1, 2 }.SequenceEqual(result.Value as List<object>)); - }
122-128: Remove extra blank linesAssert.IsTrue(new List<object> { 1, 4, 5, 2, 3 }.SequenceEqual(result as List<object>)); - var operation2 = new ParseAddOperation(new object[] { 1, 4, 5, 6 }); var result2 = operation2.Apply(null, "testKey"); - Assert.IsTrue(new List<object> { 1, 4, 5, 6 }.SequenceEqual(result2 as List<object>));
148-160: Remove extra blank linesAssert.AreEqual(operation, result); - result = operation.MergeWithPrevious(ParseDeleteOperation.Instance); Assert.AreEqual(operation, result); result = operation.MergeWithPrevious(new ParseAddOperation(new List<object> { 1 })); Assert.AreEqual(operation, result); - result = operation.MergeWithPrevious(new ParseAddUniqueOperation(new List<object> { 1 })); Assert.AreEqual(operation, result); - result = operation.MergeWithPrevious(null);
198-200: Remove extra blank lineAssert.AreEqual(operation, result); - }Parse.Tests/ParseQueryTests.cs (3)
2-2: Remove unused importThe
System.Collectionsimport is not used in this test file.-using System.Collections;
5-6: Remove unused importsThe
System.TextandSystem.Text.RegularExpressionsimports are not needed sinceRegexis fully qualified in the code.-using System.Text; -using System.Text.RegularExpressions;
216-219: Remove extra blank linesAssert.AreEqual(2, (innerWithin["$box"] as IEnumerable).Cast<object>().Count()); - - }Parse/Infrastructure/Data/ParseDataEncoder.cs (3)
6-6: Remove unused importThe
System.Runtime.ExceptionServicesimport is not used in this file.-using System.Runtime.ExceptionServices;
150-151: Improve method name for clarityThe method name
EncodeDictionaryStringDictis unclear. Consider a more descriptive name that reflects it handles nested dictionaries.- // Add a specialized method to handle double-only dictionaries - private object EncodeDictionaryStringDict(IDictionary<string, IDictionary<string, object>> dictionary, IServiceHub serviceHub) + // Encode dictionaries with nested dictionary values + private object EncodeNestedDictionary(IDictionary<string, IDictionary<string, object>> dictionary, IServiceHub serviceHub)
79-79: Update method call if rename is acceptedIf the method is renamed, update this call as well.
- IDictionary<string, IDictionary<string, object>> dictionary => EncodeDictionaryStringDict(dictionary, serviceHub), + IDictionary<string, IDictionary<string, object>> dictionary => EncodeNestedDictionary(dictionary, serviceHub),Parse.Tests/UserTests.cs (2)
3-5: Remove unused importsThe
System.DiagnosticsandSystem.Net.Httpimports appear to be unused in this test file.-using System.Diagnostics; -using System.Net.Http; using System.Runtime.CompilerServices;
5-5: Remove unused importThe
System.Runtime.CompilerServicesimport is not used in this test file.-using System.Runtime.CompilerServices;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
Parse.Tests/ACLTests.cs(7 hunks)Parse.Tests/AnalyticsTests.cs(1 hunks)Parse.Tests/AttributeTests.cs(1 hunks)Parse.Tests/CloudControllerTests.cs(4 hunks)Parse.Tests/CloudTests.cs(2 hunks)Parse.Tests/ConfigTests.cs(5 hunks)Parse.Tests/ConversionTests.cs(1 hunks)Parse.Tests/DecoderTests.cs(13 hunks)Parse.Tests/EncoderTests.cs(6 hunks)Parse.Tests/GeoDistanceTest.cs(1 hunks)Parse.Tests/GeoPointTests.cs(1 hunks)Parse.Tests/InternalExtensionsTests.cs(1 hunks)Parse.Tests/ObjectCoderTests.cs(2 hunks)Parse.Tests/ObjectControllerTests.cs(1 hunks)Parse.Tests/ObjectStateTests.cs(2 hunks)Parse.Tests/ObjectTests.cs(31 hunks)Parse.Tests/Parse.Tests.csproj(1 hunks)Parse.Tests/ParseOperationsTests.cs(1 hunks)Parse.Tests/ParseQueryControllerTests.cs(1 hunks)Parse.Tests/ParseQueryTests.cs(1 hunks)Parse.Tests/ProgressTests.cs(3 hunks)Parse.Tests/RelationTests.cs(1 hunks)Parse.Tests/TransientCacheControllerTests.cs(1 hunks)Parse.Tests/UserTests.cs(8 hunks)Parse/Abstractions/Infrastructure/CustomServiceHub.cs(2 hunks)Parse/Abstractions/Infrastructure/Data/IParseDataDecoder.cs(1 hunks)Parse/Abstractions/Infrastructure/IJsonConvertible.cs(1 hunks)Parse/Abstractions/Platform/Users/IParseCurrentUserController.cs(1 hunks)Parse/Infrastructure/Control/ParseAddOperation.cs(1 hunks)Parse/Infrastructure/Control/ParseAddUniqueOperation.cs(1 hunks)Parse/Infrastructure/Control/ParseDeleteOperation.cs(1 hunks)Parse/Infrastructure/Control/ParseFieldOperations.cs(2 hunks)Parse/Infrastructure/Control/ParseIncrementOperation.cs(2 hunks)Parse/Infrastructure/Control/ParseRelationOperation.cs(1 hunks)Parse/Infrastructure/Control/ParseRemoveOperation.cs(1 hunks)Parse/Infrastructure/Control/ParseSetOperation.cs(4 hunks)Parse/Infrastructure/Data/ParseDataDecoder.cs(2 hunks)Parse/Infrastructure/Data/ParseDataEncoder.cs(7 hunks)Parse/Infrastructure/Data/ParseObjectCoder.cs(3 hunks)Parse/Infrastructure/Execution/ParseCommandRunner.cs(2 hunks)Parse/Infrastructure/LateInitializedMutableServiceHub.cs(1 hunks)Parse/Infrastructure/MutableServiceHub.cs(2 hunks)Parse/Infrastructure/ParseFailureException.cs(2 hunks)Parse/Infrastructure/ServiceHub.cs(2 hunks)Parse/Platform/Cloud/ParseCloudCodeController.cs(1 hunks)Parse/Platform/Configuration/ParseConfiguration.cs(3 hunks)Parse/Platform/Files/ParseFile.cs(1 hunks)Parse/Platform/Location/ParseGeoPoint.cs(1 hunks)Parse/Platform/ParseClient.cs(2 hunks)Parse/Platform/Queries/ParseQuery.cs(3 hunks)Parse/Platform/Relations/ParseRelation.cs(1 hunks)Parse/Platform/Security/ParseACL.cs(1 hunks)Parse/Platform/Users/ParseUser.cs(1 hunks)Parse/Utilities/ParseQueryExtensions.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (10)
Parse/Infrastructure/Control/ParseAddUniqueOperation.cs (7)
Parse/Abstractions/Infrastructure/IJsonConvertible.cs (1)
IDictionary(15-15)Parse/Infrastructure/Control/ParseAddOperation.cs (1)
IDictionary(52-62)Parse/Infrastructure/Control/ParseDeleteOperation.cs (1)
IDictionary(21-27)Parse/Infrastructure/Control/ParseRemoveOperation.cs (1)
IDictionary(42-52)Parse/Infrastructure/Control/ParseSetOperation.cs (1)
IDictionary(20-43)Parse/Infrastructure/Control/ParseIncrementOperation.cs (1)
IDictionary(101-109)Parse/Infrastructure/Data/ParseObjectCoder.cs (1)
IDictionary(30-46)
Parse/Infrastructure/Control/ParseAddOperation.cs (7)
Parse/Abstractions/Infrastructure/IJsonConvertible.cs (1)
IDictionary(15-15)Parse/Infrastructure/Control/ParseDeleteOperation.cs (1)
IDictionary(21-27)Parse/Infrastructure/Control/ParseAddUniqueOperation.cs (1)
IDictionary(58-68)Parse/Infrastructure/Control/ParseRemoveOperation.cs (1)
IDictionary(42-52)Parse/Infrastructure/Control/ParseSetOperation.cs (1)
IDictionary(20-43)Parse/Infrastructure/Control/ParseIncrementOperation.cs (1)
IDictionary(101-109)Parse/Infrastructure/Data/ParseObjectCoder.cs (1)
IDictionary(30-46)
Parse/Infrastructure/LateInitializedMutableServiceHub.cs (1)
Parse/Infrastructure/Utilities/LateInitializer.cs (1)
LateInitializer(14-90)
Parse/Platform/Cloud/ParseCloudCodeController.cs (2)
Parse/Infrastructure/Data/ParseDataDecoder.cs (1)
Decode(23-63)Parse/Abstractions/Infrastructure/Data/IParseDataDecoder.cs (1)
Decode(14-14)
Parse/Abstractions/Infrastructure/IJsonConvertible.cs (1)
Parse/Platform/ParseClient.cs (1)
IServiceHub(171-182)
Parse/Infrastructure/Data/ParseObjectCoder.cs (2)
Parse/Infrastructure/Data/ParseDataDecoder.cs (1)
Decode(23-63)Parse/Abstractions/Infrastructure/Data/IParseDataDecoder.cs (1)
Decode(14-14)
Parse/Platform/Configuration/ParseConfiguration.cs (4)
Parse/Utilities/ConfigurationServiceExtensions.cs (2)
ParseConfiguration(12-15)ParseConfiguration(17-20)Parse/Infrastructure/Data/ParseDataDecoder.cs (1)
Decode(23-63)Parse/Abstractions/Infrastructure/Data/IParseDataDecoder.cs (1)
Decode(14-14)Parse/Abstractions/Infrastructure/IJsonConvertible.cs (1)
IDictionary(15-15)
Parse/Infrastructure/Data/ParseDataEncoder.cs (3)
Parse/Platform/ParseClient.cs (6)
IServiceHub(171-182)ParseClient(21-183)ParseClient(60-60)ParseClient(68-122)ParseClient(127-127)IDictionary(148-159)Parse/Infrastructure/Control/ParseSetOperation.cs (4)
ParseSetOperation(12-75)ParseSetOperation(14-17)ConvertValueToJSON(56-72)IDictionary(20-43)Parse/Abstractions/Infrastructure/IJsonConvertible.cs (1)
IDictionary(15-15)
Parse/Infrastructure/Execution/ParseCommandRunner.cs (1)
Parse/Infrastructure/ParseFailureException.cs (3)
ParseErrorPayload(274-278)ParseFailureException(8-273)ParseFailureException(267-267)
Parse.Tests/UserTests.cs (5)
Parse/Platform/ParseClient.cs (1)
Publicize(133-139)Parse/Utilities/ObjectServiceExtensions.cs (1)
AddValidClass(25-28)Parse/Platform/Objects/ParseObject.cs (1)
HandleFetchResult(801-807)Parse/Infrastructure/MutableServiceHub.cs (2)
MutableServiceHub(35-109)MutableServiceHub(69-108)Parse/Platform/Users/ParseUser.cs (12)
Task(15-37)Task(61-65)Task(88-120)Task(123-136)Task(138-149)Task(151-163)Task(165-170)Task(209-218)Task(220-228)Task(230-233)Remove(39-46)HandleSave(50-56)
⏰ 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). (2)
- GitHub Check: .NET 9.0
- GitHub Check: .NET 8.0
🔇 Additional comments (100)
Parse.Tests/Parse.Tests.csproj (1)
9-17: Good package maintenance and coverage enhancementThe addition of
coverlet.collectorfor code coverage and updates to the test SDK and MSTest packages are excellent improvements that will enhance the testing capabilities and provide better coverage reporting for this architectural refactor.Parse.Tests/ObjectTests.cs (2)
32-32: Minor formatting improvementGood fix to the indentation of the private
Clientproperty for consistency.
735-1062: Excellent test coverage expansionThe addition of numerous well-structured unit tests significantly improves coverage of the ParseObject functionality. The tests are properly isolated, have clear descriptions, and cover important edge cases and functionality like property validation, operations, and state management.
Parse.Tests/ProgressTests.cs (2)
35-46: Excellent test isolation improvementConverting from shared mock fields to per-test local mocks eliminates potential test interdependencies and makes tests more reliable and maintainable. This follows testing best practices.
52-63: Consistent test isolation patternGood consistency in applying the same local mock pattern across both test methods, ensuring uniform test isolation throughout the class.
Parse.Tests/ConversionTests.cs (1)
27-30: Improved test precision with exact exception matchingUsing
Assert.ThrowsExactly<InvalidCastException>instead ofAssert.ThrowsExceptionprovides more precise exception validation, ensuring the exact exception type is thrown rather than allowing derived types.Parse/Platform/Users/ParseUser.cs (1)
6-6: Good code cleanup and formattingThe addition of appropriate whitespace and removal of commented-out code (as noted in the summary) improves code readability and maintainability.
Parse/Abstractions/Infrastructure/CustomServiceHub.cs (1)
50-50: LGTM! Consistent service hub pattern.The new
CurrentUserproperty follows the established pattern of exposing controller functionality through the service hub abstraction. This provides convenient direct access to the current user while maintaining the architecture's consistency.Parse/Abstractions/Platform/Users/IParseCurrentUserController.cs (1)
10-10: All implementations now include theCurrentUserproperty
IParseCurrentUserControllerdeclaresParseUser CurrentUser { get; }.ParseCurrentUserControllerprovides a public getter (get => currentUser) and satisfies the interface.- No other concrete implementations of the interface exist (tests use mocks).
No further changes are required.
Parse/Platform/Security/ParseACL.cs (1)
108-108: Excellent type safety improvement.Changing the return type from
objecttoIDictionary<string, object>improves type safety and API clarity. Since the method implementation already returns a dictionary, this change makes the contract explicit and consistent with the broader standardization effort across the codebase.Parse.Tests/GeoPointTests.cs (1)
27-27: Good API simplification.Removing the second parameter from
Client.Decoder.Decodealigns with the dependency injection refactoring where the decoder now maintains an internal service hub reference. This reduces parameter complexity while maintaining the same functionality.Parse/Platform/Location/ParseGeoPoint.cs (1)
94-94: Consistent type safety improvement.The return type change from
objecttoIDictionary<string, object>maintains consistency with the broader refactoring pattern and improves type safety without changing the functional behavior.Parse/Abstractions/Infrastructure/Data/IParseDataDecoder.cs (1)
14-14: LGTM! Interface simplification improves dependency injection pattern.The removal of the
IServiceHub serviceHubparameter from theDecodemethod signature is a good architectural improvement. This change supports constructor-based dependency injection where the decoder receives the service hub instance at creation time rather than on each method call, leading to cleaner and more consistent API design.Parse/Infrastructure/LateInitializedMutableServiceHub.cs (1)
151-155: Consistent with dependency injection refactoring.The change to pass the entire service hub instance (
this) to theParseDataDecoderconstructor instead of just theClassControlleris consistent with the interface simplification inIParseDataDecoder. This allows the decoder to access all necessary services through constructor injection rather than requiring them as method parameters.Parse/Infrastructure/Control/ParseAddOperation.cs (1)
52-52: Excellent type safety improvement.Changing the return type from
objecttoIDictionary<string, object>provides better type safety and clarity about what the method returns. This standardization across Parse operation classes makes the API more predictable and reduces the need for casting in consuming code.Parse/Infrastructure/Control/ParseAddUniqueOperation.cs (1)
58-58: Consistent type safety improvement across operation classes.The return type change from
objecttoIDictionary<string, object>aligns with similar changes in other Parse operation classes likeParseAddOperation. This standardization improves type safety and API consistency across the codebase.Parse/Infrastructure/ServiceHub.cs (2)
49-49: Good architectural change to support explicit decoder initialization.Changing the
Decoderproperty from lazy initialization to an auto-implemented property with an internal setter is a positive architectural improvement. This allows explicit control over when and how the decoder is initialized, which supports the dependency injection pattern where the decoder can be set during service hub construction rather than being created on first access.
2-2: Using System directive is required.The file uses
Lazy<T>(e.g.new Lazy<IParseUserController>(…)), which resides in theSystemnamespace. Theusing System;import is therefore needed.Parse/Infrastructure/MutableServiceHub.cs (2)
2-2: Minor formatting improvement.Good addition of the blank line for better code organization and readability.
83-83: Excellent dependency injection refactor.The change from passing
ClassControllerto passing the entire service hub (this) to theParseDataDecoderconstructor is a solid architectural improvement. This allows the decoder to access all necessary services through the injected hub rather than requiring specific dependencies or runtime parameters.This change aligns well with the broader refactoring mentioned in the summary and follows good dependency injection principles.
Parse/Infrastructure/ParseFailureException.cs (1)
274-278: Well-designed error payload structure.The
ParseErrorPayloadclass provides a clean, structured way to handle error responses from the Parse server. The simple propertiescodeanderroralign well with typical REST API error formats and integrate nicely with the enhanced HTTP response handling mentioned in the summary.Parse/Infrastructure/Control/ParseDeleteOperation.cs (1)
21-21: Excellent type safety improvement.Changing the return type from
objecttoIDictionary<string, object>is a great enhancement that:
- Provides explicit type information for better code clarity
- Improves type safety and IntelliSense support
- Aligns with the broader standardization effort across Parse operation classes
- Makes the JSON-like return structure immediately apparent to consumers
The method already returned a dictionary, so this change just makes the contract explicit without any functional changes.
Parse/Platform/Files/ParseFile.cs (1)
168-168: Excellent type safety enhancement.The return type change from
objecttoIDictionary<string, object>is a valuable improvement that:
- Explicitly declares the JSON-like dictionary structure being returned
- Enhances type safety and developer experience with better IntelliSense
- Aligns perfectly with the systematic standardization across the Parse SDK
- Maintains full backward compatibility since the implementation was already returning a dictionary
This change makes the API more self-documenting and easier to work with.
Parse/Infrastructure/Control/ParseIncrementOperation.cs (2)
4-4: Good formatting improvement.The added blank line enhances code readability and organization.
101-101: Perfect type safety enhancement.The return type change from
objecttoIDictionary<string, object>continues the excellent pattern of improvements seen across the Parse operation classes. This change:
- Makes the JSON dictionary return structure explicit and type-safe
- Improves developer experience with better tooling support
- Maintains consistency with the broader SDK standardization effort
- Preserves all existing functionality while enhancing the API contract
This systematic approach to type refinement across the operation classes is commendable.
Parse/Abstractions/Infrastructure/IJsonConvertible.cs (1)
14-16: Good improvement to type safetyChanging the return type from
objecttoIDictionary<string, object>provides better compile-time type checking and makes the API contract clearer. This aligns well with the architectural refactoring objectives.Parse/Infrastructure/Control/ParseRemoveOperation.cs (1)
42-52: Return type correctly updated to match interfaceThe return type change to
IDictionary<string, object>properly implements the updatedIJsonConvertibleinterface. The implementation already returned a dictionary, so no logic changes were needed.Parse/Platform/Relations/ParseRelation.cs (1)
73-80: Consistent implementation of updated interfaceThe return type change properly implements the updated
IJsonConvertibleinterface. The method already returned the correct dictionary structure.Parse/Utilities/ParseQueryExtensions.cs (1)
7-7: Minor whitespace changeThis appears to be just a whitespace/formatting change with no functional impact.
Parse/Infrastructure/Control/ParseRelationOperation.cs (1)
85-85: LGTM: Improved type safety with specific return type.The change from
objecttoIDictionary<string, object>for theConvertToJSONmethod return type enhances type safety and makes the interface contract clearer. The implementation already returns dictionary objects, so this change aligns perfectly with the architectural refactoring goals.Parse/Infrastructure/Data/ParseObjectCoder.cs (3)
91-91: LGTM: Simplified decoder call aligns with interface changes.The removal of the
serviceHubparameter fromdecoder.Decode()correctly reflects the updatedIParseDataDecoderinterface where service hub access is now internal to the decoder implementation.
117-124: LGTM: Improved Extract method implementation.The refactored
Extract<T>method now properly checks for key existence before attempting removal and conversion, which is more robust than the previous implementation.
129-135: LGTM: Added braces improve code clarity.The addition of braces to the
PopulateServerDatamethod enhances readability without changing behavior.Parse.Tests/EncoderTests.cs (3)
34-34: LGTM: Consistent client context provision.Adding the
Clientparameter toParseObjectconstructors ensures proper client context, which aligns with the architectural refactoring goals.Also applies to: 52-52
53-55: LGTM: Improved exception testing precision.The change from
Assert.ThrowsExceptiontoAssert.ThrowsExactlyenforces exact exception type matching, which provides more precise test validation and better catches regressions.Also applies to: 106-106, 250-253
79-85: LGTM: Added test coverage for NoObjectsEncoder.The new test method
TestEncodeParseObjectWithNoObjectsEncoderprovides valuable coverage for the NoObjectsEncoder scenario, ensuring it properly throwsArgumentExceptionwhen attempting to encode a ParseObject.Parse/Platform/ParseClient.cs (1)
100-119: LGTM: Excellent decoder initialization architecture.The
FinalizeDecoderlocal method provides a clean, recursive solution for ensuring all service hubs in the hierarchy have theirDecoderproperty properly initialized. The implementation correctly:
- Handles nested
OrchestrationServiceHubstructures recursively- Prevents overwriting existing decoders with null checks
- Covers both
ServiceHubandMutableServiceHubinstances- Aligns with the architectural refactoring goals by centralizing decoder initialization
This ensures consistent decoder availability across the entire service hierarchy.
Parse.Tests/ConfigTests.cs (3)
69-69: LGTM: Simplified test comments improve readability.The removal of difficulty level indicators from test comments makes the code cleaner while maintaining the descriptive nature of the test methods.
Also applies to: 79-79, 93-93, 103-103
118-125: LGTM: Activated test improves error handling coverage.The
Get_ThrowsExceptionNotFoundtest provides valuable coverage for the error case when accessing non-existent configuration keys, ensuring properKeyNotFoundExceptionbehavior.
138-138: LGTM: Mock setup aligns with simplified decoder interface.The removal of the second parameter from the
mockDecoder.Setup()call correctly reflects the updatedIParseDataDecoder.Decodemethod signature that no longer requires a service hub parameter.Parse/Platform/Queries/ParseQuery.cs (2)
814-836: Well-implemented staticOrmethod with proper validation.The implementation correctly validates input queries, ensures they target the same class, and constructs the
$orclause appropriately. The error messages are clear and helpful.
937-942: ProperGetHashCodeimplementation using standard hash combining technique.The implementation correctly combines hash codes from key properties using the standard multiplier approach. This complements the existing
Equalsmethod well.Parse/Infrastructure/Control/ParseSetOperation.cs (1)
20-43: Good type safety improvement but verify primitive type handling logic.The return type change to
IDictionary<string, object>improves type safety. However, the logic for primitive types (lines 30-33) wraps them in a dictionary with a "value" key, which may not be the intended behavior for set operations.Consider whether primitive values should be wrapped in a dictionary or handled differently for set operations.
Parse.Tests/ObjectControllerTests.cs (1)
21-30: Excellent test isolation improvements.The addition of proper
ParseClientinitialization inSetUpand cleanup inTearDownsignificantly improves test isolation and prevents potential test interference. TheClient.Publicize()call ensures the client is properly initialized for testing.Parse.Tests/ObjectStateTests.cs (2)
5-6: Good addition of necessary using statements.The
MoqandParse.Abstractions.Infrastructureusing statements are appropriately added to support the new test functionality with mocking.
168-305: Comprehensive test coverage expansion with good edge case handling.The new test methods provide excellent coverage of edge cases:
- Null value handling in cloning
- Exception resilience in mutation operations
- Dictionary decoding with date/time parsing
- Graceful handling of invalid data
- Field operation exception handling
- State copying verification
The tests are well-structured with clear descriptions and appropriate assertions.
Parse.Tests/CloudControllerTests.cs (1)
24-24: Excellent test isolation improvements.The refactoring from shared mock fields to local mock creation using the
CreateMockRunnerhelper method significantly improves test isolation and maintainability. Each test now has its own clean mock setup without risk of shared state interference.Also applies to: 140-151
Parse.Tests/TransientCacheControllerTests.cs (2)
36-50: Test logic is correct and follows good patterns.The removal test properly validates that items are removed from the cache and uses appropriate assertions.
72-76: Exception test is correctly implemented.The test properly validates that
GetRelativeFilethrows the expectedNotSupportedException.Parse.Tests/GeoDistanceTest.cs (5)
8-8: Modern C# syntax usage is good.Using file-scoped namespaces is a good practice for test files.
14-19: Basic constructor test is correct.The test properly validates that the constructor sets the radians value.
43-49: Factory method test logic is correct.The test properly validates the
FromMilesstatic factory method.
53-59: Factory method test logic is correct.The test properly validates the
FromKilometersstatic factory method.
64-69: Factory method test logic is correct.The test properly validates the
FromRadiansstatic factory method.Parse.Tests/AttributeTests.cs (4)
19-28: Property test logic is correct.The test properly validates that PreserveAttribute boolean properties can be set and retrieved correctly.
31-35: Basic instantiation test is appropriate.Simple test to ensure LinkerSafeAttribute can be created without exceptions.
38-43: Reflection-based attribute validation is correct.The test properly uses reflection to verify that PreserveWrapperTypes has the expected PreserveAttribute with AllMembers=true.
47-58: AOT type registration test is comprehensive.The test validates that specific wrapper types are registered in the AOTPreservations list, which is important for AOT compilation scenarios.
Parse.Tests/CloudTests.cs (5)
27-65: Mock setup and test logic are correct.The refactoring successfully eliminates shared state by creating local mocks. The test properly validates that missing "result" key throws ParseFailureException.
Key improvements:
- Local mock instantiation improves test isolation
- Simplified Decode mock setup aligns with interface changes
- Proper exception testing
41-42: Fully qualified HttpStatusCode usage is consistent.Using
System.Net.HttpStatusCodeinstead of a using directive is consistent with the refactoring pattern.
49-51: Simplified Decode mock setup is correct.The change from two parameters to one parameter in the Decode method mock aligns with the interface refactoring mentioned in the PR summary.
68-105: Second test method follows same good patterns.The second test method uses the same improved patterns as the first, with local mock setup and proper exception testing.
22-25: Post-Test Cleanup Verification
We didn’t find any static fields or singleton references toParseCorePluginsin this repo, so it’s unclear whetherReset()is necessary. Please confirm:
- If
ParseCorePlugins.Instanceretains global state that could leak between tests, re-enable the cleanup:-#warning Skipped post-test-evaluation cleaning method may be needed. +[TestCleanup] +public void TearDown() => ParseCorePlugins.Instance.Reset();- Otherwise, remove the
#warningand the commented-out method entirely to avoid confusion.Parse.Tests/AnalyticsTests.cs (3)
22-52: Test refactoring and logic are correct.The refactoring successfully moves from shared state to local mock instantiation. The test properly validates that
TrackAnalyticsEventAsynccalls the controller with expected parameters.
55-87: Test with dimensions is well-implemented.The test properly validates analytics tracking with custom dimensions, including verification that the dimensions dictionary is correctly passed to the controller.
90-119: App launch tracking test is correct.The test validates the
TrackLaunchAsyncfunctionality, properly checking thatTrackAppOpenedAsyncis called with null push hash.Parse/Platform/Configuration/ParseConfiguration.cs (2)
28-28: LGTM! Decoder interface simplificationThe removal of the
serviceHubparameter fromdecoder.Decodealigns with the architectural refactor to simplify the decoder interface.
110-110: Good type safety improvementChanging the return type from
objecttoIDictionary<string, object>provides better type safety and clarity about what the method returns.Parse/Infrastructure/Execution/ParseCommandRunner.cs (1)
92-111: Well-structured error handling for 400 Bad RequestGood implementation of JSON deserialization for error payloads with proper exception handling when deserialization fails.
Parse.Tests/RelationTests.cs (1)
11-27: Significant reduction in test coverageAccording to the AI summary, this file previously contained extensive tests for user relation management, exception handling, and other relation-related functionality. The removal of these tests significantly reduces coverage.
Please verify that the removed relation management tests have been moved to another test file or are covered elsewhere in the test suite. If not, consider restoring these tests to maintain adequate test coverage.
Parse.Tests/DecoderTests.cs (2)
43-142: Excellent test coverage for field operationsThe new test methods provide comprehensive coverage for decoding various field operations (Increment, Delete, Add, AddUnique, Remove). The tests are well-structured with clear Arrange-Act-Assert patterns and descriptive assertions.
22-22: Consistent decoder interface simplificationThe removal of the
Clientparameter from alldecoder.Decodecalls is consistent with the architectural refactor to simplify the decoder interface.Also applies to: 36-41
Parse/Infrastructure/Control/ParseFieldOperations.cs (2)
37-38: Good improvement to comparer initializationMaking the comparer field
readonlyand properly initializing it ensures thread safety and prevents accidental modifications.
40-105: Well-structured field operation decoding implementationThe new
Decodemethod provides comprehensive support for all Parse field operations with proper recursive decoding of nested objects. The implementation is clean, uses modern C# features effectively, and has appropriate error handling for unsupported operations.Parse.Tests/ParseQueryControllerTests.cs (4)
34-47: Good dependency injection setup with proper service hub initialization.The test setup correctly uses
MutableServiceHubwithSetDefaults()to ensure all dependencies are properly initialized, and creates a test-configuredParseClient. This pattern aligns well with the architectural refactoring objectives.
49-75: Comprehensive test for FindAsync with proper response structure.The test correctly mocks the server response structure with the expected "results" array containing Parse objects with proper type annotations. The assertions validate both the count and object properties appropriately.
77-95: Well-structured CountAsync test with correct response format.The test properly mocks the expected server response format for count queries and validates the returned count value.
97-119: FirstAsync test validates correct object extraction.The test ensures that the first object is properly extracted and decoded from the results array, with appropriate assertions on the returned object's properties.
Parse.Tests/ObjectCoderTests.cs (1)
84-105: Well-structured async delete test with proper verification.The test correctly mocks the command runner and verifies that the delete operation calls
RunCommandAsyncexactly once. The setup and assertions are appropriate for testing the delete functionality.Parse/Infrastructure/Data/ParseDataDecoder.cs (3)
16-19: Excellent dependency injection implementation with proper validation.The refactoring to inject
IServiceHubdirectly into the decoder centralizes service access and eliminates the need to pass it as a parameter to decode methods. The null check withArgumentNullExceptionensures proper validation.
23-62: Well-structured switch expression with comprehensive type handling.The switch expression pattern provides clear, readable logic for handling different Parse data types. The explicit null handling and consistent use of
KeyNotFoundExceptionfor missing required fields improves error clarity. The recursive decoding for dictionaries and lists maintains the decoding tree properly.
65-66: DecodePointer method correctly uses injected services.The method properly uses the internal
Servicesproperty instead of the external parameter, maintaining consistency with the refactored architecture.Parse.Tests/InternalExtensionsTests.cs (7)
16-24: Good null safety test for Safe() extension method.The test correctly validates that calling
Safe()on a null task returns a completed task, which is essential for preventing null reference exceptions in async workflows.
26-35: Comprehensive test for generic Safe() method.The test properly validates that
Safe<T>()returns a completed task with the default value for the type when called on a null task, ensuring type safety in async operations.
37-53: Thorough dictionary extension testing with both positive and negative cases.Both
GetOrDefaulttests properly validate the expected behavior when keys exist and don't exist, ensuring the extension method works correctly for dictionary lookups.
55-71: Collection equality tests cover both equal and unequal scenarios.The tests correctly validate that
CollectionsEqualdistinguishes between collections with the same elements in the same order versus different orders, which is the expected behavior for sequence equality.
89-104: Excellent WhileAsync test with predicate-controlled loop.The test properly validates that the
WhileAsyncmethod executes the loop body the correct number of times based on the predicate condition, ensuring the async looping utility works as expected.
140-153: Good exception propagation test for faulted tasks.The test correctly validates that
OnSuccesspropagates exceptions from faulted tasks, which is crucial for proper error handling in async continuation chains.
155-167: Proper cancellation handling test.The test appropriately validates that canceled tasks are handled correctly by the
OnSuccessextension, ensuring proper cancellation token propagation.Parse.Tests/ACLTests.cs (8)
19-52: Well-structured test setup with comprehensive mocking.The test initialization properly mocks the service hub and class controller, with appropriate setup for object instantiation. The cleanup method ensures proper test isolation.
58-64: Simple but effective default constructor test.The test validates basic ACL instantiation, which is fundamental for ensuring the class can be created without errors.
66-81: Comprehensive ParseUser constructor test.The test properly validates that creating an ACL with a ParseUser owner sets the appropriate read and write permissions for that user, testing both ObjectId-based and ParseUser-based access methods.
145-157: Good public access property testing.The test validates both getter and setter behavior for public read/write access properties, ensuring the basic public access functionality works correctly.
178-189: Excellent null input validation testing.The test properly validates that ACL methods throw
ArgumentExceptionwhen passed null user IDs, which is essential for preventing runtime errors and ensuring proper input validation.
220-234: Comprehensive role access testing.The test thoroughly validates role-based read/write access functionality, including both setting and getting permissions, which is crucial for role-based security models.
236-254: Good JSON conversion test with proper type validation.The test validates that ACL JSON conversion returns the expected
IDictionary<string, object>type and contains the appropriate keys for users and roles, aligning with the standardized JSON conversion return types mentioned in the AI summary.
257-277: Excellent edge case testing for invalid ACL data.Both tests properly validate that the ACL handles invalid data gracefully, including invalid public key values and non-dictionary user permission data. This ensures robustness against malformed input data.
Parse.Tests/ParseOperationsTests.cs (1)
226-234: Good test coverage for edge casesThe test properly validates that incrementing non-numeric values throws an exception.
Parse.Tests/UserTests.cs (2)
148-205: Excellent test refactoring with proper isolationThe refactored
TestLogOutmethod demonstrates best practices:
- Complete isolation from static state
- Clear mock setup and verification
- Proper assertion of both the command execution and local cache clearing
This is a significant improvement over the previous implementation.
378-539: Good test coverage for edge casesThe new tests provide excellent coverage for:
- Missing credentials validation
- Authentication state checks
- Username removal protection
- New user save restrictions
- Auth data linking checks
- Password removal verification
These tests strengthen the overall test suite quality.
| public object ConvertValueToJSON(IServiceHub serviceHub = null) | ||
| { | ||
| // Get the values of the dictionary | ||
| var vals = ConvertToJSON(serviceHub).Values; | ||
|
|
||
|
|
||
|
|
||
| // Check if vals is a ValueCollection and contains exactly one element , that's how we get operations working! because they are dict<string,obj> of dict<string,obj> | ||
| if (vals.Count == 1) | ||
| { | ||
| // Return the first and only value | ||
| return vals.FirstOrDefault(); | ||
| } | ||
|
|
||
| // Return vals if no single value is found | ||
| return vals; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Questionable method design and implementation.
The ConvertValueToJSON method has several concerns:
- The method name is misleading - it's unwrapping dictionary values, not converting to JSON
- The comment on line 63 is verbose and unclear
- The logic assumes the dictionary always has exactly one value, which may not be reliable
- The return type is
object, losing type safety
Consider renaming this method and clarifying its purpose, or removing it if it's not essential to the API.
🤖 Prompt for AI Agents
In Parse/Infrastructure/Control/ParseSetOperation.cs around lines 56 to 72, the
ConvertValueToJSON method is misleadingly named and has unclear comments,
unreliable assumptions about dictionary size, and returns a loosely typed
object. Rename the method to better reflect its purpose, such as
ExtractSingleValueOrValues, simplify and clarify the comments to explain the
logic clearly, add checks to handle cases where the dictionary has zero or
multiple values more robustly, and consider using a more specific return type or
generics to improve type safety. If this method is not essential, evaluate
removing it to keep the API clean.
| // Decode the result | ||
| var decoded = Decoder.Decode(commandResult.Item2, serviceHub) as IDictionary<string, object>; | ||
|
|
||
| if (decoded == null) | ||
| { | ||
| throw new ParseFailureException(ParseFailureException.ErrorCode.OtherCause, "Failed to decode cloud function response."); | ||
| } | ||
| var decoded = Decoder.Decode(commandResult.Item2) as IDictionary<string, object>; | ||
|
|
||
| // Extract the result key | ||
| if (decoded.TryGetValue("result", out var result)) |
There was a problem hiding this comment.
Add null check after the cast to prevent potential NullReferenceException
The cast to IDictionary<string, object> on line 56 could return null if the decoded object is not a dictionary. The subsequent TryGetValue call on line 59 would throw a NullReferenceException if decoded is null.
Consider adding a null check:
// Decode the result
var decoded = Decoder.Decode(commandResult.Item2) as IDictionary<string, object>;
+if (decoded == null)
+{
+ throw new ParseFailureException(ParseFailureException.ErrorCode.OtherCause, "Cloud function returned invalid response format.");
+}
// Extract the result key
if (decoded.TryGetValue("result", out var result))🤖 Prompt for AI Agents
In Parse/Platform/Cloud/ParseCloudCodeController.cs around lines 55 to 59, add a
null check after casting the decoded object to IDictionary<string, object> to
prevent a NullReferenceException. Specifically, verify that the decoded variable
is not null before calling TryGetValue on it. If decoded is null, handle that
case appropriately to avoid exceptions.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #412 +/- ##
==========================================
+ Coverage 48.20% 54.53% +6.33%
==========================================
Files 106 106
Lines 6199 6273 +74
Branches 950 981 +31
==========================================
+ Hits 2988 3421 +433
+ Misses 2907 2523 -384
- Partials 304 329 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@YBTopaz8 First of all, thanks for your efforts with this PR. If you remember the discussion from the last redesign, it was incredibly complex to review because many changes were packed into a single PR. Is there a way to break this down into smaller, discrete PRs, also to minimize the risk of bugs? We also try to avoid breaking changes as much as possible. |
Removed unnecessary checks for response codes 100 and 200. Updated handling for response code 401 to throw `InvalidSessionToken` instead of `ObjectNotFound`. Retained structure for response code 400 with added deserialization of `ParseErrorPayload`.
|
Thank you for the feedback. I need to be clear that this changeset is a single, atomic architectural upgrade. The core of this work is a critical bug fix to the DI system, and all subsequent features and tests are built directly upon that new, stable foundation. As such, these changes cannot be broken up into smaller, preceding PRs unless one has time for that —the dependency is foundational. This refactor is not a redesign for its own sake; it's a necessary stabilization of the previous architecture that simplifies the developer experience and eliminates a class of runtime bugs, as detailed in the PR description. I understand your team's process, but it's also difficult to provide the necessary documentation and context when my foundational README PR from last March still hasn't been reviewed. A comprehensive review of this PR would depend on that context. This contribution is offered as a complete, necessary solution. Please let me know if you are able to review and accept this foundational contribution as a whole. |
|
@YBTopaz8 #410 seems to be just a few lines in the README. If a PR falls through the cracks please comment as reminder. I posted a comment there.
Could you break this down briefly - what are the improvements and new features? We'd need a list of all breaking changes for the changelog entry. |
PR Classification:
Major architectural improvement, new features, and increased test coverage.
PR Summary:
This is a major stability and quality-of-life release that overhauls the internal architecture of the SDK, making it more reliable and easier to use. It also lays the critical groundwork for future features like the Parse Local Datastore (LDS)!
BREAKING CHANGE⚠️
If you have created a custom implementation of the IParseDataDecoder interface, its Decode method signature has changed. It no longer requires the IServiceHub parameter. Please update your implementation to match the new interface. This change was made to eliminate a common source of NullReferenceExceptions.
New Features & Improvements
Easier Access to the Current User!
You can now get the current user with a simple shortcut:
Foundation for Offline Storage (LDS): The SDK can now fully understand all server-side data operations (Increment, AddUnique, etc.). This is the first and most important step toward enabling full offline data caching and synchronization. Stay tuned!
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Documentation
Style
These updates enhance reliability, maintainability, and test coverage across the platform.