Skip to content

Commit

Permalink
[#607] network: Do not work with Address pointers
Browse files Browse the repository at this point in the history
`network.Address` structure in most cases created once and used read-only.

Replace `AddressFromString` function with `Address.FromString` method with
the same purpose and implementation. Make all libraries to work with value.

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
  • Loading branch information
Leonard Lyubich authored and cthulhu-rider committed Jun 18, 2021
1 parent 5de074f commit adbbad0
Show file tree
Hide file tree
Showing 35 changed files with 128 additions and 97 deletions.
8 changes: 4 additions & 4 deletions cmd/neofs-cli/modules/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,15 @@ func getKeyFromWallet(w *wallet.Wallet, addrStr string) (*ecdsa.PrivateKey, erro

// getEndpointAddress returns network address structure that stores multiaddr
// inside, parsed from global arguments.
func getEndpointAddress() (*network.Address, error) {
func getEndpointAddress() (addr network.Address, err error) {
endpoint := viper.GetString("rpc")

addr, err := network.AddressFromString(endpoint)
err = addr.FromString(endpoint)
if err != nil {
return nil, errInvalidEndpoint
err = errInvalidEndpoint
}

return addr, nil
return
}

// getSDKClient returns default neofs-api-go sdk client. Consider using
Expand Down
4 changes: 2 additions & 2 deletions cmd/neofs-node/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type cfg struct {

cfgNodeInfo cfgNodeInfo

localAddr *network.Address
localAddr network.Address

cfgObject cfgObject

Expand Down Expand Up @@ -308,7 +308,7 @@ func initCfg(path string) *cfg {
return c
}

func (c *cfg) LocalAddress() *network.Address {
func (c *cfg) LocalAddress() network.Address {
return c.localAddr
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/neofs-node/config/node/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,13 @@ func Wallet(c *config.Config) *keys.PrivateKey {
// from "node" section as network.Address.
//
// Panics if value is not a valid NeoFS network address
func BootstrapAddress(c *config.Config) *network.Address {
func BootstrapAddress(c *config.Config) (addr network.Address) {
v := config.StringSafe(c.Sub(subsection), "address")
if v == "" {
panic(errAddressNotSet)
}

addr, err := network.AddressFromString(v)
err := addr.FromString(v)
if err != nil {
panic(fmt.Errorf("could not convert bootstrap address %s to %T: %w", v, addr, err))
}
Expand Down
6 changes: 4 additions & 2 deletions cmd/neofs-node/config/node/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@ func TestNodeSection(t *testing.T) {
relay := Relay(c)
wKey := Wallet(c)

expectedAddr, err := network.AddressFromString("s01.neofs.devenv:8080")
var expectedAddr network.Address

err := expectedAddr.FromString("s01.neofs.devenv:8080")
require.NoError(t, err)

require.Equal(t, "NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM", key.Address())
require.Equal(t, true, addr.Equal(*expectedAddr))
require.Equal(t, true, addr.Equal(expectedAddr))
require.Equal(t, true, relay)

require.Len(t, attributes, 2)
Expand Down
8 changes: 5 additions & 3 deletions cmd/neofs-node/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ type remoteLoadAnnounceProvider struct {
loadAddrSrc network.LocalAddressSource

clientCache interface {
Get(*network.Address) (apiClient.Client, error)
Get(network.Address) (apiClient.Client, error)
}

deadEndProvider loadcontroller.WriterProvider
Expand All @@ -218,12 +218,14 @@ func (r *remoteLoadAnnounceProvider) InitRemote(srv loadroute.ServerInfo) (loadc

addr := srv.Address()

netAddr, err := network.AddressFromString(addr)
var netAddr network.Address

err := netAddr.FromString(addr)
if err != nil {
return nil, fmt.Errorf("could not convert address to IP format: %w", err)
}

if network.IsLocalAddress(r.loadAddrSrc, *netAddr) {
if network.IsLocalAddress(r.loadAddrSrc, netAddr) {
// if local => return no-op writer
return loadcontroller.SimpleWriterProvider(new(nopLoadWriter)), nil
}
Expand Down
10 changes: 6 additions & 4 deletions cmd/neofs-node/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ type reputationClientConstructor struct {
trustStorage *truststorage.Storage

basicConstructor interface {
Get(*network.Address) (client.Client, error)
Get(network.Address) (client.Client, error)
}
}

Expand Down Expand Up @@ -492,7 +492,7 @@ func (c *reputationClient) SearchObject(ctx context.Context, prm *client.SearchO
return ids, err
}

func (c *reputationClientConstructor) Get(addr *network.Address) (client.Client, error) {
func (c *reputationClientConstructor) Get(addr network.Address) (client.Client, error) {
cl, err := c.basicConstructor.Get(addr)
if err != nil {
return nil, err
Expand All @@ -501,9 +501,11 @@ func (c *reputationClientConstructor) Get(addr *network.Address) (client.Client,
nm, err := netmap.GetLatestNetworkMap(c.nmSrc)
if err == nil {
for i := range nm.Nodes {
netAddr, err := network.AddressFromString(nm.Nodes[i].Address())
var netAddr network.Address

err := netAddr.FromString(nm.Nodes[i].Address())
if err == nil {
if netAddr.Equal(*addr) {
if netAddr.Equal(addr) {
prm := truststorage.UpdatePrm{}
prm.SetPeer(reputation.PeerIDFromBytes(nm.Nodes[i].PublicKey()))

Expand Down
8 changes: 5 additions & 3 deletions cmd/neofs-node/reputation/common/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type clientCache interface {
Get(*network.Address) (apiClient.Client, error)
Get(network.Address) (apiClient.Client, error)
}

// clientKeyRemoteProvider must provide remote writer and take into account
Expand Down Expand Up @@ -72,12 +72,14 @@ func (rtp *remoteTrustProvider) InitRemote(srv reputationcommon.ServerInfo) (rep

addr := srv.Address()

netAddr, err := network.AddressFromString(addr)
var netAddr network.Address

err := netAddr.FromString(addr)
if err != nil {
return nil, fmt.Errorf("could not convert address to IP format: %w", err)
}

if network.IsLocalAddress(rtp.localAddrSrc, *netAddr) {
if network.IsLocalAddress(rtp.localAddrSrc, netAddr) {
// if local => return no-op writer
return trustcontroller.SimpleWriterProvider(new(NopReputationWriter)), nil
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/innerring/processors/audit/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ func (ap *Processor) findStorageGroups(cid *cid.ID, shuffled netmap.Nodes) []*ob
zap.Int("total_tries", ln),
)

netAddr, err := network.AddressFromString(shuffled[i].Address())
var netAddr network.Address

err := netAddr.FromString(shuffled[i].Address())
if err != nil {
log.Warn("can't parse remote address", zap.String("error", err.Error()))

Expand Down
2 changes: 1 addition & 1 deletion pkg/innerring/processors/audit/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type (

// NeoFSClientCache is an interface for cache of neofs RPC clients
NeoFSClientCache interface {
Get(address *network.Address) (SDKClient.Client, error)
Get(address network.Address) (SDKClient.Client, error)
}

TaskManager interface {
Expand Down
16 changes: 11 additions & 5 deletions pkg/innerring/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type (
ClientCache struct {
log *zap.Logger
cache interface {
Get(address *network.Address) (client.Client, error)
Get(address network.Address) (client.Client, error)
CloseAll()
}
key *ecdsa.PrivateKey
Expand All @@ -49,7 +49,7 @@ func newClientCache(p *clientCacheParams) *ClientCache {
}
}

func (c *ClientCache) Get(address *network.Address) (client.Client, error) {
func (c *ClientCache) Get(address network.Address) (client.Client, error) {
// Because cache is used by `ClientCache` exclusively,
// client will always have valid key.
return c.cache.Get(address)
Expand All @@ -75,7 +75,9 @@ func (c *ClientCache) getSG(ctx context.Context, addr *object.Address, nm *netma
getParams.WithAddress(addr)

for _, node := range placement.FlattenNodes(nodes) {
netAddr, err := network.AddressFromString(node.Address())
var netAddr network.Address

err := netAddr.FromString(node.Address())
if err != nil {
c.log.Warn("can't parse remote address",
zap.String("address", node.Address()),
Expand Down Expand Up @@ -137,7 +139,9 @@ func (c *ClientCache) GetHeader(task *audit.Task, node *netmap.Node, id *object.
headParams.WithMainFields()
headParams.WithAddress(objAddress)

netAddr, err := network.AddressFromString(node.Address())
var netAddr network.Address

err := netAddr.FromString(node.Address())
if err != nil {
return nil, fmt.Errorf("can't parse remote address %s: %w", node.Address(), err)
}
Expand Down Expand Up @@ -173,7 +177,9 @@ func (c *ClientCache) GetRangeHash(task *audit.Task, node *netmap.Node, id *obje
rangeParams.WithRangeList(rng)
rangeParams.WithSalt(nil) // it MUST be nil for correct hash concatenation in PDP game

netAddr, err := network.AddressFromString(node.Address())
var netAddr network.Address

err := netAddr.FromString(node.Address())
if err != nil {
return nil, fmt.Errorf("can't parse remote address %s: %w", node.Address(), err)
}
Expand Down
23 changes: 9 additions & 14 deletions pkg/network/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type Address struct {
// LocalAddressSource is an interface of local
// network address container with read access.
type LocalAddressSource interface {
LocalAddress() *Address
LocalAddress() Address
}

// String returns multiaddr string.
Expand Down Expand Up @@ -57,26 +57,21 @@ func (a Address) HostAddr() string {
return host
}

// AddressFromString restores address from a string representation.
// FromString restores Address from a string representation.
//
// Supports MultiAddr and HostAddr strings.
func AddressFromString(s string) (*Address, error) {
ma, err := multiaddr.NewMultiaddr(s)
func (a *Address) FromString(s string) error {
var err error

a.ma, err = multiaddr.NewMultiaddr(s)
if err != nil {
s, err = multiaddrStringFromHostAddr(s)
if err != nil {
return nil, err
}

ma, err = multiaddr.NewMultiaddr(s) // don't want recursion there
if err != nil {
return nil, err
if err == nil {
a.ma, err = multiaddr.NewMultiaddr(s)
}
}

return &Address{
ma: ma,
}, nil
return err
}

// multiaddrStringFromHostAddr converts "localhost:8080" to "/dns4/localhost/tcp/8080"
Expand Down
14 changes: 10 additions & 4 deletions pkg/network/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ func TestAddressFromString(t *testing.T) {
{"[2004:eb1::1]:8080", buildMultiaddr("/ip6/2004:eb1::1/tcp/8080", t)},
}

var addr Address

for _, testcase := range testcases {
addr, err := AddressFromString(testcase.inp)
err := addr.FromString(testcase.inp)
require.NoError(t, err)
require.Equal(t, testcase.exp, addr.ma, testcase.inp)
}
Expand Down Expand Up @@ -68,14 +70,18 @@ func buildMultiaddr(s string, t *testing.T) multiaddr.Multiaddr {
func TestAddress_WriteToNodeInfo(t *testing.T) {
a := "127.0.0.1:8080"

addr, err := AddressFromString(a)
var addr Address

err := addr.FromString(a)
require.NoError(t, err)

var ni netmap.NodeInfo

addr.WriteToNodeInfo(&ni)

restored, err := AddressFromString(ni.Address())
var restored Address

err = restored.FromString(ni.Address())
require.NoError(t, err)
require.True(t, restored.Equal(*addr))
require.True(t, restored.Equal(addr))
}
2 changes: 1 addition & 1 deletion pkg/network/cache/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NewSDKClientCache(opts ...client.Option) *ClientCache {
}

// Get function returns existing client or creates a new one.
func (c *ClientCache) Get(netAddr *network.Address) (client.Client, error) {
func (c *ClientCache) Get(netAddr network.Address) (client.Client, error) {
// multiaddr is used as a key in client cache since
// same host may have different connections(with tls or not),
// therefore, host+port pair is not unique
Expand Down
4 changes: 3 additions & 1 deletion pkg/network/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ func TestAddress_AddTLS(t *testing.T) {

addr.AddTLS()

netAddr, err := AddressFromString(test.want)
var netAddr Address

err := netAddr.FromString(test.want)
require.NoError(t, err)

require.True(t, netAddr.Equal(addr), test.input)
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/object/get/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ func (exec *execCtx) headChild(id *objectSDK.ID) (*object.Object, bool) {
}
}

func (exec execCtx) remoteClient(node *network.Address) (getClient, bool) {
func (exec execCtx) remoteClient(node network.Address) (getClient, bool) {
log := exec.log.With(zap.Stringer("node", node))

c, err := exec.svc.clientCache.get(node)
Expand Down
7 changes: 4 additions & 3 deletions pkg/services/object/get/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (p *testPlacementBuilder) BuildPlacement(addr *objectSDK.Address, _ *netmap
return vs, nil
}

func (c *testClientCache) get(mAddr *network.Address) (getClient, error) {
func (c *testClientCache) get(mAddr network.Address) (getClient, error) {
v, ok := c.clients[mAddr.HostAddr()]
if !ok {
return nil, errors.New("could not construct client")
Expand Down Expand Up @@ -406,8 +406,9 @@ func testNodeMatrix(t testing.TB, dim []int) ([]netmap.Nodes, [][]string) {
strconv.Itoa(60000+j),
)

var err error
na, err := network.AddressFromString(a)
var na network.Address

err := na.FromString(a)
require.NoError(t, err)

as[j] = na.HostAddr()
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/object/get/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"go.uber.org/zap"
)

func (exec *execCtx) processNode(ctx context.Context, addr *network.Address) bool {
func (exec *execCtx) processNode(ctx context.Context, addr network.Address) bool {
log := exec.log.With(zap.Stringer("remote node", addr))

log.Debug("processing node...")
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/object/get/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type cfg struct {
}

clientCache interface {
get(*network.Address) (getClient, error)
get(network.Address) (getClient, error)
}

traverserGenerator interface {
Expand Down Expand Up @@ -93,7 +93,7 @@ func WithLocalStorageEngine(e *engine.StorageEngine) Option {
}

type ClientConstructor interface {
Get(*network.Address) (client.Client, error)
Get(network.Address) (client.Client, error)
}

// WithClientConstructor returns option to set constructor of remote node clients.
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/object/get/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (s *SimpleObjectWriter) Object() *object.Object {
return s.obj.Object()
}

func (c *clientCacheWrapper) get(addr *network.Address) (getClient, error) {
func (c *clientCacheWrapper) get(addr network.Address) (getClient, error) {
clt, err := c.cache.Get(addr)

return &clientWrapper{
Expand Down
Loading

0 comments on commit adbbad0

Please sign in to comment.