Add size check before sync lfs file (#1045)

Co-authored-by: Zhang ZeHua <pulltheflower@163.com>

Author: csg-pr-bot

mirror/lfssyncer/lfs.go

@@ -428,7 +428,7 @@ func (w *LfsSyncWorker) getSyncPointers(
 	var toBeUpdateLfsMetaObjects []database.LfsMetaObject
 	for _, lfsMetaObject := range lfsMetaObjects {
 		objectKey := common.BuildLfsPath(repo.ID, lfsMetaObject.Oid, repo.Migrated)
-		exists, err := w.CheckIfLFSFileExists(ctx, objectKey)
+		exists, err := w.CheckIfLFSFileExists(ctx, objectKey, lfsMetaObject.Size)
 		if err != nil {
 			slog.Error(
 				"failed to check if lfs file exists",
@@ -567,7 +567,7 @@ func (w *LfsSyncWorker) downloadAndUploadLFSFile(
 ) error {
 	var uploadID string
 	objectKey := common.BuildLfsPath(repo.ID, pointer.Oid, repo.Migrated)
-	exists, err := w.CheckIfLFSFileExists(ctx, objectKey)
+	exists, err := w.CheckIfLFSFileExists(ctx, objectKey, pointer.Size)
 	if err != nil {
 		slog.Error(
 			"failed to check if lfs file exists",
@@ -1206,15 +1206,26 @@ func (w *LfsSyncWorker) downloadRange(
 func (w *LfsSyncWorker) CheckIfLFSFileExists(
 	ctx context.Context,
 	objectKey string,
+	size int64,
 ) (bool, error) {
-	_, err := w.ossClient.StatObject(ctx, w.config.S3.Bucket, objectKey, minio.StatObjectOptions{})
+	objInfo, err := w.ossClient.StatObject(ctx, w.config.S3.Bucket, objectKey, minio.StatObjectOptions{})
 	if err != nil {
 		// Check if it's a "not found" error
 		if strings.Contains(err.Error(), "NoSuchKey") || strings.Contains(err.Error(), "not found") {
 			return false, nil
 		}
 		return false, err
 	}
+
+	// Check if the file size matches the expected size
+	if objInfo.Size != size {
+		// Delete the mismatched object
+		if err := w.ossClient.RemoveObject(ctx, w.config.S3.Bucket, objectKey, minio.RemoveObjectOptions{}); err != nil {
+			return false, fmt.Errorf("failed to remove mismatched object: %w", err)
+		}
+		return false, nil
+	}
+
 	return true, nil
 }
 

mirror/lfssyncer/lfs_test.go

@@ -544,29 +544,60 @@ func (suite *LfsSyncWorkerTestSuite) TestGetSyncPointers() {
 }
 
 func (suite *LfsSyncWorkerTestSuite) TestCheckIfLFSFileExists() {
-	objectKey := "test-object-key"
+	oid := "test-object-oid"
 
 	tests := []struct {
 		name           string
 		statError      error
+		objectSize     int64
+		expectedSize   int64
+		expectDelete   bool
+		deleteError    error
 		expectedExists bool
 		expectedError  bool
 	}{
 		{
-			name:           "file exists",
+			name:           "file exists and size matches",
 			statError:      nil,
+			objectSize:     1024,
+			expectedSize:   1024,
+			expectDelete:   false,
 			expectedExists: true,
 			expectedError:  false,
 		},
+		{
+			name:           "file exists but size mismatch",
+			statError:      nil,
+			objectSize:     2048,
+			expectedSize:   1024,
+			expectDelete:   true,
+			deleteError:    nil,
+			expectedExists: false,
+			expectedError:  false,
+		},
+		{
+			name:           "file exists but size mismatch with delete error",
+			statError:      nil,
+			objectSize:     2048,
+			expectedSize:   1024,
+			expectDelete:   true,
+			deleteError:    errors.New("delete error"),
+			expectedExists: false,
+			expectedError:  true,
+		},
 		{
 			name:           "file not found",
 			statError:      errors.New("NoSuchKey"),
+			expectedSize:   1024,
+			expectDelete:   false,
 			expectedExists: false,
 			expectedError:  false,
 		},
 		{
 			name:           "other error",
 			statError:      errors.New("network error"),
+			expectedSize:   1024,
+			expectDelete:   false,
 			expectedExists: false,
 			expectedError:  true,
 		},
@@ -578,12 +609,15 @@ func (suite *LfsSyncWorkerTestSuite) TestCheckIfLFSFileExists() {
 			worker, mocks := newTestLfsSyncWorker(t, "")
 
 			if tt.statError == nil {
-				mocks.ossClient.EXPECT().StatObject(suite.ctx, worker.config.S3.Bucket, objectKey, mock.Anything).Return(minio.ObjectInfo{}, nil)
+				mocks.ossClient.EXPECT().StatObject(suite.ctx, worker.config.S3.Bucket, oid, mock.Anything).Return(minio.ObjectInfo{Size: tt.objectSize}, nil)
+				if tt.expectDelete {
+					mocks.ossClient.EXPECT().RemoveObject(suite.ctx, worker.config.S3.Bucket, oid, mock.Anything).Return(tt.deleteError)
+				}
 			} else {
-				mocks.ossClient.EXPECT().StatObject(suite.ctx, worker.config.S3.Bucket, objectKey, mock.Anything).Return(minio.ObjectInfo{}, tt.statError)
+				mocks.ossClient.EXPECT().StatObject(suite.ctx, worker.config.S3.Bucket, oid, mock.Anything).Return(minio.ObjectInfo{}, tt.statError)
 			}
 
-			exists, err := worker.CheckIfLFSFileExists(suite.ctx, objectKey)
+			exists, err := worker.CheckIfLFSFileExists(suite.ctx, oid, tt.expectedSize)
 
 			if tt.expectedError {
 				assert.Error(t, err)