[#78] policy: Introduce ListTargets method for Policy contract #80

Merged
fyrchik merged 1 commit from aarifullin/frostfs-contract:feat/78-list_targets into master 2024-09-04 19:51:18 +00:00
Member
  • Introduce a new method ListTargets that lists targets by kind.
  • Slightly fix key mapping - also concatenate kind to prefix.
  • Write unit-tests.
  • Regenerate rpcclient.

Check comments in #79

Close #78

* Introduce a new method ListTargets that lists targets by kind. * Slightly fix key mapping - also concatenate kind to prefix. * Write unit-tests. * Regenerate rpcclient. Check comments in [#79](https://git.frostfs.info/TrueCloudLab/frostfs-contract/pulls/79) Close #78
aarifullin requested review from storage-core-committers 2024-02-19 12:08:45 +00:00
aarifullin requested review from storage-core-developers 2024-02-19 12:08:45 +00:00
aarifullin requested review from storage-services-committers 2024-02-19 12:08:55 +00:00
aarifullin requested review from storage-services-developers 2024-02-19 12:09:01 +00:00
dstepanov-yadro reviewed 2024-02-19 13:07:03 +00:00
@ -161,1 +166,4 @@
storage.Delete(ctx, key)
// If no chains are left for the target, then remove the mapping.
it := storage.Find(ctx, key, storage.KeysOnly)

Hm, I don't understand.


	storage.Delete(ctx, key) <--- here `key` record deleted

	// If no chains are left for the target, then remove the mapping.
	it := storage.Find(ctx, key, storage.KeysOnly) <--- how it can be present here?
Hm, I don't understand. ``` storage.Delete(ctx, key) <--- here `key` record deleted // If no chains are left for the target, then remove the mapping. it := storage.Find(ctx, key, storage.KeysOnly) <--- how it can be present here? ```
Author
Member

Oh, sorry, my bad! You're right.
I have fixed that. After removal the methods are checking if records with n/c + encoded_num prefix still exist in the storage

Oh, sorry, my bad! You're right. I have fixed that. After _removal_ the methods are checking if records with `n/c + encoded_num` prefix still exist in the storage
aarifullin force-pushed feat/78-list_targets from 7000b294b3 to 8e8453ac39 2024-02-19 13:29:17 +00:00 Compare
dkirillov approved these changes 2024-02-19 13:43:59 +00:00
dstepanov-yadro reviewed 2024-02-19 15:42:04 +00:00
@ -177,1 +189,4 @@
}
// If no chains are left for the target, then remove the mapping.
prefix := append([]byte{byte(entity)}, common.ToFixedWidth64(entityNameNum)...)

Is mappingKeyPrefix required?

Is `mappingKeyPrefix` required?
Author
Member

Nope. mappingKeyPrefix is used to create mapKey:
'm' + 'n'/'c' + "namespace_name/CID"

So, it is for mapping. But chains, themselves, are kept in the contract storage like that:

'n'/'c' + [mapping_encoded_to_bytes](8 bytes) + chain_name

Nope. `mappingKeyPrefix` is used to create `mapKey`: `'m' + 'n'/'c' + "namespace_name/CID"` So, it is for mapping. But chains, themselves, are kept in the contract storage like that: `'n'/'c' + [mapping_encoded_to_bytes](8 bytes) + chain_name`
fyrchik reviewed 2024-02-20 07:37:56 +00:00
policy/doc.go Outdated
@ -8,3 +8,3 @@
| 'n' + uint16(len(namespace)) + namespace | []byte | Container chain |
| 'm' + entity name (namespace/container) | []byte | Mapped name to an encoded number |
| 'counter' | uint64 | Integer counter used for mapping |
| 'Counter' | uint64 | Integer counter used for mapping |
Owner

in another commit, please

in another commit, please
Author
Member

Fixed

Fixed
fyrchik marked this conversation as resolved
@ -129,2 +133,3 @@
entityNameBytes := mapToNumericCreateIfNotExists(ctx, []byte(entityName))
entityNameBytes := mapToNumericCreateIfNotExists(ctx, entity, []byte(entityName))
key := storageKey(entity, entityNameBytes, name)
Owner

empty line

empty line
Author
Member

Fixed

Fixed
fyrchik marked this conversation as resolved
@ -151,0 +175,4 @@
s, err := e.TestInvoke(t, "listTargets", kind)
require.NoError(t, err)
if s.Len() == 0 {
Owner

require.Equal(t, 0, s.Len(), "stack is empty")?

`require.Equal(t, 0, s.Len(), "stack is empty")`?
Author
Member

Fixed

Fixed
fyrchik marked this conversation as resolved
aarifullin force-pushed feat/78-list_targets from 8e8453ac39 to 43c90af97d 2024-02-20 09:00:13 +00:00 Compare
acid-ant approved these changes 2024-02-20 09:00:25 +00:00
fyrchik approved these changes 2024-02-22 07:33:18 +00:00
fyrchik merged commit 43c90af97d into master 2024-02-22 07:33:25 +00:00
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-core-developers
TrueCloudLab/storage-services-developers
No milestone
No project
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-contract#80
No description provided.