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

Add concurrency headers #79

Merged
merged 6 commits into from
Jan 23, 2024
Merged

Add concurrency headers #79

merged 6 commits into from
Jan 23, 2024

Conversation

BogdanIrimie
Copy link
Contributor

@BogdanIrimie BogdanIrimie commented Jan 22, 2024

Add IfMatch header check to SetObject, SetRelation, DeleteObject, DeleteRelation, and DeleteManifest.
Add IfNotmatch header check to GetObject and GetRelatio

The headers are used for optimist concurrency.
In the case of IfMatch HTTP 412 - Precondition Failed is returned if the IfMatch header value does not match the etag
In the case of IfNotMatch HTTP 304-Not Modified is returned if the IfNotMatch header values do match the etag

resolves https://github.com/aserto-dev/workspace/issues/527

@BogdanIrimie BogdanIrimie self-assigned this Jan 22, 2024
@BogdanIrimie BogdanIrimie added the enhancement New feature or request label Jan 22, 2024
Copy link

github-actions bot commented Jan 22, 2024

Pull Request Test Coverage Report for Build 7625492122

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 28.032%

Totals Coverage Status
Change from base Build 7477818265: 0.2%
Covered Lines: 1040
Relevant Lines: 3710

💛 - Coveralls

@@ -46,6 +48,12 @@ func (s *Writer) SetObject(ctx context.Context, req *dsw3.SetObjectRequest) (*ds
return err
}

ifMatchHeader := metautils.ExtractIncoming(ctx).Get(headers.IfMatch)
// if the updReq.Etag == "" this means the this is an insert
if ifMatchHeader != "" && updReq.Etag != "" && ifMatchHeader != updReq.Etag {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The condition detects inserts because otherwise those would always be rejected with a HashMissmatch error.

Comment on lines 89 to 90
obj := &dsc3.Object{Type: req.ObjectType, Id: req.ObjectId}
updReq, err := bdb.UpdateMetadata(ctx, tx, bdb.ObjectsPath, ds.Object(obj).Key(), obj)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do a DB call only if If-Match header is specified.

@BogdanIrimie BogdanIrimie marked this pull request as ready for review January 23, 2024 11:19
@ronenh ronenh merged commit e0135e3 into main Jan 23, 2024
2 checks passed
@ronenh ronenh deleted the add-concurrency-headers branch January 23, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants