Make updatepath restore path index on return

pkg/networkservice/common/interpose/server_test.go

@@ -54,8 +54,6 @@ func TestInterposeServer(t *testing.T) {
 	client := next.NewNetworkServiceClient(
 		updatepath.NewClient("client"),
 		adapters.NewServerToClient(next.NewNetworkServiceServer(
-			// actual `connect.NewServer()` should not update `request.Connection.Path.PathIndex`
-			new(restorePathServer),
 			updatepath.NewServer("nsmgr"),
 			clienturl.NewServer(&nseURL),
 			interposeServer,
@@ -66,11 +64,9 @@ func TestInterposeServer(t *testing.T) {
 			}),
 		)),
 		adapters.NewServerToClient(next.NewNetworkServiceServer(
-			new(restorePathServer),
 			updatepath.NewServer("interpose-nse"),
 		)),
 		adapters.NewServerToClient(next.NewNetworkServiceServer(
-			new(restorePathServer),
 			updatepath.NewServer("nsmgr"),
 			clienturl.NewServer(&nseURL),
 			interposeServer,
@@ -81,7 +77,6 @@ func TestInterposeServer(t *testing.T) {
 			}),
 		)),
 		adapters.NewServerToClient(next.NewNetworkServiceServer(
-			new(restorePathServer),
 			updatepath.NewServer("endpoint"),
 			touchServer,
 		)),
@@ -119,22 +114,6 @@ func TestInterposeServer(t *testing.T) {
 	require.True(t, touchServer.touched)
 }
 
-type restorePathServer struct{}
-
-func (s *restorePathServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
-	pathIndex := request.Connection.Path.Index
-	conn, err := next.Server(ctx).Request(ctx, request)
-	request.Connection.Path.Index = pathIndex
-	return conn, err
-}
-
-func (s *restorePathServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) {
-	pathIndex := conn.Path.Index
-	_, err := next.Server(ctx).Close(ctx, conn)
-	conn.Path.Index = pathIndex
-	return &empty.Empty{}, err
-}
-
 type touchServer struct {
 	touched bool
 }

pkg/networkservice/common/updatepath/client.go

@@ -44,15 +44,24 @@ func (i *updatePathClient) Request(ctx context.Context, request *networkservice.
 		request.Connection = &networkservice.Connection{}
 	}
 
-	request.Connection, err = updatePath(request.Connection, i.name)
+	var index uint32
+	request.Connection, index, err = updatePath(request.Connection, i.name)
 	if err != nil {
 		return nil, err
 	}
-	return next.Client(ctx).Request(ctx, request, opts...)
+
+	conn, err = next.Client(ctx).Request(ctx, request, opts...)
+	if err != nil {
+		return nil, err
+	}
+
+	conn.Path.Index = index
+
+	return conn, err
 }
 
 func (i *updatePathClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (_ *empty.Empty, err error) {
-	conn, err = updatePath(conn, i.name)
+	conn, _, err = updatePath(conn, i.name)
 	if err != nil {
 		return nil, err
 	}

pkg/networkservice/common/updatepath/client_test.go

@@ -21,13 +21,16 @@ import (
 	"context"
 	"testing"
 
+	"github.com/golang/protobuf/ptypes/empty"
 	"github.com/stretchr/testify/require"
-
-	"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatepath"
+	"google.golang.org/grpc"
 
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
-	"github.com/stretchr/testify/assert"
 	"go.uber.org/goleak"
+
+	"github.com/networkservicemesh/sdk/pkg/networkservice/common/updatepath"
+	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
+	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/checks/checkrequest"
 )
 
 const (
@@ -59,84 +62,147 @@ func request(connectionID string, pathIndex uint32) *networkservice.NetworkServi
 
 func TestNewClient_SetNewConnectionId(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
 	client := updatepath.NewClient("nse-3")
+
 	conn, err := client.Request(context.Background(), request(connectionID, 1))
-	require.NotNil(t, conn)
 	require.NoError(t, err)
+	require.NotNil(t, conn)
 	require.Equal(t, 3, len(conn.Path.PathSegments))
 	require.Equal(t, "nse-3", conn.Path.PathSegments[2].Name)
 	require.NotEqual(t, conn.Id, connectionID)
 }
 
 func TestNewClient_SetNewConnectionId2(t *testing.T) {
 	defer goleak.VerifyNone(t)
-	client := updatepath.NewClient("nse-2")
+
+	client := next.NewNetworkServiceClient(
+		updatepath.NewClient("nse-2"),
+		checkrequest.NewClient(t, func(t *testing.T, request *networkservice.NetworkServiceRequest) {
+			require.Equal(t, 1, int(request.Connection.Path.Index))
+		}),
+	)
+
 	conn, err := client.Request(context.Background(), request(connectionID, 0))
-	require.NotNil(t, conn)
 	require.NoError(t, err)
-	require.Equal(t, 1, int(conn.Path.Index))
+	require.NotNil(t, conn)
 	require.Equal(t, conn.Id, pathSegmentID2)
 }
 
 func TestNewClient_SetNewConnectionIdSecondReq(t *testing.T) {
 	defer goleak.VerifyNone(t)
+
 	client := updatepath.NewClient("nse-1")
+
 	conn, err := client.Request(context.Background(), request(connectionID, 0))
-	assert.NotNil(t, conn)
-	assert.NoError(t, err)
-	assert.Equal(t, conn.Id, pathSegmentID1)
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.Equal(t, conn.Id, pathSegmentID1)
 
 	firstConnID := conn.Id
 
-	conn.GetPath().Index = 0
 	conn, err = client.Request(context.Background(), &networkservice.NetworkServiceRequest{
 		Connection: conn,
 	})
-	assert.NotNil(t, conn)
-	assert.NoError(t, err)
-	assert.Equal(t, firstConnID, conn.Id)
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.Equal(t, firstConnID, conn.Id)
 }
 
 func TestNewClient_PathSegmentNameEqualClientName(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
 	client := updatepath.NewClient("nse-2")
+
 	conn, err := client.Request(context.Background(), request(connectionID, 1))
-	assert.NotNil(t, conn)
-	assert.NoError(t, err)
-	assert.NotEqual(t, conn.Id, connectionID)
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.NotEqual(t, conn.Id, connectionID)
 }
 
 func TestNewClient_PathSegmentIdEqualConnectionId(t *testing.T) {
 	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
 	client := updatepath.NewClient("nse-2")
+
 	conn, err := client.Request(context.Background(), request(pathSegmentID2, 1))
-	assert.NotNil(t, conn)
-	assert.NoError(t, err)
-	assert.Equal(t, conn.Id, pathSegmentID2)
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.Equal(t, conn.Id, pathSegmentID2)
 }
 
 func TestNewClient_PathSegmentNameAndIDEqualClientNameAndID(t *testing.T) {
 	defer goleak.VerifyNone(t)
+
 	client := updatepath.NewClient("nse-2")
+
 	conn, err := client.Request(context.Background(), request(pathSegmentID2, 1))
-	assert.NotNil(t, conn)
-	assert.NoError(t, err)
-	assert.Equal(t, conn.Id, pathSegmentID2)
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.Equal(t, conn.Id, pathSegmentID2)
 }
 
 func TestNewClient_InvalidIndex(t *testing.T) {
 	defer goleak.VerifyNone(t)
+
 	client := updatepath.NewClient("nse-3")
+
 	conn, err := client.Request(context.Background(), request(connectionID, 2))
-	require.NotNil(t, err)
+	require.Error(t, err)
 	require.Nil(t, conn)
 }
 
 func TestNewClient_NoConnection(t *testing.T) {
 	defer goleak.VerifyNone(t)
+
 	client := updatepath.NewClient("nse-3")
+
 	conn, err := client.Request(context.Background(), &networkservice.NetworkServiceRequest{})
-	assert.NotNil(t, conn)
-	assert.NoError(t, err)
-	assert.NotEqual(t, conn.Id, connectionID)
+	require.NoError(t, err)
+	require.NotNil(t, conn)
+	require.NotEqual(t, conn.Id, connectionID)
+}
+
+func TestNewClient_RestoreIndex(t *testing.T) {
+	defer goleak.VerifyNone(t, goleak.IgnoreCurrent())
+
+	client := next.NewNetworkServiceClient(
+		updatepath.NewClient("nse-1"),
+		newPathIndexClient(t, 0),
+		updatepath.NewClient("nse-2"),
+		newPathIndexClient(t, 1),
+		updatepath.NewClient("nse-2"),
+		newPathIndexClient(t, 1),
+		updatepath.NewClient("nse-3"),
+		newPathIndexClient(t, 2),
+	)
+
+	_, err := client.Request(context.Background(), request(connectionID, 0))
+	require.NoError(t, err)
+}
+
+type pathIndexClient struct {
+	t     *testing.T
+	index uint32
+}
+
+func newPathIndexClient(t *testing.T, index uint32) *pathIndexClient {
+	return &pathIndexClient{
+		t:     t,
+		index: index,
+	}
+}
+
+func (c *pathIndexClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) {
+	require.Equal(c.t, c.index, request.Connection.Path.Index)
+	conn, err := next.Client(ctx).Request(ctx, request, opts...)
+	if err != nil {
+		return nil, err
+	}
+	require.Equal(c.t, c.index, conn.Path.Index)
+	return conn, nil
+}
+
+func (c *pathIndexClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) {
+	return next.Client(ctx).Close(ctx, conn, opts...)
 }

pkg/networkservice/common/updatepath/common.go

@@ -18,8 +18,9 @@ package updatepath
 
 import (
 	"github.com/google/uuid"
-	"github.com/networkservicemesh/api/pkg/api/networkservice"
 	"github.com/pkg/errors"
+
+	"github.com/networkservicemesh/api/pkg/api/networkservice"
 )
 
 /*
@@ -32,7 +33,7 @@ import (
 		2.3 if path has next segment available, but next name is not equal to segmentName, will return error.
 	3. if Index == 0, and there is no current segment will add one.
 */
-func updatePath(conn *networkservice.Connection, segmentName string) (*networkservice.Connection, error) {
+func updatePath(conn *networkservice.Connection, segmentName string) (*networkservice.Connection, uint32, error) {
 	if conn.Path == nil {
 		conn.Path = &networkservice.Path{}
 	}
@@ -48,23 +49,23 @@ func updatePath(conn *networkservice.Connection, segmentName string) (*networkse
 			Name: segmentName,
 			Id:   conn.Id,
 		})
-		return conn, nil
+		return conn, 0, nil
 	}
 
 	path := conn.GetPath()
 
 	if int(path.Index) < len(path.PathSegments) && path.PathSegments[path.Index].Name == segmentName {
 		// We already in current segment, just update connection Id, no need to increment
 		conn.Id = path.PathSegments[path.Index].Id
-		return conn, nil
+		return conn, path.Index, nil
 	}
 
 	// We need to move to next item
 	nextIndex := int(path.Index) + 1
 
 	if nextIndex > len(path.GetPathSegments()) {
 		// We have index > segments count
-		return nil, errors.Errorf("NetworkServiceRequest.Connection.Path.Index+1==%d should be less or equal len(NetworkServiceRequest.Connection.Path.PathSegement)==%d",
+		return nil, 0, errors.Errorf("NetworkServiceRequest.Connection.Path.Index+1==%d should be less or equal len(NetworkServiceRequest.Connection.Path.PathSegement)==%d",
 			nextIndex, len(path.GetPathSegments()))
 	}
 	if nextIndex < len(path.GetPathSegments()) && path.GetPathSegments()[nextIndex].Name != segmentName {
@@ -88,5 +89,5 @@ func updatePath(conn *networkservice.Connection, segmentName string) (*networkse
 		conn.Id = path.GetPathSegments()[conn.Path.Index].Id
 	}
 
-	return conn, nil
+	return conn, path.Index - 1, nil
 }

pkg/networkservice/common/updatepath/server.go

@@ -21,6 +21,7 @@ import (
 	"context"
 
 	"github.com/golang/protobuf/ptypes/empty"
+
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
 
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
@@ -38,19 +39,29 @@ func NewServer(name string) networkservice.NetworkServiceServer {
 	return &updatePathServer{name: name}
 }
 
-func (i *updatePathServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (_ *networkservice.Connection, err error) {
+func (i *updatePathServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (conn *networkservice.Connection, err error) {
 	if request.Connection == nil {
 		request.Connection = &networkservice.Connection{}
 	}
-	request.Connection, err = updatePath(request.Connection, i.name)
+
+	var index uint32
+	request.Connection, index, err = updatePath(request.Connection, i.name)
+	if err != nil {
+		return nil, err
+	}
+
+	conn, err = next.Server(ctx).Request(ctx, request)
 	if err != nil {
 		return nil, err
 	}
-	return next.Server(ctx).Request(ctx, request)
+
+	conn.Path.Index = index
+
+	return conn, err
 }
 
 func (i *updatePathServer) Close(ctx context.Context, conn *networkservice.Connection) (_ *empty.Empty, err error) {
-	conn, err = updatePath(conn, i.name)
+	conn, _, err = updatePath(conn, i.name)
 	if err != nil {
 		return nil, err
 	}