nspcc-dev/neofs-s3-gw

Fork 0

mirror of https://github.com/nspcc-dev/neofs-s3-gw.git synced 2026-03-01 04:29:15 +00:00

Don't return version-id to HeadObject and GetObject in unversioned bucket #278

New issue

Closed

opened 2025-12-28 17:36:50 +00:00 by sami · 5 comments

sami commented

2025-12-28 17:36:50 +00:00

Owner

Originally created by @masterSplinter01 on GitHub (Jul 5, 2022).

Originally assigned to: @masterSplinter01 on GitHub.

In a tree-service branch:

$ aws s3api create-bucket --bucket test6 --endpoint-url http://127.0.0.1:8084   
$ aws s3api put-object --key a --body ~/Downloads/cat.jpg   --bucket test6 --endpoint-url http://127.0.0.1:8084
$ aws s3api list-object-versions  --bucket test6 --endpoint-url http://127.0.0.1:8084
{
  "Versions": [
    {
      "ETag": "03719ec2b4558dc89c82c47283b4ab9dd17e6e56d1858f254892707491443536",
      "Size": 87422,
      "Key": "a",
      "VersionId": "null",
      "IsLatest": true,
      "LastModified": "2022-07-05T21:08:07+00:00",
      "Owner": {
        "DisplayName": "NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM",
        "ID": "NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM"
      }
    }
  ]
}

$ aws s3api head-object --key a  --bucket test6 --endpoint-url http://127.0.0.1:8084
{
  "LastModified": "2022-07-05T21:08:07+00:00",
  "ContentLength": 87422,
  "ETag": "03719ec2b4558dc89c82c47283b4ab9dd17e6e56d1858f254892707491443536",
  "VersionId": "xBVPsuyhPs4fg7t1SMAhAbKucSutapFeew4wMfeFqxo",
  "ContentType": "image/jpeg",
  "Metadata": {
    "Content-Type": "image/jpeg"
  }
}

Originally created by @masterSplinter01 on GitHub (Jul 5, 2022). Originally assigned to: @masterSplinter01 on GitHub. In a `tree-service` branch: ``` $ aws s3api create-bucket --bucket test6 --endpoint-url http://127.0.0.1:8084 $ aws s3api put-object --key a --body ~/Downloads/cat.jpg --bucket test6 --endpoint-url http://127.0.0.1:8084 $ aws s3api list-object-versions --bucket test6 --endpoint-url http://127.0.0.1:8084 { "Versions": [ { "ETag": "03719ec2b4558dc89c82c47283b4ab9dd17e6e56d1858f254892707491443536", "Size": 87422, "Key": "a", "VersionId": "null", "IsLatest": true, "LastModified": "2022-07-05T21:08:07+00:00", "Owner": { "DisplayName": "NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM", "ID": "NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM" } } ] } $ aws s3api head-object --key a --bucket test6 --endpoint-url http://127.0.0.1:8084 { "LastModified": "2022-07-05T21:08:07+00:00", "ContentLength": 87422, "ETag": "03719ec2b4558dc89c82c47283b4ab9dd17e6e56d1858f254892707491443536", "VersionId": "xBVPsuyhPs4fg7t1SMAhAbKucSutapFeew4wMfeFqxo", "ContentType": "image/jpeg", "Metadata": { "Content-Type": "image/jpeg" } } ```

sami

2025-12-28 17:36:50 +00:00

closed this issue
added the
tree-service

U3
labels

sami commented

2025-12-28 17:36:50 +00:00

Author

Owner

@masterSplinter01 commented on GitHub (Jul 11, 2022):

After some view I think we should to do the following things:

We don't need in ObjectInfo as in independent struct, in almost most usages we extract it from ExtendedObjectInfo. ObjectInfo will remain, but we can replace usages ObjectInfo with ExtendedObjectInfo in layer package

type ExtendedObjectInfo struct {
	ObjectInfo  *ObjectInfo
	NodeVersion *NodeVersion
	IsLatest    bool
}
type NodeVersion struct {
	BaseNodeVersion
	DeleteMarker  *DeleteMarkerInfo
	IsUnversioned bool
}

Make methods of ObjectInfo as methods of ExtendedObjectInfo.
ObjectVersionInfo :

	ObjectVersionInfo struct {
		Object        *data.ObjectInfo
		IsLatest      bool
		IsUnversioned bool
	}

seems redundant, we can use ExtendedObjectInfo instead of this.

@masterSplinter01 commented on GitHub (Jul 11, 2022): After some view I think we should to do the following things: 1. We don't need in `ObjectInfo` as in independent struct, in **almost** most usages we extract it from `ExtendedObjectInfo`. `ObjectInfo` will remain, but we can replace usages `ObjectInfo` with `ExtendedObjectInfo` in `layer` package ``` type ExtendedObjectInfo struct { ObjectInfo *ObjectInfo NodeVersion *NodeVersion IsLatest bool } type NodeVersion struct { BaseNodeVersion DeleteMarker *DeleteMarkerInfo IsUnversioned bool } ``` 2. Make methods of `ObjectInfo` as methods of `ExtendedObjectInfo`. 3. `ObjectVersionInfo` : ``` ObjectVersionInfo struct { Object *data.ObjectInfo IsLatest bool IsUnversioned bool } ``` seems redundant, we can use `ExtendedObjectInfo` instead of this.

sami commented

2025-12-28 17:36:50 +00:00

Author

Owner

@alexvanin commented on GitHub (Jul 12, 2022):

We don't need in ObjectInfo as in independent struct, in almost most usages we extract it from ExtendedObjectInfo

I think the issues with this is that ObjectInfo is formed based on object.Head() response, and ExtendedObjectInfo requires RPC with tree service. So they originate from different network requests, I think it is a good reason to keep them separated.

but we can replace usages ObjectInfo with ExtendedObjectInfo in layer package

It has pros: use single structure in the code. But it also has cons: if layer doesn't need to work with versions, why bother and pass ExtendedObjectInfo instead of ObjectInfo?

Make methods of ObjectInfo as methods of ExtendedObjectInfo.

I guess we can inline ObjectInfo into ExtendedObjectInfo like

type ExtendedObjectInfo struct {
	ObjectInfo
	NodeVersion *NodeVersion
	IsLatest    bool
}

seems redundant, we can use ExtendedObjectInfo instead of this.

It seems so.

/cc @KirillovDenis

@alexvanin commented on GitHub (Jul 12, 2022): > We don't need in ObjectInfo as in independent struct, in almost most usages we extract it from ExtendedObjectInfo I think the issues with this is that `ObjectInfo` is formed based on `object.Head()` response, and `ExtendedObjectInfo` requires RPC with tree service. So they originate from different network requests, I think it is a good reason to keep them separated. > but we can replace usages ObjectInfo with ExtendedObjectInfo in layer package It has **pros**: use single structure in the code. But it also has **cons**: if `layer` doesn't need to work with versions, why bother and pass `ExtendedObjectInfo` instead of `ObjectInfo`? > Make methods of ObjectInfo as methods of ExtendedObjectInfo. I guess we can inline `ObjectInfo` into `ExtendedObjectInfo` like ```go type ExtendedObjectInfo struct { ObjectInfo NodeVersion *NodeVersion IsLatest bool } ``` > seems redundant, we can use ExtendedObjectInfo instead of this. It seems so. /cc @KirillovDenis

sami commented

2025-12-28 17:36:50 +00:00

Author

Owner

@masterSplinter01 commented on GitHub (Jul 12, 2022):

I don't suggest to drop ObjectInfo. I suggest to make it an unexported struct.
Wherever we call objectHead (with objectInfoFromMeta) and objectInfoFromObjectsCacheOrNeoFS we already have made requests to the tree service.

I guess we can inline ObjectInfo into ExtendedObjectInfo like

I'm not sure it will be enough, the reason why we should move methods from ObjectInfo to ExtendedObjectInfo is ability return correct version with Version function: null or version-id. ExtendedObjectInfo has enough information to answer the question, ObjectInfo hasn't. And we will not have to check what returned Version and fix it after like here:

versionID := ver.Object.Version()
if ver.IsUnversioned {
	versionID = layer.UnversionedObjectVersionID
}

@masterSplinter01 commented on GitHub (Jul 12, 2022): I don't suggest to drop `ObjectInfo`. I suggest to make it an unexported struct. Wherever we call `objectHead` (with `objectInfoFromMeta`) and `objectInfoFromObjectsCacheOrNeoFS` we already have made requests to the tree service. > I guess we can inline ObjectInfo into ExtendedObjectInfo like I'm not sure it will be enough, the reason why we should move methods from `ObjectInfo` to `ExtendedObjectInfo` is ability return correct version with `Version` function: `null` or `version-id`. `ExtendedObjectInfo` has enough information to answer the question, `ObjectInfo` hasn't. And we will not have to check what returned `Version` and fix it after like here: ``` versionID := ver.Object.Version() if ver.IsUnversioned { versionID = layer.UnversionedObjectVersionID } ```

sami commented

2025-12-28 17:36:50 +00:00

Author

Owner

@KirillovDenis commented on GitHub (Jul 13, 2022):

Wherever we call objectHead (with objectInfoFromMeta) and objectInfoFromObjectsCacheOrNeoFS we already have made requests to the tree service.

Yes, but here and therefore here we don't need additional fields from ExtendedObjectInfo .

the reason why we should move methods from ObjectInfo to ExtendedObjectInfo is ability return correct version with Version function

Let's just add (not move) Version method to ExtendedObjectInfo.

@KirillovDenis commented on GitHub (Jul 13, 2022): > Wherever we call objectHead (with `objectInfoFromMeta`) and `objectInfoFromObjectsCacheOrNeoFS` we already have made requests to the tree service. Yes, but [here](https://github.com/nspcc-dev/neofs-s3-gw/blob/04fe747972c050b5545438db5362451e4c311faf/api/layer/object.go#L415) and therefore [here](https://github.com/nspcc-dev/neofs-s3-gw/blob/04fe747972c050b5545438db5362451e4c311faf/api/layer/object.go#L512) we don't need additional fields from `ExtendedObjectInfo` . > the reason why we should move methods from `ObjectInfo` to `ExtendedObjectInfo` is ability return correct version with Version function Let's just add (not move) `Version` method to `ExtendedObjectInfo`.

sami commented

2025-12-28 17:36:50 +00:00

Author

Owner

@KirillovDenis commented on GitHub (Jul 13, 2022):

Maybe we can introduce new function GetExtendedObjectInfo along with
nspcc-dev/neofs-s3-gw@04fe747972/api/layer/layer.go (L216)
and use it when necessary?

@KirillovDenis commented on GitHub (Jul 13, 2022): Maybe we can introduce new function `GetExtendedObjectInfo` along with https://github.com/nspcc-dev/neofs-s3-gw/blob/04fe747972c050b5545438db5362451e4c311faf/api/layer/layer.go#L216 and use it when necessary?