From 202eb7c1da264a7bf134b32c41c2cd30e0ebcc0e Mon Sep 17 00:00:00 2001 From: Sneha Gunta Date: Thu, 30 Apr 2026 19:19:38 -0400 Subject: [PATCH 1/2] Use LockId and LockToken tiny types in tuple commands Replace raw string with model.LockId and model.LockToken in AcquireLockCommand, FencingCheck, and AcquireLockResult. Validate at the service boundary via NewLockId/NewLockToken; remove redundant DeserializeLockId/DeserializeLockToken calls in the usecase layer. Made-with: Cursor --- internal/biz/usecase/tuples/commands.go | 8 ++-- .../biz/usecase/tuples/tuple_crud_usecase.go | 9 ++-- .../usecase/tuples/tuple_crud_usecase_test.go | 10 ++--- internal/service/tuples/tuples.go | 41 +++++++++++++++---- internal/service/tuples/tuples_test.go | 27 +++++++----- 5 files changed, 62 insertions(+), 33 deletions(-) diff --git a/internal/biz/usecase/tuples/commands.go b/internal/biz/usecase/tuples/commands.go index 33a90a6dd..0384b353c 100644 --- a/internal/biz/usecase/tuples/commands.go +++ b/internal/biz/usecase/tuples/commands.go @@ -30,13 +30,13 @@ type ReadTuplesCommand struct { // AcquireLockCommand - domain command for acquiring locks (DEPRECATED). // This command exists only for RBAC backward compatibility and will be removed. type AcquireLockCommand struct { - LockId string + LockId model.LockId } // FencingCheck represents distributed locking parameters. type FencingCheck struct { - LockId string - LockToken string + LockId model.LockId + LockToken model.LockToken } // CreateTuplesResult - result for CreateTuples operation. @@ -51,5 +51,5 @@ type DeleteTuplesResult struct { // AcquireLockResult - result for AcquireLock operation. type AcquireLockResult struct { - LockToken string + LockToken model.LockToken } diff --git a/internal/biz/usecase/tuples/tuple_crud_usecase.go b/internal/biz/usecase/tuples/tuple_crud_usecase.go index b4f34023f..eea689adc 100644 --- a/internal/biz/usecase/tuples/tuple_crud_usecase.go +++ b/internal/biz/usecase/tuples/tuple_crud_usecase.go @@ -36,7 +36,7 @@ func (uc *TupleCrudUseCase) CreateTuples(ctx context.Context, cmd CreateTuplesCo var fencing *model.FencingCheck if cmd.FencingCheck != nil { - fc := model.NewFencingCheck(model.DeserializeLockId(cmd.FencingCheck.LockId), model.DeserializeLockToken(cmd.FencingCheck.LockToken)) + fc := model.NewFencingCheck(cmd.FencingCheck.LockId, cmd.FencingCheck.LockToken) fencing = &fc } @@ -61,7 +61,7 @@ func (uc *TupleCrudUseCase) DeleteTuples(ctx context.Context, cmd DeleteTuplesCo var fencing *model.FencingCheck if cmd.FencingCheck != nil { - fc := model.NewFencingCheck(model.DeserializeLockId(cmd.FencingCheck.LockId), model.DeserializeLockToken(cmd.FencingCheck.LockToken)) + fc := model.NewFencingCheck(cmd.FencingCheck.LockId, cmd.FencingCheck.LockToken) fencing = &fc } @@ -96,13 +96,12 @@ func (uc *TupleCrudUseCase) AcquireLock(ctx context.Context, cmd AcquireLockComm return nil, err } - lockId := model.DeserializeLockId(cmd.LockId) - result, err := uc.Authz.AcquireLock(ctx, lockId) + result, err := uc.Authz.AcquireLock(ctx, cmd.LockId) if err != nil { return nil, err } return &AcquireLockResult{ - LockToken: result.LockToken().String(), + LockToken: result.LockToken(), }, nil } diff --git a/internal/biz/usecase/tuples/tuple_crud_usecase_test.go b/internal/biz/usecase/tuples/tuple_crud_usecase_test.go index a3c615c1b..9116f9cff 100644 --- a/internal/biz/usecase/tuples/tuple_crud_usecase_test.go +++ b/internal/biz/usecase/tuples/tuple_crud_usecase_test.go @@ -267,7 +267,7 @@ func TestAcquireLock_Success(t *testing.T) { uc := New(&data.AllowAllRelationsRepository{}, meta, log.DefaultLogger) cmd := AcquireLockCommand{ - LockId: "lock-123", + LockId: model.DeserializeLockId("lock-123"), } result, err := uc.AcquireLock(ctx, cmd) @@ -283,7 +283,7 @@ func TestAcquireLock_UsesAcquireLockRelation(t *testing.T) { uc := New(&data.AllowAllRelationsRepository{}, meta, log.DefaultLogger) cmd := AcquireLockCommand{ - LockId: "lock-123", + LockId: model.DeserializeLockId("lock-123"), } _, _ = uc.AcquireLock(ctx, cmd) @@ -298,7 +298,7 @@ func TestAcquireLock_MetaAuthzDenied(t *testing.T) { uc := New(&data.AllowAllRelationsRepository{}, meta, log.DefaultLogger) cmd := AcquireLockCommand{ - LockId: "lock-123", + LockId: model.DeserializeLockId("lock-123"), } _, err := uc.AcquireLock(ctx, cmd) @@ -312,7 +312,7 @@ func TestAcquireLock_MetaAuthzContextMissing(t *testing.T) { uc := New(&data.AllowAllRelationsRepository{}, meta, log.DefaultLogger) cmd := AcquireLockCommand{ - LockId: "lock-123", + LockId: model.DeserializeLockId("lock-123"), } _, err := uc.AcquireLock(context.Background(), cmd) @@ -528,7 +528,7 @@ func TestWhitelistMetaAuthorizer_Integration_AllOperations(t *testing.T) { }) t.Run("AcquireLock", func(t *testing.T) { - cmd := AcquireLockCommand{LockId: "lock-123"} + cmd := AcquireLockCommand{LockId: model.DeserializeLockId("lock-123")} _, err := uc.AcquireLock(ctx, cmd) require.NoError(t, err) }) diff --git a/internal/service/tuples/tuples.go b/internal/service/tuples/tuples.go index 8b2663f7d..b22bf7ca0 100644 --- a/internal/service/tuples/tuples.go +++ b/internal/service/tuples/tuples.go @@ -90,7 +90,10 @@ func (s *TupleService) ReadTuples(req *pb.ReadTuplesRequest, stream pb.KesselTup // AcquireLock acquires a distributed lock (DEPRECATED). // This endpoint exists only for RBAC backward compatibility. func (s *TupleService) AcquireLock(ctx context.Context, req *pb.AcquireLockRequest) (*pb.AcquireLockResponse, error) { - cmd := toAcquireLockCommand(req) + cmd, err := toAcquireLockCommand(req) + if err != nil { + return nil, err + } result, err := s.Ctl.AcquireLock(ctx, cmd) if err != nil { @@ -98,7 +101,7 @@ func (s *TupleService) AcquireLock(ctx context.Context, req *pb.AcquireLockReque } return &pb.AcquireLockResponse{ - LockToken: result.LockToken, + LockToken: result.LockToken.String(), }, nil } @@ -116,9 +119,18 @@ func toCreateTuplesCommand(req *pb.CreateTuplesRequest) (tuplesctl.CreateTuplesC } if req.GetFencingCheck() != nil { + fc := req.GetFencingCheck() + lockId, err := model.NewLockId(fc.GetLockId()) + if err != nil { + return tuplesctl.CreateTuplesCommand{}, fmt.Errorf("invalid fencing lock id: %w", err) + } + lockToken, err := model.NewLockToken(fc.GetLockToken()) + if err != nil { + return tuplesctl.CreateTuplesCommand{}, fmt.Errorf("invalid fencing lock token: %w", err) + } cmd.FencingCheck = &tuplesctl.FencingCheck{ - LockId: req.GetFencingCheck().GetLockId(), - LockToken: req.GetFencingCheck().GetLockToken(), + LockId: lockId, + LockToken: lockToken, } } @@ -131,9 +143,18 @@ func toDeleteTuplesCommand(req *pb.DeleteTuplesRequest) (tuplesctl.DeleteTuplesC } if req.GetFencingCheck() != nil { + fc := req.GetFencingCheck() + lockId, err := model.NewLockId(fc.GetLockId()) + if err != nil { + return tuplesctl.DeleteTuplesCommand{}, fmt.Errorf("invalid fencing lock id: %w", err) + } + lockToken, err := model.NewLockToken(fc.GetLockToken()) + if err != nil { + return tuplesctl.DeleteTuplesCommand{}, fmt.Errorf("invalid fencing lock token: %w", err) + } cmd.FencingCheck = &tuplesctl.FencingCheck{ - LockId: req.GetFencingCheck().GetLockId(), - LockToken: req.GetFencingCheck().GetLockToken(), + LockId: lockId, + LockToken: lockToken, } } @@ -148,10 +169,12 @@ func toReadTuplesCommand(req *pb.ReadTuplesRequest) (tuplesctl.ReadTuplesCommand }, nil } -func toAcquireLockCommand(req *pb.AcquireLockRequest) tuplesctl.AcquireLockCommand { - return tuplesctl.AcquireLockCommand{ - LockId: req.GetLockId(), +func toAcquireLockCommand(req *pb.AcquireLockRequest) (tuplesctl.AcquireLockCommand, error) { + lockId, err := model.NewLockId(req.GetLockId()) + if err != nil { + return tuplesctl.AcquireLockCommand{}, fmt.Errorf("invalid lock id: %w", err) } + return tuplesctl.AcquireLockCommand{LockId: lockId}, nil } // --- proto → domain helpers --- diff --git a/internal/service/tuples/tuples_test.go b/internal/service/tuples/tuples_test.go index a69300eca..b0f5d0502 100644 --- a/internal/service/tuples/tuples_test.go +++ b/internal/service/tuples/tuples_test.go @@ -57,8 +57,10 @@ func TestToCreateTuplesCommand(t *testing.T) { require.NoError(t, err) require.NotNil(t, cmd.FencingCheck) - assert.Equal(t, "lock-1", cmd.FencingCheck.LockId) - assert.Equal(t, "token-1", cmd.FencingCheck.LockToken) + expectedLockId := model.DeserializeLockId("lock-1") + expectedLockToken := model.DeserializeLockToken("token-1") + assert.Equal(t, expectedLockId, cmd.FencingCheck.LockId) + assert.Equal(t, expectedLockToken, cmd.FencingCheck.LockToken) }) t.Run("invalid relationship - bad resource ID", func(t *testing.T) { @@ -127,7 +129,10 @@ func TestToDeleteTuplesCommand(t *testing.T) { require.NoError(t, err) require.NotNil(t, cmd.FencingCheck) - assert.Equal(t, "lock-2", cmd.FencingCheck.LockId) + expectedLockId := model.DeserializeLockId("lock-2") + expectedLockToken := model.DeserializeLockToken("token-2") + assert.Equal(t, expectedLockId, cmd.FencingCheck.LockId) + assert.Equal(t, expectedLockToken, cmd.FencingCheck.LockToken) }) } @@ -171,9 +176,11 @@ func TestToAcquireLockCommand(t *testing.T) { LockId: "lock-123", } - cmd := toAcquireLockCommand(req) + cmd, err := toAcquireLockCommand(req) - assert.Equal(t, "lock-123", cmd.LockId) + require.NoError(t, err) + expectedLockId := model.DeserializeLockId("lock-123") + assert.Equal(t, expectedLockId, cmd.LockId) } func TestRelationshipToRelationsTuple(t *testing.T) { @@ -462,8 +469,8 @@ func TestReadTuplesItemToProto(t *testing.T) { object, model.DeserializeRelation("member"), model.NewSubjectReferenceWithoutRelation(subRes), - "", - "", + model.DeserializeContinuationToken(""), + model.DeserializeConsistencyToken(""), ) result := readTuplesItemToProto(item) @@ -497,8 +504,8 @@ func TestReadTuplesItemToProto(t *testing.T) { object, model.DeserializeRelation("member"), model.NewSubjectReference(subRes, &r), - "", - "", + model.DeserializeContinuationToken(""), + model.DeserializeConsistencyToken(""), ) result := readTuplesItemToProto(item) @@ -524,7 +531,7 @@ func TestReadTuplesItemToProto(t *testing.T) { object, model.DeserializeRelation("member"), model.NewSubjectReferenceWithoutRelation(subRes), - "page-token-abc", + model.DeserializeContinuationToken("page-token-abc"), model.DeserializeConsistencyToken("ct-123"), ) From e17c2a13f6f29a777528986dfcfef0859aa865ee Mon Sep 17 00:00:00 2001 From: Rajagopalan Ranganathan Date: Fri, 1 May 2026 17:39:15 +0100 Subject: [PATCH 2/2] Minor refactor to move the fencing check lockid,lockotken to common method --- internal/service/tuples/tuples.go | 54 +++++++++++++++---------------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/internal/service/tuples/tuples.go b/internal/service/tuples/tuples.go index b22bf7ca0..e6c3e4e46 100644 --- a/internal/service/tuples/tuples.go +++ b/internal/service/tuples/tuples.go @@ -118,21 +118,11 @@ func toCreateTuplesCommand(req *pb.CreateTuplesRequest) (tuplesctl.CreateTuplesC Upsert: req.GetUpsert(), } - if req.GetFencingCheck() != nil { - fc := req.GetFencingCheck() - lockId, err := model.NewLockId(fc.GetLockId()) - if err != nil { - return tuplesctl.CreateTuplesCommand{}, fmt.Errorf("invalid fencing lock id: %w", err) - } - lockToken, err := model.NewLockToken(fc.GetLockToken()) - if err != nil { - return tuplesctl.CreateTuplesCommand{}, fmt.Errorf("invalid fencing lock token: %w", err) - } - cmd.FencingCheck = &tuplesctl.FencingCheck{ - LockId: lockId, - LockToken: lockToken, - } + fc, err := fencingCheckFromProto(req.GetFencingCheck()) + if err != nil { + return tuplesctl.CreateTuplesCommand{}, err } + cmd.FencingCheck = fc return cmd, nil } @@ -142,21 +132,11 @@ func toDeleteTuplesCommand(req *pb.DeleteTuplesRequest) (tuplesctl.DeleteTuplesC Filter: tupleFilterFromProto(req.GetFilter()), } - if req.GetFencingCheck() != nil { - fc := req.GetFencingCheck() - lockId, err := model.NewLockId(fc.GetLockId()) - if err != nil { - return tuplesctl.DeleteTuplesCommand{}, fmt.Errorf("invalid fencing lock id: %w", err) - } - lockToken, err := model.NewLockToken(fc.GetLockToken()) - if err != nil { - return tuplesctl.DeleteTuplesCommand{}, fmt.Errorf("invalid fencing lock token: %w", err) - } - cmd.FencingCheck = &tuplesctl.FencingCheck{ - LockId: lockId, - LockToken: lockToken, - } + fc, err := fencingCheckFromProto(req.GetFencingCheck()) + if err != nil { + return tuplesctl.DeleteTuplesCommand{}, err } + cmd.FencingCheck = fc return cmd, nil } @@ -179,6 +159,24 @@ func toAcquireLockCommand(req *pb.AcquireLockRequest) (tuplesctl.AcquireLockComm // --- proto → domain helpers --- +func fencingCheckFromProto(fc *pb.RelationFencingCheck) (*tuplesctl.FencingCheck, error) { + if fc == nil { + return nil, nil + } + lockId, err := model.NewLockId(fc.GetLockId()) + if err != nil { + return nil, fmt.Errorf("invalid fencing lock id: %w", err) + } + lockToken, err := model.NewLockToken(fc.GetLockToken()) + if err != nil { + return nil, fmt.Errorf("invalid fencing lock token: %w", err) + } + return &tuplesctl.FencingCheck{ + LockId: lockId, + LockToken: lockToken, + }, nil +} + func relationshipsToRelationsTuples(rels []*pb.Relationship) ([]model.RelationsTuple, error) { tuples := make([]model.RelationsTuple, len(rels)) for i, rel := range rels {