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

feat: add support of null to remove the CORS configuration from bucket #438

Merged
merged 2 commits into from Jul 22, 2020

Conversation

athakor
Copy link
Contributor

@athakor athakor commented Jul 20, 2020

Fixes #437

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 20, 2020
@athakor athakor requested a review from frankyn July 20, 2020 14:25
@codecov
Copy link

codecov bot commented Jul 20, 2020

Codecov Report

Merging #438 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #438      +/-   ##
============================================
+ Coverage     63.15%   63.17%   +0.01%     
- Complexity      610      619       +9     
============================================
  Files            32       32              
  Lines          5133     5133              
  Branches        490      489       -1     
============================================
+ Hits           3242     3243       +1     
  Misses         1726     1726              
+ Partials        165      164       -1     
Impacted Files Coverage Δ Complexity Δ
...main/java/com/google/cloud/storage/BucketInfo.java 81.74% <100.00%> (+0.16%) 83.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e63feb7...47f366d. Read the comment docs.

@@ -1509,7 +1509,7 @@ public StorageClass getStorageClass() {
* (CORS)</a>
*/
public List<Cors> getCors() {
return cors;
return cors != null ? cors : ImmutableList.<Cors>of();
Copy link
Member

Choose a reason for hiding this comment

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

Please leave this field as is and revert change. There are potentially 4 cases which are considered here and we don't want to mess them up.

  1. Cors are set and field selector is selected then returns not-null.
  2. Cors are set but field selector isn't selected then returns null.
  3. Cors are not set and field selector is selected then returns null.
  4. Cors are not set and field selector isn't selected then returns null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@athakor
Copy link
Contributor Author

athakor commented Jul 22, 2020

@frankyn PTAL

@frankyn frankyn merged commit f8a4b12 into googleapis:master Jul 22, 2020
pmakani pushed a commit to pmakani/java-storage that referenced this pull request Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to remove CORS configuration from bucket by setting null
2 participants