[#215] Fix get latest version node #215
2 changed files with 159 additions and 3 deletions
|
@ -586,7 +586,7 @@ func (c *Tree) GetLatestVersion(ctx context.Context, bktInfo *data.BucketInfo, o
|
|||
TreeID: versionTree,
|
||||
Path: path,
|
||||
Meta: meta,
|
||||
LatestOnly: true,
|
||||
LatestOnly: false,
|
||||
AllAttrs: false,
|
||||
}
|
||||
nodes, err := c.service.GetNodes(ctx, p)
|
||||
|
@ -594,11 +594,43 @@ func (c *Tree) GetLatestVersion(ctx context.Context, bktInfo *data.BucketInfo, o
|
|||
return nil, err
|
||||
}
|
||||
|
||||
if len(nodes) == 0 {
|
||||
latestNode, err := getLatestNode(nodes)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return newNodeVersion(objectName, latestNode)
|
||||
}
|
||||
|
||||
func getLatestNode(nodes []NodeResponse) (NodeResponse, error) {
|
||||
var (
|
||||
maxCreationTime uint64
|
||||
targetIndexNode = -1
|
||||
)
|
||||
dkirillov marked this conversation as resolved
Outdated
|
||||
|
||||
for i, node := range nodes {
|
||||
dkirillov marked this conversation as resolved
Outdated
dkirillov
commented
I'm not sure it's enough to check length of I'm not sure it's enough to check length of `GetMeta` (as I understand intermediate node also contains some meta: `FileName`). Probably it's better to check for `OID` attribute that must exist in case of object-version node
|
||||
currentCreationTime := node.GetTimestamp()
|
||||
if checkExistOID(node.GetMeta()) && currentCreationTime > maxCreationTime {
|
||||
maxCreationTime = currentCreationTime
|
||||
targetIndexNode = i
|
||||
}
|
||||
}
|
||||
dkirillov marked this conversation as resolved
Outdated
dkirillov
commented
What if the What if the `nodes` doesn't contain any node with `OID` attribute?
alexvanin
commented
Let's write a unit test for Let's write a unit test for `getLatestNode` and check there some corner cases here.
r.loginov
commented
Added a check of this case. Now in this case, the error > What if the `nodes` doesn't contain any node with `OID` attribute?
Added a check of this case. Now in this case, the error `layer.ErrNodeNotFound` will be returned.
|
||||
|
||||
if targetIndexNode == -1 {
|
||||
return nil, layer.ErrNodeNotFound
|
||||
}
|
||||
|
||||
return newNodeVersion(objectName, nodes[0])
|
||||
return nodes[targetIndexNode], nil
|
||||
}
|
||||
|
||||
func checkExistOID(meta []Meta) bool {
|
||||
for _, kv := range meta {
|
||||
if kv.GetKey() == "OID" {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// pathFromName splits name by '/'.
|
||||
|
|
|
@ -168,3 +168,127 @@ func TestTreeServiceAddVersion(t *testing.T) {
|
|||
require.Len(t, versions, 1)
|
||||
require.Equal(t, storedNode, versions[0])
|
||||
}
|
||||
|
||||
func TestGetLatestNode(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
nodes []NodeResponse
|
||||
exceptedNodeID uint64
|
||||
error bool
|
||||
}{
|
||||
{
|
||||
name: "empty",
|
||||
nodes: []NodeResponse{},
|
||||
error: true,
|
||||
},
|
||||
{
|
||||
name: "one node of the object version",
|
||||
nodes: []NodeResponse{
|
||||
nodeResponse{
|
||||
nodeID: 1,
|
||||
parentID: 0,
|
||||
timestamp: 1,
|
||||
meta: []nodeMeta{
|
||||
{
|
||||
key: oidKV,
|
||||
value: []byte(oidtest.ID().String()),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
exceptedNodeID: 1,
|
||||
},
|
||||
{
|
||||
name: "one node of the object version and one node of the secondary object",
|
||||
nodes: []NodeResponse{
|
||||
nodeResponse{
|
||||
nodeID: 2,
|
||||
parentID: 0,
|
||||
timestamp: 3,
|
||||
meta: []nodeMeta{},
|
||||
},
|
||||
nodeResponse{
|
||||
nodeID: 1,
|
||||
parentID: 0,
|
||||
timestamp: 1,
|
||||
meta: []nodeMeta{
|
||||
{
|
||||
key: oidKV,
|
||||
value: []byte(oidtest.ID().String()),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
exceptedNodeID: 1,
|
||||
},
|
||||
{
|
||||
name: "all nodes represent a secondary object",
|
||||
nodes: []NodeResponse{
|
||||
nodeResponse{
|
||||
nodeID: 2,
|
||||
parentID: 0,
|
||||
timestamp: 3,
|
||||
meta: []nodeMeta{},
|
||||
},
|
||||
nodeResponse{
|
||||
nodeID: 4,
|
||||
parentID: 0,
|
||||
timestamp: 5,
|
||||
meta: []nodeMeta{},
|
||||
},
|
||||
},
|
||||
error: true,
|
||||
},
|
||||
{
|
||||
name: "several nodes of different types and with different timestamp",
|
||||
nodes: []NodeResponse{
|
||||
nodeResponse{
|
||||
nodeID: 1,
|
||||
parentID: 0,
|
||||
timestamp: 1,
|
||||
meta: []nodeMeta{
|
||||
{
|
||||
key: oidKV,
|
||||
value: []byte(oidtest.ID().String()),
|
||||
},
|
||||
},
|
||||
},
|
||||
nodeResponse{
|
||||
nodeID: 3,
|
||||
parentID: 0,
|
||||
timestamp: 3,
|
||||
meta: []nodeMeta{},
|
||||
},
|
||||
nodeResponse{
|
||||
nodeID: 4,
|
||||
parentID: 0,
|
||||
timestamp: 4,
|
||||
meta: []nodeMeta{
|
||||
{
|
||||
key: oidKV,
|
||||
value: []byte(oidtest.ID().String()),
|
||||
},
|
||||
},
|
||||
},
|
||||
nodeResponse{
|
||||
nodeID: 6,
|
||||
parentID: 0,
|
||||
timestamp: 6,
|
||||
meta: []nodeMeta{},
|
||||
},
|
||||
},
|
||||
exceptedNodeID: 4,
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
actualNode, err := getLatestNode(tc.nodes)
|
||||
if tc.error {
|
||||
require.Error(t, err)
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, tc.exceptedNodeID, actualNode.GetNodeID())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue
If we initialize
targetIndexNode
with-1
then we can dropobjectNodeExist
variable and check forBut it's optional