Skip to content
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

Implement +[GTMSessionUploadFetcher fetcherWithSessionIdentifier:] #403

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

bhamiltoncx
Copy link
Member

@bhamiltoncx bhamiltoncx commented Sep 20, 2024

+[GTMSessionUploadFetcher fetcherWithSessionIdentifier:] is declared to return instancetype, so it should return GTMSessionUploadFetcher*.

In reality, that method has no implementation in GTMSessionUploadFetcher.m, so it always returns GTMSessionFetcher*. That means calling GTMSessionUploadFetcher-only methods on the resulting object will crash, since they're not implemented.

The method +[GTMSessionUploadFetcher uploadFetcherForSessionIdentifier:] is implemented to return the correct type, so this PR implements +[GTMSessionUploadFetcher fetcherWithSessionIdentifier:] to thunk through to that method.

Fixes: #402

`+[GTMSessionUploadFetcher fetcherWithSessionIdentifier:]` is declared to return instancetype, so it should return `GTMSessionUploadFetcher*`.

In reality, that method has no implementation in `GTMSessionUploadFetcher.m`, so it always returns `GTMSessionFetcher*`. That means calling `GTMSessionUploadFetcher`-only methods on the resulting object will crash, since they're not implemented.

The method `+[GTMSessionUploadFetcher uploadFetcherForSessionIdentifier:]` is implemented to return the correct type, so this PR implements `+[GTMSessionUploadFetcher fetcherWithSessionIdentifier:]` to thunk through to that method.

Fixes: google#402
@thomasvl
Copy link
Member

thomasvl commented Oct 1, 2024

We can ignore the visionOS failures. Github dropped the SDK from their runners and main has since been updated to stop trying to run the tests as a result.

@thomasvl thomasvl merged commit 65615d7 into google:main Oct 4, 2024
23 of 25 checks passed
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.

Calling +[GTMSessionUploadFetcher fetcherWithSessionIdentifier:] incorrectly returns GTMSessionFetcher
2 participants