-
-
Notifications
You must be signed in to change notification settings - Fork 261
Finish Refactor, Make Configurable Controller Pipeline, and Make ParseClients Dependency-Injectable #329
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
Conversation
…ems into core dependency injection container.
…to allow tests to pass, and apply minor refactorings.
…paces until it is converted into a generalized DI container or one is made.
…nstead of the old plugins system, then make the main library compile. Also, restructure metadata injection, and inject revocable session token information as well as all server connection data, as opposed to having any of it statically defined or stored in the ParseClient class.
…ontainer interface IServiceHub, and rewrite tests to account for the change.
…p residual and/or unneeded files and systems.
…ub implementation instance passed to ParseClient constructor as BuildHub call target if the implementation also implements IMutableServiceHub, make sure IMutableServiceHub-implementing IServiceHub implementations passed to ParseClient constructor are passed the given IServerConnectionData implementation instance if they have null ServerConnectionData properties, allow values to be set in LateInitializer, change LateInitializer to be an internal class, add tests for LateInitializer, rename Parse.Test to Parse.Tests, update appveyor build script to reflect Parse.Test rename and Visual Studio target version bump, add various syntax adjustments to make code more idiomatic, remove ParsePushModule.cs, and fix a race condition in ParseCommandRunner.cs.
…method, and update documentation of non-cloning ParseClient constructors and LogInAsync IServiceHub extension method.
…acheController, add IDiskFileCacheController, rename ICacheLocationConfiguration and related items to derivative names of IRelativeCacheLocationGenerator, rename various items within renamed IStorageController, allow IDiskFileCacheController to regenerate tracked file wrappers and memory cache when requested storage paths change, make CacheController not auto-create files until needed, rename CacheLocationMutator to RelativeCacheLocationMutato, make AbsoluteCachePathMutator for changing the absolute cache path from a file in subfolder of LocalApplicationData or equivalent on other platforms, document various items, rename ParseClient.Configuration to ServerConnectionData and remove it to use IServiceHub-inherited member, rename IStorageDictionary to IDataCache, rename VirtualStorageDictionary to FileBackedCache, remove Parse.Abstractions.Storage and Parse.Abstractions.Internal namespaces, and update README.md to reflect changes.
… VirtualStorageDictionary to VirtualDataStore.
…TransientCacheController, and rename VirtualDataStore to VirtualCache.
…Object.Services publicly settable.
|
Here are the use case demonstrations. |
|
@TobiasPott can you please test these, and review the changes? |
|
@TheFanatr Hi Alex, you're welcome and yes as soon as I find the time to give them a test I'll do so. |
|
@TobiasPott have you had a chance to look at this yet? I’ve now noticed that I made some changes to the sample projects in my other repo without testing them, so I will fix that, but the Unity sample should still work. This is a fairly significant set of changes to the current master branch, so I’d appreciate feedback on if it worked for you and what you think about the changes I’m proposing. |
|
@TheFanatr unfortunately I did not have the time yet to take a look at it but I have it on my schedule (had it for this week) but will be able to start today (if my plans don't get busted again). I'm a bit confused what I should look at now, after you mentioned that you have fix changes in sample projects, should I take a look only on the Unity one or on the others too? |
|
@TobiasPott Try them all, but the sample I made with Blazor may not work because I removed a workaround for something and didn’t check if it still worked. |
|
@TheFanatr Okay, I did go through the Console and Unity example (I was slightly amazed that you used the UIElements for runtime stuff). One question I came up with a second ago. Did you run the Unity example on any target devices (Android, iOS, Windows), I only tried in-editor and would do it for iOS, Android and Windows Standalone. |
Yeah, I was going to use the built-in UI but then I remembered how much of a pain it was last time I did, and UIElements was basically perfect for my use case so all I had to do was find a renderer and figure out how to hack it to do what I want.
I only tested it in the editor and a build for Windows Standalone. I have multiple iOS devices, so I could look into testing it on one of them, but I’ve never done it before and don’t have a macOS device; if I’m required to have one, I won’t be able to do it because I don’t really want to set up a whole VM just for this unless there’s no other option and it needs to be done for this PR to get merged. |
|
Also just to note, a lot of the commits make it seem like some important files were outright deleted but really their code was either just siphoned out and put somewhere else, or they were just moved. A lot of them were at least partially rewritten too. |
|
Okay, it is good to know that you did try it on windows standalone. I'll keep it in mind to not panic when I check the commits of your PR. |
|
@TheFanatr
For all of those platforms it is necessary to use mutators to replace the default implementation and provide valid metadata and valid caching paths. TL;DR: I'm done with the review, will resolve the merge conflict and merge your pull request today and prepare the additional information for the above mentioned platforms. Thank you very, very much for your work and effort and contribution to this project. =) |
This does what said I would do at the end of one of my comments on #294, namely that
ParseClientis now instantiable, but optionally usable as a singleton, with the ability to configure a full service pipeline for each instance, replacing the previous plugin system with a monolithicIServiceHubwhich contains all controllers needed for the SDK to operate. This allows the SDK operations to be fully customized, with any internal component swappable with a custom implementation, to change the way the SDK works, while also allowing use cases such as multiple concurrent users for use on servers, as well as connections to multiple Parse Server instances at once. This also theoretically makes it a lot more easy for users to fix platform-specific issues in a fairly straightforward manner by simply placing a new controller implementation into theIMutableServiceHubexposed to aIServiceHubMutatorthat they implement, which can be used by simply adding it toParseClientconstructor calls, and can be redistributed as one file. Using this pipeline, having theParseClientconstructor manually configure certain services which are deemed problematic based on some given types is no longer necessary, as providedIServiceHubMutatorimplementations can now perform such tasks on request, in a highly-customizeable fashion. The solution to make Unity and some other platforms work, namely via configuringParseClient.Configurationand providing anIStorageConfiguration, is now replaced with components that can perform these actions separately, on demand; specifically, via the providedAbsoluteCacheLocationMutator,MetadataMutator, and/orRelativeCacheLocationMutator. This was tested with the latest beta build of Unity with the default player settings, as well as a Blazor server project with multiple clients connected concurrently, with ACL support. These demonstration projects will be released at some point in the near future. The instructions for how to use the SDK in this form are in the updated README.md.I have also finished reorganizing the folder structure into one that makes much more sense, and will help people find files more easily. From this, the project can be modularized out much more easily, and the beginnings of an extracted abstractions library, under the Abstractions folder, can also be found. I may add the code for configuring the problematic-on-cross-platform controllers manually to a monolithic
IServiceHubMutatorimplementation that comes with the SDK so it's easier for people to troubleshoot, but it may just be simpler for platform-specific libraries with such configuration mutators be released on NuGet, or in Unity's case as a file via the package manager.To be clear, the part in the updated README.md under the "Use in Unity Clients" section, can be applied to any other platform where automatic value generation for metadata items via reflection, or some other bug, has caused initialization to fail on a given platform.