search

johannesboyne / gofakes3

A simple fake AWS S3 object storage (used for local test-runs against AWS S3 APIs)
MIT License
361 stars 84 forks source link

Investigate and fix issue 64 #67

Closed johannesboyne closed 2 years ago

johannesboyne commented 2 years ago

Issue #64 PutObjectTagging overwrites metadata showed that metadata is not carried over correctly.

This fixes the open issue by checking if the object exists before and merging the metadata fields.

sonarcloud[bot] commented 2 years ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

codecov-commenter commented 2 years ago

Codecov Report

Merging #67 (5c13452) into master (83a58ec) will increase coverage by 0.08%. The diff coverage is 57.89%.

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
+ Coverage   67.36%   67.45%   +0.08%     
==========================================
  Files          28       28              
  Lines        2534     2553      +19     
==========================================
+ Hits         1707     1722      +15     
- Misses        581      583       +2     
- Partials      246      248       +2
Impacted Files Coverage Δ
backend/s3afero/multi.go 39.50% <33.33%> (+0.65%) :arrow_up:
backend/s3afero/single.go 40.00% <33.33%> (+0.79%) :arrow_up:
backend/s3mem/backend.go 74.25% <33.33%> (-0.47%) :arrow_down:
backend.go 91.30% <80.00%> (-8.70%) :arrow_down:

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 83a58ec...5c13452. Read the comment docs.

johannesboyne commented 2 years ago

Hey @shabbyrobe - would be great to get your view regarding this PR. Did I miss a place, and is the logic clear? The codecov/patch is a bit stricked here. The changes are actually tested by the added test-case IMO.

shabbyrobe commented 2 years ago

Sorry for the delay. It all looks good to me! The one callout I'd have would be to suggest adding a note to the documentation in the Backend interface itself for each function affected, just as a note to implementers to say that they should call MergeMetadata, but that's not super important.