Add Validation ClientSeq is Sequential With DocInfo.Checkpoint.ClientSeq

yorkie-team · Aug 11, 2024 · 511f8f1 · 511f8f1
1 parent b2472f4
commit 511f8f1
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 26 deletions.
diff --git a/server/packs/packs.go b/server/packs/packs.go
@@ -39,8 +39,11 @@ import (
 )
 
 var (
-	// ErrClientSeqNotSequential is returned when ClientSeq in reqPack.Changes are not sequential
-	ErrClientSeqNotSequential = errors.New("ClientSeq in reqPack.Changes are not sequential")
+	// ErrClientSeqNotSequentialWithCheckpoint is returned when ClientSeq in reqPack are not sequential With DocInfo.Checkpoint.ClientSeq
+	ErrClientSeqNotSequentialWithCheckpoint = errors.New("ClientSeq is not sequential with DocInfo.Checkpoint.ClientSeq")
+
+	// ErrClientSeqInChangesAreNotSequential is returned when ClientSeq in reqPack.Changes are not sequential
+	ErrClientSeqInChangesAreNotSequential = errors.New("ClientSeq in reqPack.Changes are not sequential")
 )
 
 // PushPullKey creates a new sync.Key of PushPull for the given document.
@@ -83,7 +86,8 @@ func PushPull(
 
 	// TODO: Changes may be reordered or missing during communication on the network.
 	// We should check the change.pack with checkpoint to make sure the changes are in the correct order.
-	err := validateClientSeqSequential(reqPack.Changes)
+	checkpoint := clientInfo.Checkpoint(docInfo.ID)
+	err := validateClientSeqSequential(reqPack.Changes, checkpoint)
 	if err != nil {
 		return nil, err
 	}
@@ -284,11 +288,34 @@ func BuildDocumentForServerSeq(
 	return doc, nil
 }
 
-func validateClientSeqSequential(changes []*change.Change) error {
-	if len(changes) <= 1 {
+func validateClientSeqSequential(changes []*change.Change, checkpoint change.Checkpoint) error {
+	if len(changes) < 1 {
 		return nil
 	}
 
+	if err := validateClientSeqSequentialWithCheckpoint(changes, checkpoint); err != nil {
+		return err
+	}
+
+	return validateClientSeqInChangesAreSequential(changes)
+}
+
+func validateClientSeqSequentialWithCheckpoint(changes []*change.Change, checkpoint change.Checkpoint) error {
+	expectedClientSeq := checkpoint.ClientSeq + 1
+	actualFirstClientSeq := changes[0].ClientSeq()
+
+	if expectedClientSeq != actualFirstClientSeq {
+		return fmt.Errorf(
+			"ClientSeq is not sequential with DocInfo.Checkpoint.ClientSeq (expected: %d, actual: %d) : %w",
+			expectedClientSeq,
+			actualFirstClientSeq,
+			ErrClientSeqNotSequentialWithCheckpoint,
+		)
+	}
+	return nil
+}
+
+func validateClientSeqInChangesAreSequential(changes []*change.Change) error {
 	nextClientSeq := changes[0].ClientSeq()
 	for _, cn := range changes[1:] {
 		nextClientSeq++
@@ -298,7 +325,7 @@ func validateClientSeqSequential(changes []*change.Change) error {
 				"ClientSeq in Changes are not sequential (expected: %d, actual: %d) : %w",
 				nextClientSeq,
 				cn.ClientSeq(),
-				ErrClientSeqNotSequential,
+				ErrClientSeqInChangesAreNotSequential,
 			)
 		}
 	}

diff --git a/server/rpc/connecthelper/status.go b/server/rpc/connecthelper/status.go
@@ -39,16 +39,17 @@ import (
 // errorToConnectCode maps an error to connectRPC status code.
 var errorToConnectCode = map[error]connect.Code{
 	// InvalidArgument means the request is malformed.
-	converter.ErrPackRequired:       connect.CodeInvalidArgument,
-	converter.ErrCheckpointRequired: connect.CodeInvalidArgument,
-	time.ErrInvalidHexString:        connect.CodeInvalidArgument,
-	time.ErrInvalidActorID:          connect.CodeInvalidArgument,
-	types.ErrInvalidID:              connect.CodeInvalidArgument,
-	clients.ErrInvalidClientID:      connect.CodeInvalidArgument,
-	clients.ErrInvalidClientKey:     connect.CodeInvalidArgument,
-	key.ErrInvalidKey:               connect.CodeInvalidArgument,
-	types.ErrEmptyProjectFields:     connect.CodeInvalidArgument,
-	packs.ErrClientSeqNotSequential: connect.CodeInvalidArgument,
+	converter.ErrPackRequired:                     connect.CodeInvalidArgument,
+	converter.ErrCheckpointRequired:               connect.CodeInvalidArgument,
+	time.ErrInvalidHexString:                      connect.CodeInvalidArgument,
+	time.ErrInvalidActorID:                        connect.CodeInvalidArgument,
+	types.ErrInvalidID:                            connect.CodeInvalidArgument,
+	clients.ErrInvalidClientID:                    connect.CodeInvalidArgument,
+	clients.ErrInvalidClientKey:                   connect.CodeInvalidArgument,
+	key.ErrInvalidKey:                             connect.CodeInvalidArgument,
+	types.ErrEmptyProjectFields:                   connect.CodeInvalidArgument,
+	packs.ErrClientSeqNotSequentialWithCheckpoint: connect.CodeInvalidArgument,
+	packs.ErrClientSeqInChangesAreNotSequential:   connect.CodeInvalidArgument,
 
 	// NotFound means the requested resource does not exist.
 	database.ErrProjectNotFound:  connect.CodeNotFound,
@@ -92,16 +93,17 @@ var errorToConnectCode = map[error]connect.Code{
 // TODO(hackerwins): We need to add codes by hand for each error. It would be
 // better to generate this map automatically.
 var errorToCode = map[error]string{
-	converter.ErrPackRequired:       "ErrPackRequired",
-	converter.ErrCheckpointRequired: "ErrCheckpointRequired",
-	time.ErrInvalidHexString:        "ErrInvalidHexString",
-	time.ErrInvalidActorID:          "ErrInvalidActorID",
-	types.ErrInvalidID:              "ErrInvalidID",
-	clients.ErrInvalidClientID:      "ErrInvalidClientID",
-	clients.ErrInvalidClientKey:     "ErrInvalidClientKey",
-	key.ErrInvalidKey:               "ErrInvalidKey",
-	types.ErrEmptyProjectFields:     "ErrEmptyProjectFields",
-	packs.ErrClientSeqNotSequential: "ErrClientSeqNotSequential",
+	converter.ErrPackRequired:                     "ErrPackRequired",
+	converter.ErrCheckpointRequired:               "ErrCheckpointRequired",
+	time.ErrInvalidHexString:                      "ErrInvalidHexString",
+	time.ErrInvalidActorID:                        "ErrInvalidActorID",
+	types.ErrInvalidID:                            "ErrInvalidID",
+	clients.ErrInvalidClientID:                    "ErrInvalidClientID",
+	clients.ErrInvalidClientKey:                   "ErrInvalidClientKey",
+	key.ErrInvalidKey:                             "ErrInvalidKey",
+	types.ErrEmptyProjectFields:                   "ErrEmptyProjectFields",
+	packs.ErrClientSeqNotSequentialWithCheckpoint: "ErrClientSeqNotSequentialWithCheckpoint",
+	packs.ErrClientSeqInChangesAreNotSequential:   "ErrClientSeqInChangesAreNotSequential",
 
 	database.ErrProjectNotFound:  "ErrProjectNotFound",
 	database.ErrClientNotFound:   "ErrClientNotFound",