Add new upload and edit download requests #206

tatiana-nspcc · 2024-05-12T23:27:48Z

Close #168.

codecov · 2024-05-12T23:30:28Z

Codecov Report

Attention: Patch coverage is 21.83908% with 340 lines in your changes are missing coverage. Please review.

Project coverage is 22.13%. Comparing base (d860495) to head (3747724).
Report is 1 commits behind head on master.

Files	Patch %	Lines
handlers/newObjects.go	0.00%	152 Missing ⚠️
handlers/apiserver/rest-server.gen.go	36.62%	92 Missing and 17 partials ⚠️
handlers/objects.go	0.00%	57 Missing ⚠️
handlers/preflight.go	0.00%	15 Missing ⚠️
handlers/util.go	82.05%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #206      +/-   ##
==========================================
- Coverage   22.14%   22.13%   -0.02%     
==========================================
  Files          16       17       +1     
  Lines        2894     3293     +399     
==========================================
+ Hits          641      729      +88     
- Misses       2137     2431     +294     
- Partials      116      133      +17

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

roman-khimov

I think in general this approach should be OK for us. S3 goes via PutObjectTagging with XML-encoded data, but we can't have a separate request since objects are immutable. Caveats:

data with embedded newlines, it can't be handled in headers easily (requires safe encoding), special characters can be a problem too
some data comes from headers and some from these attributes, we need to lay down the rules explicitly on how they interact
well-known attributes can be handled via well-known headers

spec/rest.yaml

roman-khimov · 2024-05-13T15:19:45Z

spec/rest.yaml

+            type: string
+      requestBody:
+        content:
+          multipart/form-data:


Let's think if we need it to be a multipart/form-data.

What do you mean? What other options do we have?

Now the object can be uploaded directly to the body request.

handlers/objects.go

handlers/util.go

spec/rest.yaml

handlers/util.go

roman-khimov · 2024-05-21T20:42:07Z

spec/rest.yaml

+              - "2024-12-31T23:59:59Z" represents the last moment of 2024 in UTC.
+              - "2024-12-31T15:59:59-08:00" represents 3:59 PM on December 31, 2024, Pacific Time.
+            - `__NEOFS__EXPIRATION_TIMESTAMP` - specifies the exact timestamp of expiration.
+            - `__NEOFS__EXPIRATION_DURATION` - specifies the duration until expiration in Go's duration format. 


Where this comes from?

It was here for ages:

neofs-rest-gw/handlers/util.go

Line 37 in f024e79

ExpirationDurationAttr = SystemAttributePrefix + "EXPIRATION_DURATION"

and

neofs-rest-gw/handlers/util.go

Line 114 in f024e79

func prepareExpirationHeader(headers map[string]string, epochDurations *epochDurations, now time.Time) error {

roman-khimov · 2024-05-21T20:42:23Z

spec/rest.yaml

+              Examples:
+              - "2024-12-31T23:59:59Z" represents the last moment of 2024 in UTC.
+              - "2024-12-31T15:59:59-08:00" represents 3:59 PM on December 31, 2024, Pacific Time.
+            - `__NEOFS__EXPIRATION_TIMESTAMP` - specifies the exact timestamp of expiration.


And this one?

Answered in the other thread.

roman-khimov · 2024-05-21T20:43:43Z

spec/rest.yaml

+            All attributes are in a JSON-formatted map of key-value pairs, where the key is the 
+            attribute name and the value is the attribute value.
+            You can also use one of these special attributes:
+            - `__NEOFS__EXPIRATION_RFC3339` - specifies the expiration time in RFC3339 format. 


These shouldn't be written like this, they're not a real object attributes, API knows nothing about them. We can emulate some of them for user convenience, but we better do this via real HTTP headers, leave JSON for real attributes.

In this case, we will use the following headers for the three attributes: X-Neofs-EXPIRATION_DURATION, X-Neofs-EXPIRATION_TIMESTAMP, X-Neofs-EXPIRATION_RFC3339 as they are in the old GET request. Okay?

docs/migration-new-upload.md

smallhive · 2024-05-22T09:17:05Z

handlers/objects.go

@@ -434,7 +436,7 @@ func (a *RestAPI) getByAddress(ctx echo.Context, addr oid.Address, downloadParam

 	var (
 		payloadSize               = header.PayloadSize()
-		contentType               = a.setAttributes(ctx, payloadSize, addr.Container().String(), addr.Object().String(), header, downloadParam)
+		contentType               = a.setAttributes(ctx, payloadSize, addr.Container().String(), addr.Object().String(), header, downloadParam, jsonParam)


Looks like this function may require a small argument refactoring. To many parameters, we should consider moving them in struct. But maybe in different issue

Fixed using a structure.

smallhive · 2024-05-22T09:19:33Z

handlers/objects.go

+	var useJSON bool
+	if jsonParam != nil && paramIsPositive(*jsonParam) {
+		useJSON = true


We may simplify a bit

useJSON := jsonParam != nil && paramIsPositive(*jsonParam)

Tnx, fixed.

smallhive · 2024-05-22T09:24:29Z

handlers/util.go

+func paramIsPositive(s string) bool {
+	switch s {
+	case "1", "t", "T", "true", "TRUE", "True", "y", "yes", "Y", "YES", "Yes":
+		return true
+	}
+	return false
+}


All usage of the func is working with the pointer. I consider it makes sense to replace the parameter s string to s *string and handle nil inside this function. Yes, it can affect the tests, but simplify the code in general

spec/rest.yaml

Signed-off-by: Tatiana Nesterenko <[email protected]>

Add a new POST request `/objects/{cId}` that accepts the `X-Attributes` header. Also, new GET and HEAD requests are added for object downloading: `/objects/{containerId}/by_id/{objectId}` and `/objects/{containerId}/by_attribute/{attrKey}/{attrVal}`. Signed-off-by: Tatiana Nesterenko <[email protected]>

Signed-off-by: Tatiana Nesterenko <[email protected]>

tatiana-nspcc · 2024-05-27T09:16:21Z

After smallhive's review, I only rebased the branch on the current master.

roman-khimov requested a review from mike-petrov May 13, 2024 09:35

roman-khimov reviewed May 13, 2024

View reviewed changes

handlers/util.go Show resolved Hide resolved

tatiana-nspcc force-pushed the 168-new-upload-download branch from a11b6e2 to fa56453 Compare May 16, 2024 21:35

tatiana-nspcc marked this pull request as ready for review May 17, 2024 10:30

tatiana-nspcc requested a review from smallhive as a code owner May 17, 2024 10:30

tatiana-nspcc requested a review from roman-khimov May 17, 2024 10:30

tatiana-nspcc changed the title ~~168 new upload download~~ Add new upload and edit download requests May 17, 2024

roman-khimov reviewed May 21, 2024

View reviewed changes

smallhive reviewed May 22, 2024

View reviewed changes

tatiana-nspcc force-pushed the 168-new-upload-download branch 2 times, most recently from bde50cf to a7b4cd7 Compare May 26, 2024 23:12

tatiana-nspcc requested review from smallhive and roman-khimov May 26, 2024 23:22

smallhive approved these changes May 27, 2024

View reviewed changes

tatiana-nspcc added 8 commits May 27, 2024 10:13

util.go: Reformat func filterHeaders

64f0f40

Signed-off-by: Tatiana Nesterenko <[email protected]>

handlers: Add object attr Timestamp from Date header

8a3d6c1

Signed-off-by: Tatiana Nesterenko <[email protected]>

handlers: Add OPTIONS for all new requests

bd36434

Signed-off-by: Tatiana Nesterenko <[email protected]>

docs: Add note about new upload and download requests

e9cc770

Signed-off-by: Tatiana Nesterenko <[email protected]>

CHANGELOG: Add Updating from 0.8.3 information

3dd5e2a

Signed-off-by: Tatiana Nesterenko <[email protected]>

handlers: Add tests for util functions of /objects/{cId}

eb60bec

Signed-off-by: Tatiana Nesterenko <[email protected]>

tests: Add integration tests for the new requests

3747724

Signed-off-by: Tatiana Nesterenko <[email protected]>

tatiana-nspcc force-pushed the 168-new-upload-download branch from a7b4cd7 to 3747724 Compare May 27, 2024 09:14

roman-khimov approved these changes May 29, 2024

View reviewed changes

roman-khimov merged commit 93e652d into master May 29, 2024
12 of 14 checks passed

roman-khimov deleted the 168-new-upload-download branch May 29, 2024 18:57

tatiana-nspcc mentioned this pull request May 29, 2024

Add tests for new upload/download requests nspcc-dev/neofs-testcases#803

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new upload and edit download requests #206

Add new upload and edit download requests #206

tatiana-nspcc commented May 12, 2024 •

edited

Loading

codecov bot commented May 12, 2024 •

edited

Loading

roman-khimov left a comment

roman-khimov May 13, 2024

tatiana-nspcc May 17, 2024

tatiana-nspcc May 26, 2024

roman-khimov May 21, 2024

tatiana-nspcc May 22, 2024

roman-khimov May 21, 2024

tatiana-nspcc May 26, 2024

roman-khimov May 21, 2024

tatiana-nspcc May 22, 2024

roman-khimov May 22, 2024

tatiana-nspcc May 26, 2024

smallhive May 22, 2024

tatiana-nspcc May 26, 2024

smallhive May 22, 2024

tatiana-nspcc May 26, 2024

smallhive May 22, 2024

tatiana-nspcc May 26, 2024

tatiana-nspcc commented May 27, 2024

Add new upload and edit download requests #206

Add new upload and edit download requests #206

Conversation

tatiana-nspcc commented May 12, 2024 • edited Loading

codecov bot commented May 12, 2024 • edited Loading

Codecov Report

roman-khimov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tatiana-nspcc commented May 27, 2024

tatiana-nspcc commented May 12, 2024 •

edited

Loading

codecov bot commented May 12, 2024 •

edited

Loading