Skip to content

Feature/add sdkwrapper identifier - #625

Open
w-goog wants to merge 13 commits into
mainfrom
feature/add-sdkwrapper-identifier
Open

Feature/add sdkwrapper identifier#625
w-goog wants to merge 13 commits into
mainfrom
feature/add-sdkwrapper-identifier

Conversation

@w-goog

@w-goog w-goog commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

A small number of clients bundle GoogleSignIn-iOS with their own SDKs. To ensure that the way we understand adoption of the SDK is accurate, we'd like to allow those bundlers to say "Hi, this clientID may be App123, but I, Bundler0, am handling it on their behalf".

This is entirely optional and represents no functional change to the way authorization requests are handled on the backend.

@w-goog
w-goog requested a review from mdmathias August 21, 2026 15:18
@w-goog
w-goog marked this pull request as ready for review August 21, 2026 15:18
}

+ (void)setWrapperIdentifier:(nullable NSString *)wrapperIdentifier {
[GIDSignInPreferences setWrapperIdentifier:wrapperIdentifier];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to enforce the "set once" language in property's doc comment?

// Both tokens expired 10 seconds ago.
GIDGoogleUser *user = [self googleUserWithAccessTokenExpiresIn:-10 idTokenExpiresIn:-10];

XCTestExpectation *expectation = [self expectationWithDescription:@"Callback is called"];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the description more descriptive? I know the pattern in the library is to use "Callback is called," but I've had trouble in past with debugging failed tests when I see a wall of that output. It's hard to find which callback wasn't called.


os_unfair_lock_lock(&gWrapperIdentifierLock);
NSString *current = gWrapperIdentifier;
if (current != nil && ![current isEqualToString:sanitized]) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the "set once" enforcement is here. That's a little surprising, but I think I understand the reasoning. I don't know if this can be addressed, but I do still feel like it's confusing to see the doc comment on the property of the other class, and then for the actual enforcement to be here. ¯_(ツ)_/¯

Probably no big deal. Feel free to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants