Skip to content

Conversation

cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Apr 3, 2025

not critical extension for now (nspcc-dev/neofs-api#305 (comment)), but useful in general

`Container.SetAuthKey` was tested by mistake.

Signed-off-by: Leonard Lyubich <[email protected]>
Zero (undefined overall) session token is returned from the cache on
miss, so it makes no sense to access its fields.

Signed-off-by: Leonard Lyubich <[email protected]>
For the sake of transparency and flexibility: struct field is of []byte
type. Also, field can be extended within nspcc-dev/neofs-api#305.
New methods will allow to adapt variadic format in the app without lib
changes.

Signed-off-by: Leonard Lyubich <[email protected]>
API added in e0c7bdb allows it.

Signed-off-by: Leonard Lyubich <[email protected]>
API added in e0c7bdb allows it.

Signed-off-by: Leonard Lyubich <[email protected]>
Copy link

codecov bot commented Apr 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.25%. Comparing base (ac5d07b) to head (764938f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #704      +/-   ##
==========================================
+ Coverage   55.22%   55.25%   +0.03%     
==========================================
  Files         166      166              
  Lines       22803    22803              
==========================================
+ Hits        12593    12600       +7     
+ Misses       9937     9931       -6     
+ Partials      273      272       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cthulhu-rider cthulhu-rider marked this pull request as ready for review April 3, 2025 14:34
Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

One of the differences of proto/* and "normal" types to me is to avoid the possibility of creating an incorrect instance. This makes it easier to create an invalid token.


// SetAuthKeyBytes sets binary-encoded public key corresponding to the private key
// bound to the session.
func (x *commonData) SetAuthKeyBytes(b []byte) {
Copy link
Member

Choose a reason for hiding this comment

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

This makes the API ambiguous to me. If you want raw bytes --- proto/session is here for you.

}

var key neofsecdsa.PublicKey
if err = key.Decode(res.PublicKey()); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Actually this suggests another inconsistency (SDK-level), sometimes keys are proper keys and sometimes they are bytes. Depends on type.

@cthulhu-rider
Copy link
Contributor Author

according to nspcc-dev/neofs-api#305 (comment), this is not needed for now, meaning redundant. So i'll leave this PR as a reminder. If we'll need to support smth other than keys, changes may become relevant

@cthulhu-rider cthulhu-rider marked this pull request as draft April 3, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants