Fix metadata, add helper generator

[Bolodya1997]

Issues

1. metadata server doesn't pass error from the next.Server() on request
2. metadata provides raw sync.Map with no types

Solution

1. Fix, add tests
2. Create metadata helper generator:

gen.go:

package connect

//go:generate bash -c "genny -in=$(go list -f '{{.Dir}} github.com/networkservicemesh/sdk/pkg/tools/metadatahelper)/meta_data.template.go -out=client_meta_data.gen.go -pkg=$GOPACKAGE gen 'prefix=client valueType=networkservice.NetworkServiceClient'"

client_meta_data.gen.go:

// This file was automatically generated by genny.
// Any changes will be lost if this file is regenerated.
// see https://github.com/cheekybits/genny

package connect

import (
	"context"
	"sync"

	"github.com/networkservicemesh/api/pkg/api/networkservice"
	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata"
)

type _ClientKeyType struct{}

type clientMetadataHelper struct {
	m *sync.Map
}

func clientMetadata(ctx context.Context, isClient bool) *clientMetadataHelper {
	return &clientMetadataHelper{
		m: metadata.Map(ctx, isClient),
	}
}

func (h *clientMetadataHelper) Store(value networkservice.NetworkServiceClient) {
	h.m.Store(_ClientKeyType{}, value)
}

func (h *clientMetadataHelper) LoadOrStore(value networkservice.NetworkServiceClient) (networkservice.NetworkServiceClient, bool) {
	raw, ok := h.m.LoadOrStore(_ClientKeyType{}, value)
	return raw.(networkservice.NetworkServiceClient), ok
}

func (h *clientMetadataHelper) Load() (networkservice.NetworkServiceClient, bool) {
	if raw, ok := h.m.Load(_ClientKeyType{}); ok {
		return raw.(networkservice.NetworkServiceClient), true
	}
	return func() (v networkservice.NetworkServiceClient) { return }(), false
}

func (h *clientMetadataHelper) LoadAndDelete() (networkservice.NetworkServiceClient, bool) {
	if raw, ok := h.m.LoadAndDelete(_ClientKeyType{}); ok {
		return raw.(networkservice.NetworkServiceClient), true
	}
	return func() (v networkservice.NetworkServiceClient) { return }(), false
}

func (h *clientMetadataHelper) Delete() {
	h.m.Delete(_ClientKeyType{})
}

[edwarnicke]

Good catch!

[edwarnicke]

Although... it should always be true that the conn..GetId() below is the same as the request.GetConnection().GetId() ... its good defensive coding what you've done here, but an issue here would have been a sign of a bug in updatetoken ...

[edwarnicke]

Is there a way to reference the meta_data_template.go file that doesn't involve file system? I ask, because this is all fine for using metadata inside the sdk repo... but would not work at all from the sdk-vpp repo...

[Bolodya1997]

I have added scripts/genny.sh, so now we can get template by package + file name:

//go:generate bash ../../../scripts/genny.sh -in=github.com/networkservicemesh/sdk/pkg/tools/metadatahelper/meta_data.template.go -out=int_meta_data.gen.go gen "prefix=Int valueType=int"

such script can be copied to other repositories.

UPD: we can use go list -f '{{.Dir}}' package-name to find imported package in file system. And it is actually imported, because we are dependent on module containing such package.

//go:generate bash -c "genny -in=$(go list -f '{{.Dir}} github.com/networkservicemesh/sdk/pkg/tools/metadatahelper)/meta_data.template.go -out=client_meta_data.gen.go -pkg=$GOPACKAGE gen 'prefix=client valueType=networkservice.NetworkServiceClient'"

[edwarnicke]

Clever solution :)

[denis-tingaikin]

I very like the idea to use genny :)

But I feel we still able to not use it because actually, we can solve this via exist sync map generator.

Example

import (
	"sync"
	"testing"
	"github.com/networkservicemesh/api/pkg/api/registry"
	"github.com/networkservicemesh/sdk/pkg/registry/memory"
	"github.com/stretchr/testify/require"
)
func TestCastSyncMapToTypedMap(t *testing.T) {
	var metaDataMap sync.Map // == metadata.Map(...)
	metaDataMap.Store("key", &registry.NetworkServiceEndpoint{})
	memoryMap := (*memory.NetworkServiceEndpointSyncMap)(&metaDataMap)
	val, _ := memoryMap.Load("key")
	require.NotNil(t, val)
}

Note: by registry.NetworkServiceEndpoint can be used any other generated syncmap :)

[edwarnicke]

I very like the idea to use genny :)

But I feel we still able to not use it because actually, we can solve this via exist sync map generator.

Example

import (
	"sync"
	"testing"
	"github.com/networkservicemesh/api/pkg/api/registry"
	"github.com/networkservicemesh/sdk/pkg/registry/memory"
	"github.com/stretchr/testify/require"
)
func TestCastSyncMapToTypedMap(t *testing.T) {
	var metaDataMap sync.Map // == metadata.Map(...)
	metaDataMap.Store("key", &registry.NetworkServiceEndpoint{})
	memoryMap := (*memory.NetworkServiceEndpointSyncMap)(&metaDataMap)
	val, _ := memoryMap.Load("key")
	require.NotNil(t, val)
}

Note: by registry.NetworkServiceEndpoint can be used any other generated syncmap :)

I like the idea of using a simpler approach via syncmap... but don't quite follow your proposal above...

[denis-tingaikin]

@edwarnicke I guess we can try to use generated sync maps to cast map from metadata to typed sync.Map (look at code suggestion #612 (review))
In this case we do not need add genny.

[Bolodya1997]

@edwarnicke
We have discussed this internally with @denis-tingajkin and @haiodo and probably such helper is too much for the cases we want to cover with it:

• as long as per-connection metadata is being cleaned up with on connection close, we don't actually need LoadAndDelete and Delete methods;
• for the Store and LoadOrStore methods we most probably need only one of them, but not both.

Here is the thing we need to use in chain element:

type keyType struct{}

func storeConnInfo(ctx context.Context, connInfo *connectionInfo) {
	metadata.Map(ctx, false).Store(keyType{}, connInfo)
}

func loadConnInfo(ctx context.Context) (*connectionInfo, bool) {
	if raw, ok := metadata.Map(ctx, false).Load(keyType{}); ok {
		return raw.(*connectionInfo), true
	}
	return nil, false
}

and we don't want to create a template for it, because it is too small and making a template would affect our ability to give good names for such Store/Load methods.

So I am closing this PR and opening another one to fix issues, add testing and add metadata chain elements into the client, endpoint chains #621.