EasyPost
Thread-safe rearchitecture
Description
- Services (static functions) called via a property of a Client object
- Non-static functions called via an instance of an object
- Files re-organized for better readability
- All unit tests reworked to account for new structure, all based on base UnitTest (using XUnit rather than MSTest)
Closes #98
Testing
- All unit tests pass
- All cassettes re-recorded
Pull Request Type
Please select the option(s) that are relevant to this PR.
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Improvement (fixing a typo, updating readme, renaming a variable name, etc)
This PR needs to be put up against the v4 branch, not master. The v4 branch may also need to be updated as I created it a couple weeks ago.
Good news, in my journey to fix an odd bug for the .NET Standard build process on CI, all unit tests now take 1-2 seconds to run. Of course, there's still the several-minute process of building everything prior to running unit tests, but hey, progress.
@Justintime50 This is ready for review, pending your decision on how to handle beta features.
NOTE: We'll need to move EndShipper to GA either in this PR or a future PR, once it's enabled server-side.
I'm also not seeing any tests that check if multi-threading does indeed work (safely). We'll need those added too.
I'm also not seeing any tests that check if multi-threading does indeed work (safely). We'll need those added too.
Thoughts on how to prove that? Prove that the API key for a given client is what is was originally even after a new client (with a different key) is made?
Thoughts on how to prove that? Prove that the API key for a given client is what is was originally even after a new client (with a different key) is made?
Hmmm, fair point. Would it make sense to have an API key getter? Once you create a client you can call the getter on the client object? Could even assist users when debugging problems because they can programmatically determine which client is using which API key. Then in our tests, we can attempt to multithread a few requests with different API keys, assert the getters pull out the right keys and don't mix them up on different threads?
Thoughts on how to prove that? Prove that the API key for a given client is what is was originally even after a new client (with a different key) is made?
Hmmm, fair point. Would it make sense to have an API key getter? Once you create a client you can call the getter on the client object? Could even assist users when debugging problems because they can programmatically determine which client is using which API key. Then in our tests, we can attempt to multithread a few requests with different API keys, assert the getters pull out the right keys and don't mix them up on different threads?
Sounds like a plan. I had originally designed the Client to be a black box (once you set the API key, you can't retrieve it or change it), but I've since re-designed it since that level of obfuscation is probably more harmful than helpful. I will write a unit test to confirm that API keys remain with the client as expected. Which API keys should we use? The test key and what else, a fake key, the production key?
Sounds like a plan. I had originally designed the Client to be a black box (once you set the API key, you can't retrieve it or change it), but I've since re-designed it since that level of obfuscation is probably more harmful than helpful. I will write a unit test to confirm that API keys remain with the client as expected. Which API keys should we use? The test key and what else, a fake key, the production key?
We should just use multiple fake keys. No need to make real API calls for these particular tests and no need to expose our API keys in assertions either. 123, 456, 789 should do, 3 separate threads in a single test and three assertions?
Feedback has been addressed. Just waiting on decision about the Beta visibility.
@nwithan8 were you able to attempt the namespacing concept for beta?
Yes, I have implemented it. I have a concern that I've commented (the best I could) in Client.cs. I'd like to discuss how future beta features might look to determine if my concerns are founded or not.
Beta access has been reworked again. This implementation is less automagical, but does give us as maintainers more fine-grained control. Came about as I was able to disconnect the idea of "beta group of features" and "beta API prefix".
Users access the "beta group of features" via myClient.Beta.
Meanwhile, each and every API call in the library (regardless of what group they belong to) can, on an individual basis, target a specific API version (defaults to general availability).
This will allow us finer control over what endpoint each function hits (albeit, slightly more maintenance), and can scale to accommodate future other API versions (imagine, Address.Create always hits v2, but Address.Retrieve hits a new v3, and there's a beta feature called Address.Whatever that hits beta, all in the same Address class)
My only thought here is instead of calling it
ApiVersion.Generalwe call itApiVersion.Currentto be a bit more clear?
Latest, Current, GeneralAvailability, whatever you want.