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

Support the projection parameter in getObject #1520

Merged
merged 1 commit into from
Mar 18, 2024

Conversation

manuteleco
Copy link
Contributor

According to the behavior described in the official documentation. That is, ACL information should only be returned if projection=full is provided as GET parameter.

This behavior is not testeable using the Go GCS client, because it hardcodes the value full when performing the HTTP request (see implementation).

Copy link
Owner

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

Thank you very much for contributing! This looks good overall.

This behavior is not testeable using the Go GCS client, because it hardcodes the value full when performing the HTTP request (see implementation).

Do you think it'd make sense to write a test that sends a low-level request that bypasses the client?

There are some examples in object_test.go, such as TestServerClientObjectUpdateContentType.

I guess we'd want to test the default behavior and explicitly setting the projection to noAcl?

@manuteleco manuteleco requested a review from fsouza March 16, 2024 10:11
Copy link
Owner

@fsouza fsouza left a comment

Choose a reason for hiding this comment

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

Thank you very much! This looks good, I can merge it once the merge conflict is resolved.

fakestorage/response.go Outdated Show resolved Hide resolved
According to the behavior described in the [official documentation][1].
That is, ACL information should only be returned if `projection=full` is
provided as GET parameter.

This behavior is not testeable using the Go GCS client, because it
hardcodes the value `full` when performing the HTTP request (see
[implementation][2]).

[1]: https://cloud.google.com/storage/docs/json_api/v1/objects/get#parameters
[2]: https://github.com/googleapis/google-cloud-go/blob/storage/v1.39.0/storage/http_client.go#L413
@manuteleco manuteleco force-pushed the support_projection_in_getobject branch from 98693d7 to bb21b0a Compare March 18, 2024 20:23
@fsouza fsouza merged commit c595ce6 into fsouza:main Mar 18, 2024
27 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.

2 participants