PROTOTYPE: Auth separation #2405
Conversation
This as a *lot* of copies of dependencies - later commits will try to reduce this set.
This lets us remove BaseClientService and ClientServiceRequest. (That should then let us remove a lot more dependencies.)
(There may be more.) We can use a separate default auth-specific HttpClientFactory without nearly as many features.
We can hand-craft the serialization of these requests (and fix the responses, too)
This forces the user to specify a data store when creating a GoogleWebAuthorizationBroken, but that's probably okay.
We'll need a replacement "default serializer" but that doesn't need to be public.
jskeet
added
the
do not merge
|
The first commit basically copies everything Google.Apis.Auth needs (at all) into the project, then the rest of the commits remove them again, as far as we can... |
| /// <summary> | ||
| /// Asynchronously authorizes the specified user. | ||
| /// Requires user interaction; see <see cref="GoogleWebAuthorizationBroker"/> remarks for more details. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// In case no data store is specified, <see cref="Google.Apis.Util.Store.FileDataStore"/> will be used by |
There was a problem hiding this comment.
Hmmm, I'm not convinced it's a good idea to not provide a data store by default. I think FileDataStore might be a good candidate to move to that very light common library.
There was a problem hiding this comment.
Possibly. I'd really hoped to get away without any concrete classes - and more importantly, FileDataStore uses NewtonsoftJsonSerializer.Instance :(
| namespace Google.Apis.Auth.ExistingDependencies; | ||
|
|
||
| // The replacement for NewtonsoftJsonSerializer | ||
| internal class ReplacementSerializer |
There was a problem hiding this comment.
Is the idea to take a dependency on System.Text.Json or to just serialize/deserialize by hand?
There was a problem hiding this comment.
I have no opinion on that at the moment :)
| @@ -14,6 +14,9 @@ | |||
| limitations under the License. | |||
| */ | |||
|
|
|||
| // Note: the only reference to this is in ServiceAccountCredential. If we throw a different exception, | |||
| // e.g. InvalidOperationException, we could remove GoogleApiException, SingleError and RequestError. | |||
There was a problem hiding this comment.
With some of the initiatives around making errors more actionable I'd still expect things to move in the direction of using specific exception types with structured error messages, even for auth endpoints.
There was a problem hiding this comment.
Hmm. We should work out what to do on that front then; but it's worth at least being aware that this is the only current usage.
This wouldn't currently work at all, but highlights the dependencies from Google.Apis.Auth to Google.Apis and Google.Apis.Core.
The idea would be to create a new, very lightweight, dependency-free library of Google.Apis.Abstractions or something similar, which we'd use type forwarding for in the existing libraries, but depend on as our only dependency in Google.Apis.Auth.
The concrete code (as opposed to interface) that causes issues: