diff --git a/api/src/main/java/com/nextdocs/api/document/controller/DocumentController.java b/api/src/main/java/com/nextdocs/api/document/controller/DocumentController.java index d1a2645..8bc2a48 100644 --- a/api/src/main/java/com/nextdocs/api/document/controller/DocumentController.java +++ b/api/src/main/java/com/nextdocs/api/document/controller/DocumentController.java @@ -3,10 +3,8 @@ import com.nextdocs.api.auth.security.UserPrincipal; import com.nextdocs.api.common.response.ApiResponse; import com.nextdocs.api.common.response.PagedResponse; -import com.nextdocs.api.document.dto.request.BulkImportRequest; import com.nextdocs.api.document.dto.request.DocumentCreateRequest; import com.nextdocs.api.document.dto.request.DocumentUpdateRequest; -import com.nextdocs.api.document.dto.response.BulkImportResponse; import com.nextdocs.api.document.dto.response.DocumentResponse; import com.nextdocs.api.document.service.DocumentService; import io.swagger.v3.oas.annotations.Operation; @@ -37,6 +35,9 @@ public class DocumentController { summary = "Create a document", description = "Creates a new document owned by the authenticated user.", responses = { + @io.swagger.v3.oas.annotations.responses.ApiResponse( + responseCode = "200", + description = "Document already existed for the authenticated user"), @io.swagger.v3.oas.annotations.responses.ApiResponse( responseCode = "201", description = "Document created"), @@ -50,8 +51,10 @@ public class DocumentController { @PostMapping public ResponseEntity> create( @AuthenticationPrincipal UserPrincipal principal, @Valid @RequestBody DocumentCreateRequest request) { - DocumentResponse response = documentService.create(principal.getId(), request); - return ResponseEntity.status(HttpStatus.CREATED).body(ApiResponse.ok(response, "Document created.")); + DocumentService.CreateDocumentResult result = documentService.create(principal.getId(), request); + HttpStatus status = result.created() ? HttpStatus.CREATED : HttpStatus.OK; + String message = result.created() ? "Document created." : "Document already exists."; + return ResponseEntity.status(status).body(ApiResponse.ok(result.document(), message)); } @Operation( @@ -195,26 +198,4 @@ public ResponseEntity> restore( DocumentResponse response = documentService.restore(principal.getId(), id); return ResponseEntity.ok(ApiResponse.ok(response, "Document restored.")); } - - @Operation( - summary = "Bulk import local documents", - description = "Imports local documents into the authenticated user's account. " - + "Operation is transactional: if one document fails validation, none are imported.", - responses = { - @io.swagger.v3.oas.annotations.responses.ApiResponse( - responseCode = "201", - description = "Documents imported"), - @io.swagger.v3.oas.annotations.responses.ApiResponse( - responseCode = "400", - description = "Invalid request payload"), - @io.swagger.v3.oas.annotations.responses.ApiResponse( - responseCode = "401", - description = "Authentication required") - }) - @PostMapping("/bulk-import") - public ResponseEntity> bulkImport( - @AuthenticationPrincipal UserPrincipal principal, @Valid @RequestBody BulkImportRequest request) { - BulkImportResponse response = documentService.bulkImport(principal.getId(), request); - return ResponseEntity.status(HttpStatus.CREATED).body(ApiResponse.ok(response, "Documents imported.")); - } } diff --git a/api/src/main/java/com/nextdocs/api/document/dto/request/BulkImportItemRequest.java b/api/src/main/java/com/nextdocs/api/document/dto/request/BulkImportItemRequest.java deleted file mode 100644 index 6d2b7b5..0000000 --- a/api/src/main/java/com/nextdocs/api/document/dto/request/BulkImportItemRequest.java +++ /dev/null @@ -1,22 +0,0 @@ -package com.nextdocs.api.document.dto.request; - -import io.swagger.v3.oas.annotations.media.Schema; -import jakarta.validation.constraints.NotBlank; -import jakarta.validation.constraints.Size; - -@Schema(description = "Single document payload for bulk import") -public record BulkImportItemRequest( - @Schema(description = "Client-side local document ID", example = "local-123") - String localId, - - @Schema(description = "Document title", example = "Imported Doc") - @NotBlank(message = "Title is required") - @Size(max = 255, message = "Title must be at most 255 characters") - String title, - - @Schema(description = "Base64-encoded Yjs state") @NotBlank(message = "yjsState is required") - String yjsState, - - @Schema(description = "Optional creator label") - @Size(max = 255, message = "createdBy must be at most 255 characters") - String createdBy) {} diff --git a/api/src/main/java/com/nextdocs/api/document/dto/request/BulkImportRequest.java b/api/src/main/java/com/nextdocs/api/document/dto/request/BulkImportRequest.java deleted file mode 100644 index eec17a4..0000000 --- a/api/src/main/java/com/nextdocs/api/document/dto/request/BulkImportRequest.java +++ /dev/null @@ -1,12 +0,0 @@ -package com.nextdocs.api.document.dto.request; - -import io.swagger.v3.oas.annotations.media.Schema; -import jakarta.validation.Valid; -import jakarta.validation.constraints.NotEmpty; -import jakarta.validation.constraints.NotNull; -import java.util.List; - -@Schema(description = "Bulk import request for local documents") -public record BulkImportRequest( - @Schema(description = "Documents to import") @NotEmpty(message = "docs must not be empty") - List<@NotNull @Valid BulkImportItemRequest> docs) {} diff --git a/api/src/main/java/com/nextdocs/api/document/dto/request/DocumentCreateRequest.java b/api/src/main/java/com/nextdocs/api/document/dto/request/DocumentCreateRequest.java index b65de77..59ebc8d 100644 --- a/api/src/main/java/com/nextdocs/api/document/dto/request/DocumentCreateRequest.java +++ b/api/src/main/java/com/nextdocs/api/document/dto/request/DocumentCreateRequest.java @@ -3,9 +3,15 @@ import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.Size; +import java.util.UUID; @Schema(description = "Request body for creating a document") public record DocumentCreateRequest( + @Schema( + description = "Client-generated document ID. Must be a UUID.", + example = "550e8400-e29b-41d4-a716-446655440000") + UUID id, + @Schema(description = "Document title", example = "My First Doc") @NotBlank(message = "Title is required") @Size(max = 255, message = "Title must be at most 255 characters") @@ -18,8 +24,4 @@ public record DocumentCreateRequest( @Schema(description = "Optional creator label", example = "Anonymous") @Size(max = 255, message = "createdBy must be at most 255 characters") - String createdBy, - - @Schema(description = "Optional local source ID for idempotent import/promotion", example = "local-123") - @Size(max = 128, message = "sourceLocalId must be at most 128 characters") - String sourceLocalId) {} + String createdBy) {} diff --git a/api/src/main/java/com/nextdocs/api/document/dto/response/BulkImportItemResponse.java b/api/src/main/java/com/nextdocs/api/document/dto/response/BulkImportItemResponse.java deleted file mode 100644 index 79e7fa2..0000000 --- a/api/src/main/java/com/nextdocs/api/document/dto/response/BulkImportItemResponse.java +++ /dev/null @@ -1,10 +0,0 @@ -package com.nextdocs.api.document.dto.response; - -import io.swagger.v3.oas.annotations.media.Schema; -import java.util.UUID; - -@Schema(description = "Single imported document mapping") -public record BulkImportItemResponse( - @Schema(description = "Client local ID") String localId, - @Schema(description = "Server document ID") UUID documentId, - @Schema(description = "Imported title") String title) {} diff --git a/api/src/main/java/com/nextdocs/api/document/dto/response/BulkImportResponse.java b/api/src/main/java/com/nextdocs/api/document/dto/response/BulkImportResponse.java deleted file mode 100644 index 4049a01..0000000 --- a/api/src/main/java/com/nextdocs/api/document/dto/response/BulkImportResponse.java +++ /dev/null @@ -1,8 +0,0 @@ -package com.nextdocs.api.document.dto.response; - -import io.swagger.v3.oas.annotations.media.Schema; -import java.util.List; - -@Schema(description = "Bulk import result") -public record BulkImportResponse( - @Schema(description = "Imported documents") List imported) {} diff --git a/api/src/main/java/com/nextdocs/api/document/entity/Document.java b/api/src/main/java/com/nextdocs/api/document/entity/Document.java index b5e0845..ef26c42 100644 --- a/api/src/main/java/com/nextdocs/api/document/entity/Document.java +++ b/api/src/main/java/com/nextdocs/api/document/entity/Document.java @@ -15,8 +15,7 @@ name = "documents", indexes = { @Index(name = "idx_documents_user_created", columnList = "user_id,created_at"), - @Index(name = "idx_documents_user_updated", columnList = "user_id,updated_at"), - @Index(name = "idx_documents_user_source_local", columnList = "user_id,source_local_id") + @Index(name = "idx_documents_user_updated", columnList = "user_id,updated_at") }) @Getter @Setter @@ -26,7 +25,6 @@ public class Document { @Id - @GeneratedValue(strategy = GenerationType.UUID) private UUID id; @ManyToOne(fetch = FetchType.LAZY, optional = false) @@ -43,9 +41,6 @@ public class Document { @Column(name = "created_by", length = 255) private String createdBy; - @Column(name = "source_local_id", length = 128) - private String sourceLocalId; - @Column(name = "deleted_at") private OffsetDateTime deletedAt; diff --git a/api/src/main/java/com/nextdocs/api/document/repository/DocumentRepository.java b/api/src/main/java/com/nextdocs/api/document/repository/DocumentRepository.java index 9c9d9e6..d13cff3 100644 --- a/api/src/main/java/com/nextdocs/api/document/repository/DocumentRepository.java +++ b/api/src/main/java/com/nextdocs/api/document/repository/DocumentRepository.java @@ -2,7 +2,6 @@ import com.nextdocs.api.document.entity.Document; import java.time.OffsetDateTime; -import java.util.List; import java.util.Optional; import java.util.UUID; import org.springframework.data.domain.Page; @@ -20,10 +19,6 @@ public interface DocumentRepository extends JpaRepository { Page findAllByUser_IdAndDeletedAtIsNotNull(UUID userId, Pageable pageable); - List findAllByUser_IdAndSourceLocalIdInAndDeletedAtIsNull(UUID userId, List sourceLocalIds); - - Optional findByUser_IdAndSourceLocalIdAndDeletedAtIsNull(UUID userId, String sourceLocalId); - Optional findByIdAndUser_IdAndDeletedAtIsNull(UUID id, UUID userId); Optional findByIdAndUser_Id(UUID id, UUID userId); diff --git a/api/src/main/java/com/nextdocs/api/document/service/DocumentService.java b/api/src/main/java/com/nextdocs/api/document/service/DocumentService.java index 2ef3422..1ca3a4d 100644 --- a/api/src/main/java/com/nextdocs/api/document/service/DocumentService.java +++ b/api/src/main/java/com/nextdocs/api/document/service/DocumentService.java @@ -5,8 +5,9 @@ import com.nextdocs.api.common.exception.ApiException; import com.nextdocs.api.common.exception.ErrorCode; import com.nextdocs.api.document.config.DocumentProperties; -import com.nextdocs.api.document.dto.request.*; -import com.nextdocs.api.document.dto.response.*; +import com.nextdocs.api.document.dto.request.DocumentCreateRequest; +import com.nextdocs.api.document.dto.request.DocumentUpdateRequest; +import com.nextdocs.api.document.dto.response.DocumentResponse; import com.nextdocs.api.document.entity.Document; import com.nextdocs.api.document.entity.DocumentAccessLevel; import com.nextdocs.api.document.entity.DocumentCollaborator; @@ -16,9 +17,6 @@ import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.Base64; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import java.util.UUID; import lombok.RequiredArgsConstructor; import org.springframework.beans.factory.annotation.Autowired; @@ -29,13 +27,14 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.stereotype.Service; -import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; @Service @RequiredArgsConstructor public class DocumentService { + public record CreateDocumentResult(DocumentResponse document, boolean created) {} + private final DocumentRepository documentRepository; private final DocumentCollaboratorRepository collaboratorRepository; private final UserRepository userRepository; @@ -46,48 +45,51 @@ public class DocumentService { private DocumentService selfProxy; @Transactional - public DocumentResponse create(UUID userId, DocumentCreateRequest request) { + public CreateDocumentResult create(UUID userId, DocumentCreateRequest request) { User user = userRepository.findById(userId).orElseThrow(() -> new ApiException(ErrorCode.NOT_FOUND)); - String sourceLocalId = normalizeSourceLocalId(request.sourceLocalId()); String yjsState = request.yjsState(); - if (yjsState == null) { - throw new ApiException(ErrorCode.VALIDATION_FAILED, "yjsState is required."); - } - - if (sourceLocalId != null) { - Document existing = documentRepository - .findByUser_IdAndSourceLocalIdAndDeletedAtIsNull(userId, sourceLocalId) - .orElse(null); + UUID documentId = request.id() != null ? request.id() : UUID.randomUUID(); + if (request.id() != null) { + Document existing = + documentRepository.findByIdAndUser_Id(documentId, userId).orElse(null); if (existing != null) { - applyFields(existing, user, request.title(), yjsState, request.createdBy(), sourceLocalId); - return toResponse(documentRepository.save(existing), true); + if (existing.getDeletedAt() != null) { + throw new ApiException( + ErrorCode.CONFLICT, + "A trashed document already exists with this ID. Restore or permanently delete it first."); + } + return new CreateDocumentResult(toResponse(existing, true), false); } } Document document = Document.builder() + .id(documentId) .user(user) .title(normalizeTitle(request.title())) .yjsState(decodeBase64State(yjsState)) .createdBy(request.createdBy()) - .sourceLocalId(sourceLocalId) .build(); - if (sourceLocalId != null) { - try { - return toResponse(documentRepository.saveAndFlush(document), true); - } catch (DataIntegrityViolationException ex) { - Document existing = documentRepository - .findByUser_IdAndSourceLocalIdAndDeletedAtIsNull(userId, sourceLocalId) - .orElseThrow(() -> ex); + try { + return new CreateDocumentResult(toResponse(documentRepository.saveAndFlush(document), true), true); + } catch (DataIntegrityViolationException ex) { + if (request.id() == null) { + throw ex; + } - applyFields(existing, user, request.title(), yjsState, request.createdBy(), sourceLocalId); - return toResponse(documentRepository.save(existing), true); + Document existing = documentRepository.findById(documentId).orElseThrow(() -> ex); + if (!existing.getUser().getId().equals(userId)) { + throw new ApiException(ErrorCode.CONFLICT, "A document already exists with this ID."); } + if (existing.getDeletedAt() != null) { + throw new ApiException( + ErrorCode.CONFLICT, + "A trashed document already exists with this ID. Restore or permanently delete it first."); + } + return new CreateDocumentResult(toResponse(existing, true), false); } - - return toResponse(documentRepository.save(document), true); } @Transactional(readOnly = true) @@ -221,67 +223,6 @@ public int purgeExpiredTrash() { return purgeExpiredTrash(nowUtc); } - @Transactional(propagation = Propagation.REQUIRED, rollbackFor = Exception.class) - public BulkImportResponse bulkImport(UUID userId, BulkImportRequest request) { - User user = userRepository.findById(userId).orElseThrow(() -> new ApiException(ErrorCode.NOT_FOUND)); - - List sourceLocalIds = request.docs().stream() - .map(BulkImportItemRequest::localId) - .map(DocumentService::normalizeSourceLocalId) - .filter(id -> id != null) - .toList(); - - Map existingByLocalId = new HashMap<>(); - if (!sourceLocalIds.isEmpty()) { - existingByLocalId = - documentRepository - .findAllByUser_IdAndSourceLocalIdInAndDeletedAtIsNull(userId, sourceLocalIds) - .stream() - .collect(java.util.stream.Collectors.toMap(Document::getSourceLocalId, doc -> doc)); - } - - List imported = new java.util.ArrayList<>(); - for (BulkImportItemRequest item : request.docs()) { - String localId = normalizeSourceLocalId(item.localId()); - Document existing = localId == null ? null : existingByLocalId.get(localId); - Document target = existing == null ? new Document() : existing; - - applyFields(target, user, item.title(), item.yjsState(), item.createdBy(), localId); - - target = saveWithSourceLocalIdRetry(userId, localId, target, user, item); - - if (localId != null) { - existingByLocalId.put(localId, target); - } - - imported.add(new BulkImportItemResponse(item.localId(), target.getId(), target.getTitle())); - } - - return new BulkImportResponse(imported); - } - - private Document saveWithSourceLocalIdRetry( - UUID userId, String localId, Document target, User user, BulkImportItemRequest item) { - if (target.getId() != null) { - return documentRepository.save(target); - } - - try { - return documentRepository.saveAndFlush(target); - } catch (DataIntegrityViolationException ex) { - if (localId == null) { - throw ex; - } - - Document latest = documentRepository - .findByUser_IdAndSourceLocalIdAndDeletedAtIsNull(userId, localId) - .orElseThrow(() -> ex); - - applyFields(latest, user, item.title(), item.yjsState(), item.createdBy(), localId); - return documentRepository.save(latest); - } - } - private static String normalizeTitle(String title) { String value = title == null ? "" : title.strip(); if (value.isBlank()) { @@ -290,24 +231,6 @@ private static String normalizeTitle(String title) { return value; } - private static String normalizeSourceLocalId(String sourceLocalId) { - if (sourceLocalId == null) { - return null; - } - - String value = sourceLocalId.strip(); - return value.isBlank() ? null : value; - } - - private static void applyFields( - Document target, User user, String title, String yjsState, String createdBy, String sourceLocalId) { - target.setUser(user); - target.setTitle(normalizeTitle(title)); - target.setYjsState(decodeBase64State(yjsState)); - target.setCreatedBy(createdBy); - target.setSourceLocalId(sourceLocalId); - } - private static byte[] decodeBase64State(String yjsState) { if (yjsState == null) { return null; diff --git a/api/src/main/resources/db/migration/V3__add_source_local_id_for_documents.sql b/api/src/main/resources/db/migration/V3__add_source_local_id_for_documents.sql deleted file mode 100644 index b56fef2..0000000 --- a/api/src/main/resources/db/migration/V3__add_source_local_id_for_documents.sql +++ /dev/null @@ -1,9 +0,0 @@ -ALTER TABLE documents - ADD COLUMN source_local_id VARCHAR(128); - -CREATE INDEX IF NOT EXISTS idx_documents_user_source_local - ON documents(user_id, source_local_id); - -CREATE UNIQUE INDEX IF NOT EXISTS uq_documents_user_source_local - ON documents(user_id, source_local_id) - WHERE source_local_id IS NOT NULL; diff --git a/api/src/test/java/com/nextdocs/api/document/controller/DocumentControllerTest.java b/api/src/test/java/com/nextdocs/api/document/controller/DocumentControllerTest.java index bf98b42..1b20745 100644 --- a/api/src/test/java/com/nextdocs/api/document/controller/DocumentControllerTest.java +++ b/api/src/test/java/com/nextdocs/api/document/controller/DocumentControllerTest.java @@ -14,8 +14,6 @@ import com.nextdocs.api.auth.security.UserPrincipal; import com.nextdocs.api.common.exception.ApiException; import com.nextdocs.api.common.exception.ErrorCode; -import com.nextdocs.api.document.dto.response.BulkImportItemResponse; -import com.nextdocs.api.document.dto.response.BulkImportResponse; import com.nextdocs.api.document.dto.response.DocumentResponse; import com.nextdocs.api.document.service.DocumentService; import java.time.OffsetDateTime; @@ -75,7 +73,8 @@ void create_success_returns201() throws Exception { DocumentResponse response = new DocumentResponse( documentId, "My Doc", "AQID", "Alice", OffsetDateTime.now(), OffsetDateTime.now(), null, null); - when(documentService.create(eq(userId), any())).thenReturn(response); + when(documentService.create(eq(userId), any())) + .thenReturn(new DocumentService.CreateDocumentResult(response, true)); mockMvc.perform(post("/api/v1/documents") .with(user(principal)) @@ -92,6 +91,30 @@ void create_success_returns201() throws Exception { .andExpect(jsonPath("$.message").value("Document created.")); } + @Test + void create_existingClientDocument_returns200() throws Exception { + DocumentResponse response = new DocumentResponse( + documentId, "My Doc", "AQID", "Alice", OffsetDateTime.now(), OffsetDateTime.now(), null, null); + + when(documentService.create(eq(userId), any())) + .thenReturn(new DocumentService.CreateDocumentResult(response, false)); + + mockMvc.perform(post("/api/v1/documents") + .with(user(principal)) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "id": "%s", + "title": "My Doc", + "yjsState": "AQID" + } + """.formatted(documentId))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.success").value(true)) + .andExpect(jsonPath("$.data.id").value(documentId.toString())) + .andExpect(jsonPath("$.message").value("Document already exists.")); + } + @Test void list_success_returns200() throws Exception { DocumentResponse response = new DocumentResponse( @@ -179,31 +202,6 @@ void restore_success_returns200() throws Exception { .andExpect(jsonPath("$.data.title").value("Restored")); } - @Test - void bulkImport_success_returns201() throws Exception { - BulkImportResponse response = - new BulkImportResponse(List.of(new BulkImportItemResponse("local-1", documentId, "Imported"))); - when(documentService.bulkImport(eq(userId), any())).thenReturn(response); - - mockMvc.perform(post("/api/v1/documents/bulk-import") - .with(user(principal)) - .contentType(MediaType.APPLICATION_JSON) - .content(""" - { - "docs": [ - { - "localId": "local-1", - "title": "Imported", - "yjsState": "AQID" - } - ] - } - """)) - .andExpect(status().isCreated()) - .andExpect(jsonPath("$.success").value(true)) - .andExpect(jsonPath("$.data.imported[0].localId").value("local-1")); - } - @Test void endpoints_withoutAuthentication_return401() throws Exception { mockMvc.perform(get("/api/v1/documents")).andExpect(status().isUnauthorized()); diff --git a/api/src/test/java/com/nextdocs/api/document/service/DocumentServiceTest.java b/api/src/test/java/com/nextdocs/api/document/service/DocumentServiceTest.java index 6c70ebe..4f2d9bc 100644 --- a/api/src/test/java/com/nextdocs/api/document/service/DocumentServiceTest.java +++ b/api/src/test/java/com/nextdocs/api/document/service/DocumentServiceTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -12,6 +13,7 @@ import com.nextdocs.api.common.exception.ApiException; import com.nextdocs.api.common.exception.ErrorCode; import com.nextdocs.api.document.config.DocumentProperties; +import com.nextdocs.api.document.dto.request.DocumentCreateRequest; import com.nextdocs.api.document.dto.request.DocumentUpdateRequest; import com.nextdocs.api.document.entity.Document; import com.nextdocs.api.document.entity.DocumentAccessLevel; @@ -68,6 +70,86 @@ void purgeExpiredTrash_deletesRowsOlderThanRetentionCutoff() { assertEquals(OffsetDateTime.of(2025, 5, 16, 12, 0, 0, 0, ZoneOffset.UTC), cutoff.getValue()); } + @Test + void create_persistsClientProvidedId() { + UUID userId = UUID.randomUUID(); + UUID documentId = UUID.randomUUID(); + User user = User.builder() + .id(userId) + .email("alice@example.com") + .displayName("Alice") + .build(); + DocumentCreateRequest request = new DocumentCreateRequest(documentId, "My Doc", "AQID", "Alice"); + + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + when(documentRepository.findByIdAndUser_Id(documentId, userId)).thenReturn(Optional.empty()); + when(documentRepository.saveAndFlush(any(Document.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + DocumentService.CreateDocumentResult result = documentService.create(userId, request); + + assertTrue(result.created()); + assertEquals(documentId, result.document().id()); + verify(documentRepository).findByIdAndUser_Id(documentId, userId); + verify(documentRepository).saveAndFlush(any(Document.class)); + } + + @Test + void create_returnsExistingDocumentForMatchingClientProvidedId() { + UUID userId = UUID.randomUUID(); + UUID documentId = UUID.randomUUID(); + User user = User.builder() + .id(userId) + .email("alice@example.com") + .displayName("Alice") + .build(); + Document existing = Document.builder() + .id(documentId) + .user(user) + .title("Existing") + .yjsState(new byte[] {1, 2, 3}) + .createdBy("Alice") + .createdAt(OffsetDateTime.now(ZoneOffset.UTC)) + .updatedAt(OffsetDateTime.now(ZoneOffset.UTC)) + .build(); + DocumentCreateRequest request = new DocumentCreateRequest(documentId, "My Doc", "AQID", "Alice"); + + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + when(documentRepository.findByIdAndUser_Id(documentId, userId)).thenReturn(Optional.of(existing)); + + DocumentService.CreateDocumentResult result = documentService.create(userId, request); + + assertEquals(false, result.created()); + assertEquals(documentId, result.document().id()); + verify(documentRepository, never()).saveAndFlush(any(Document.class)); + } + + @Test + void create_rejectsTrashedDocumentIdReuse() { + UUID userId = UUID.randomUUID(); + UUID documentId = UUID.randomUUID(); + User user = User.builder() + .id(userId) + .email("alice@example.com") + .displayName("Alice") + .build(); + Document existing = Document.builder() + .id(documentId) + .user(user) + .title("Existing") + .yjsState(new byte[] {1, 2, 3}) + .deletedAt(OffsetDateTime.now(ZoneOffset.UTC)) + .build(); + DocumentCreateRequest request = new DocumentCreateRequest(documentId, "My Doc", "AQID", "Alice"); + + when(userRepository.findById(userId)).thenReturn(Optional.of(user)); + when(documentRepository.findByIdAndUser_Id(documentId, userId)).thenReturn(Optional.of(existing)); + + ApiException exception = assertThrows(ApiException.class, () -> documentService.create(userId, request)); + + assertEquals(ErrorCode.CONFLICT, exception.getErrorCode()); + verify(documentRepository, never()).saveAndFlush(any(Document.class)); + } + @Test void get_allowsGeneralAccessWhenActiveLinkExists() { UUID requesterId = UUID.randomUUID(); diff --git a/web/components/AppShell.tsx b/web/components/AppShell.tsx index 303bc08..5ac859c 100644 --- a/web/components/AppShell.tsx +++ b/web/components/AppShell.tsx @@ -13,7 +13,7 @@ import { documentService } from '@/services/document.service'; import { isUntitledTitle, isEmptyLocalDocument } from '@/lib/document-content.util'; import type { StoredDocument } from '@/types/document.types'; -const LOCAL_IMPORT_LOCK_KEY = 'nextdocs-local-import-lock'; +const LOCAL_PROMOTION_LOCK_KEY = 'nextdocs-local-promotion-lock'; const REGISTRATION_SYNC_MIN_OVERLAY_MS = 800; function wait(ms: number): Promise { @@ -31,9 +31,9 @@ function waitForNextPaint(): Promise { }); } -function tryAcquireImportLock(): boolean { +function tryAcquirePromotionLock(): boolean { const now = Date.now(); - const raw = localStorage.getItem(LOCAL_IMPORT_LOCK_KEY); + const raw = localStorage.getItem(LOCAL_PROMOTION_LOCK_KEY); const lockAgeMs = 30_000; if (raw) { @@ -43,12 +43,12 @@ function tryAcquireImportLock(): boolean { } } - localStorage.setItem(LOCAL_IMPORT_LOCK_KEY, String(now)); + localStorage.setItem(LOCAL_PROMOTION_LOCK_KEY, String(now)); return true; } -function releaseImportLock() { - localStorage.removeItem(LOCAL_IMPORT_LOCK_KEY); +function releasePromotionLock() { + localStorage.removeItem(LOCAL_PROMOTION_LOCK_KEY); } export function getDocsEligibleForAccountMove(docs: StoredDocument[]): StoredDocument[] { @@ -68,11 +68,11 @@ export function AppShell({ children }: { children: React.ReactNode }) { }, []); const { user, isTokenExpiringSoon, isAuthenticated, accessToken, lastAuthAction } = useAuth(); const didPromptImportRef = useRef(false); - const ownsLocalImportLockRef = useRef(false); - const bulkImportInFlightRef = useRef | null>(null); - const bulkImportCompletedKeyRef = useRef(null); + const ownsLocalPromotionLockRef = useRef(false); + const promotionInFlightRef = useRef | null>(null); + const promotionCompletedKeyRef = useRef(null); - const getBulkImportActionKey = useCallback(() => { + const getPromotionActionKey = useCallback(() => { if (!user?.id || !lastAuthAction) { return null; } @@ -82,14 +82,14 @@ export function AppShell({ children }: { children: React.ReactNode }) { const moveLocalDocsToAccount = useCallback( async (docs: StoredDocument[]) => { - const actionKey = getBulkImportActionKey(); + const actionKey = getPromotionActionKey(); - if (actionKey && bulkImportCompletedKeyRef.current === actionKey) { + if (actionKey && promotionCompletedKeyRef.current === actionKey) { return; } - if (bulkImportInFlightRef.current) { - await bulkImportInFlightRef.current; + if (promotionInFlightRef.current) { + await promotionInFlightRef.current; return; } @@ -98,14 +98,8 @@ export function AppShell({ children }: { children: React.ReactNode }) { throw new Error('Missing access token'); } - const imported = await documentService.bulkImportLocalDocuments(accessToken, docs); - const importedIds = new Set( - imported.imported - .map((item) => item.localId) - .filter((id): id is string => typeof id === 'string' && id.length > 0) - ); - - const allDocsConfirmed = docs.every((doc) => importedIds.has(doc.id)); + const promotedIds = await documentService.promoteGuestDocumentsToAccount(accessToken, docs); + const allDocsConfirmed = docs.every((doc) => promotedIds.includes(doc.id)); if (!allDocsConfirmed) { throw new Error('Backend did not confirm all local documents were persisted.'); } @@ -114,23 +108,23 @@ export function AppShell({ children }: { children: React.ReactNode }) { await documentService.deleteGuestDocumentsByIds(docs.map((doc) => doc.id)); if (actionKey) { - bulkImportCompletedKeyRef.current = actionKey; + promotionCompletedKeyRef.current = actionKey; } })(); - bulkImportInFlightRef.current = run; + promotionInFlightRef.current = run; try { await run; } finally { - bulkImportInFlightRef.current = null; + promotionInFlightRef.current = null; } }, - [accessToken, getBulkImportActionKey] + [accessToken, getPromotionActionKey] ); - const waitForBulkImportInFlight = useCallback(async () => { - const inFlight = bulkImportInFlightRef.current; + const waitForPromotionInFlight = useCallback(async () => { + const inFlight = promotionInFlightRef.current; if (!inFlight) { return; } @@ -142,10 +136,10 @@ export function AppShell({ children }: { children: React.ReactNode }) { } }, []); - const releaseImportLockIfOwned = useCallback(() => { - if (ownsLocalImportLockRef.current) { - releaseImportLock(); - ownsLocalImportLockRef.current = false; + const releasePromotionLockIfOwned = useCallback(() => { + if (ownsLocalPromotionLockRef.current) { + releasePromotionLock(); + ownsLocalPromotionLockRef.current = false; } }, []); @@ -156,8 +150,8 @@ export function AppShell({ children }: { children: React.ReactNode }) { setIsImportingLocalDocs(false); setLocalDocsError(null); setIsRegistrationSyncOverlayOpen(false); - releaseImportLockIfOwned(); - }, [releaseImportLockIfOwned]); + releasePromotionLockIfOwned(); + }, [releasePromotionLockIfOwned]); // Run exactly once on mount to restore session from the refresh-token cookie useEffect(() => { @@ -185,16 +179,16 @@ export function AppShell({ children }: { children: React.ReactNode }) { let isDisposed = false; const resetAfterInFlightImport = async () => { - await waitForBulkImportInFlight(); + await waitForPromotionInFlight(); if (isDisposed) { return; } didPromptImportRef.current = false; - bulkImportCompletedKeyRef.current = null; - bulkImportInFlightRef.current = null; - releaseImportLockIfOwned(); + promotionCompletedKeyRef.current = null; + promotionInFlightRef.current = null; + releasePromotionLockIfOwned(); }; void resetAfterInFlightImport(); @@ -202,7 +196,7 @@ export function AppShell({ children }: { children: React.ReactNode }) { return () => { isDisposed = true; }; - }, [isAuthenticated, user?.id, waitForBulkImportInFlight, releaseImportLockIfOwned]); + }, [isAuthenticated, user?.id, waitForPromotionInFlight, releasePromotionLockIfOwned]); // Prompt once per app load after login to promote local docs into account. useEffect(() => { @@ -219,11 +213,11 @@ export function AppShell({ children }: { children: React.ReactNode }) { return; } - if (!tryAcquireImportLock()) { + if (!tryAcquirePromotionLock()) { return; } - ownsLocalImportLockRef.current = true; + ownsLocalPromotionLockRef.current = true; let cancelled = false; @@ -248,7 +242,7 @@ export function AppShell({ children }: { children: React.ReactNode }) { if (cancelled || promotableLocalDocs.length === 0) { didPromptImportRef.current = true; setIsRegistrationSyncOverlayOpen(false); - releaseImportLockIfOwned(); + releasePromotionLockIfOwned(); return; } @@ -275,15 +269,15 @@ export function AppShell({ children }: { children: React.ReactNode }) { setLocalDocsToPromote(promotableLocalDocs); setIsLocalDocsModalOpen(true); } catch (error) { - console.error('Failed to import local documents:', error); + console.error('Failed to promote local documents:', error); setIsImportingLocalDocs(false); setIsRegistrationSyncOverlayOpen(lastAuthAction === 'register'); setLocalDocsError( - error instanceof Error ? error.message : 'Failed to import local documents.' + error instanceof Error ? error.message : 'Failed to promote local documents.' ); if (!isRegistrationFlow) { - releaseImportLockIfOwned(); + releasePromotionLockIfOwned(); } } }; @@ -294,8 +288,8 @@ export function AppShell({ children }: { children: React.ReactNode }) { cancelled = true; const releaseLockAfterInFlightImport = async () => { - await waitForBulkImportInFlight(); - releaseImportLockIfOwned(); + await waitForPromotionInFlight(); + releasePromotionLockIfOwned(); }; void releaseLockAfterInFlightImport(); @@ -307,8 +301,8 @@ export function AppShell({ children }: { children: React.ReactNode }) { lastAuthAction, closePromotionFlow, moveLocalDocsToAccount, - releaseImportLockIfOwned, - waitForBulkImportInFlight, + releasePromotionLockIfOwned, + waitForPromotionInFlight, ]); const runMoveToAccount = useCallback(async () => { @@ -336,7 +330,7 @@ export function AppShell({ children }: { children: React.ReactNode }) { } catch (error) { setIsImportingLocalDocs(false); setLocalDocsError( - error instanceof Error ? error.message : 'Failed to import local documents.' + error instanceof Error ? error.message : 'Failed to promote local documents.' ); } }, [accessToken, user?.id, localDocsToPromote, moveLocalDocsToAccount, closePromotionFlow]); diff --git a/web/components/Sidebar.tsx b/web/components/Sidebar.tsx index cc806c0..f65ea93 100644 --- a/web/components/Sidebar.tsx +++ b/web/components/Sidebar.tsx @@ -30,6 +30,7 @@ import { SettingsModal } from '@/components/SettingsModal'; import { useTheme } from '@/hooks/useTheme.hook'; import { useAuth } from '@/hooks/useAuth.hook'; import { useOfflineDocumentSelect } from '@/hooks/useOfflineDocumentSelect.hook'; +import { generateDocumentId } from '@/lib/document-id.util'; import { OFFLINE_DOCUMENT_SELECT_EVENT } from '@/lib/offline-navigation.util'; import { resolveRootDocumentId } from '@/lib/root-document.util'; @@ -421,15 +422,18 @@ function Sidebar({ onOpenAuth }: { onOpenAuth: () => void }) { const handleCreateFile = useCallback(async () => { try { - let newId: string; + const newId = generateDocumentId(); + const created = await documentService.createDocument(); + await documentService.saveDocument(newId, created.ydoc, created.meta); if (isAuthenticated && accessToken) { - const created = await documentService.createCloudDocument(accessToken); - newId = created.id; - } else { - newId = crypto.randomUUID(); - const { ydoc, meta } = await documentService.createDocument(); - await documentService.saveDocument(newId, ydoc, meta); + await documentService.createCloudDocument( + accessToken, + newId, + created.meta.title || 'Untitled', + created.ydoc, + created.meta.createdBy ?? null + ); } await refresh(false); diff --git a/web/hooks/useDocument.hook.ts b/web/hooks/useDocument.hook.ts index 11c1f09..b9f55cc 100644 --- a/web/hooks/useDocument.hook.ts +++ b/web/hooks/useDocument.hook.ts @@ -187,6 +187,7 @@ export function useDocument(documentId: string, options?: UseDocumentOptions) { const [isRealtimeConnected, setIsRealtimeConnected] = useState(false); const [realtimeProvider, setRealtimeProvider] = useState(null); const [errorState, setErrorState] = useState(null); + const [retryTrigger, setRetryTrigger] = useState(0); const lastLoadContextKeyRef = useRef(null); const { isInBackoff: isCloudReadInBackoff, @@ -212,6 +213,25 @@ export function useDocument(documentId: string, options?: UseDocumentOptions) { accessLevelRef.current = accessLevel; }, [accessLevel]); + useEffect(() => { + if (!isAuthenticated || isInitializing || errorState === null || ydoc !== null || isLoading) { + return; + } + + const retryLoad = () => { + clearCloudReadBackoff(); + setRetryTrigger((prev) => prev + 1); + }; + + window.addEventListener('cloud-documents-changed', retryLoad); + window.addEventListener('local-documents-changed', retryLoad); + + return () => { + window.removeEventListener('cloud-documents-changed', retryLoad); + window.removeEventListener('local-documents-changed', retryLoad); + }; + }, [isAuthenticated, isInitializing, errorState, ydoc, isLoading, clearCloudReadBackoff]); + useEffect(() => { if (isInitializing) { dispatch(setLoading(true)); @@ -447,6 +467,7 @@ export function useDocument(documentId: string, options?: UseDocumentOptions) { id, dispatch, isAuthenticated, + accessToken, isInitializing, ydoc, isOnline, @@ -456,6 +477,7 @@ export function useDocument(documentId: string, options?: UseDocumentOptions) { clearCloudReadBackoff, triggerCloudReadBackoff, refresh, + retryTrigger, ]); useEffect(() => { diff --git a/web/lib/idb-isolation.util.ts b/web/lib/idb-isolation.util.ts index 6c7efad..d788536 100644 --- a/web/lib/idb-isolation.util.ts +++ b/web/lib/idb-isolation.util.ts @@ -25,7 +25,9 @@ function clearNextdocsLocalStorageKeys(): void { // user because of the unique database name per user. This is just an extra cleanup // step to free up space and avoid confusion. export async function clearLocalUserData(): Promise { - if (indexedDBService.isAvailable()) { + // Guest documents live in the shared guest database and should survive + // unauthenticated session refresh failures or app reloads. + if (indexedDBService.isAvailable() && indexedDBService.getUserId() !== null) { try { await indexedDBService.clearAllDocuments(); } catch { diff --git a/web/lib/root-document.util.ts b/web/lib/root-document.util.ts index 2701fa2..b7b3cac 100644 --- a/web/lib/root-document.util.ts +++ b/web/lib/root-document.util.ts @@ -11,6 +11,12 @@ interface LocalLoadedDocument { result: DocumentLoadResult; } +interface LocalDocumentSnapshot { + id: string; + ydoc: DocumentLoadResult['ydoc']; + meta: DocumentMeta; +} + export interface ResolveRootDocumentOptions { isAuthenticated: boolean; accessToken: string | null; @@ -71,14 +77,26 @@ async function getMostRecentLocalDocument( }; } -async function createLocalDocument(): Promise { - const newDocumentId = generateDocumentId(); - const created = await documentService.createDocument(); - await documentService.saveDocument(newDocumentId, created.ydoc, created.meta, { +async function createLocalDocumentSnapshot(title?: string): Promise { + const id = generateDocumentId(); + const created = await documentService.createDocument(title); + return { + id, + ydoc: created.ydoc, + meta: created.meta, + }; +} + +async function persistLocalDocument(snapshot: LocalDocumentSnapshot): Promise { + await documentService.saveDocument(snapshot.id, snapshot.ydoc, snapshot.meta, { touchUpdatedAt: false, }); documentService.emitLocalDocumentsChanged(); - return newDocumentId; + return snapshot.id; +} + +async function createLocalDocument(title?: string): Promise { + return persistLocalDocument(await createLocalDocumentSnapshot(title)); } async function migrateLocalDocumentToCloud( @@ -88,55 +106,20 @@ async function migrateLocalDocumentToCloud( const normalizedTitle = normalizeDocumentTitle(localDoc.result.meta.title); const created = await documentService.createCloudDocument( accessToken, + localDoc.id, normalizedTitle, - localDoc.id + localDoc.result.ydoc, + localDoc.result.meta.createdBy ?? null ); try { - await documentService.saveCloudDocument( - created.id, - localDoc.result.ydoc, - { - ...localDoc.result.meta, - title: normalizedTitle, - }, - accessToken - ); - } catch (saveError) { - try { - await documentService.deleteCloudDocumentPermanently(created.id, accessToken); - } catch (rollbackError) { - console.error( - 'Failed to rollback partially-created cloud document during root local migration:', - rollbackError - ); - } - - throw saveError; - } - - try { - await documentService.saveDocument( - created.id, - localDoc.result.ydoc, - { - ...localDoc.result.meta, - title: normalizedTitle, - }, - { touchUpdatedAt: false } - ); + await documentService.saveDocument(localDoc.id, localDoc.result.ydoc, created.meta, { + touchUpdatedAt: false, + }); } catch (cacheError) { console.warn('Failed to cache migrated cloud document locally:', cacheError); } - if (created.id !== localDoc.id) { - try { - await documentService.deleteDocument(localDoc.id); - } catch (deleteError) { - console.warn('Failed to remove old local document after cloud migration:', deleteError); - } - } - documentService.emitCloudDocumentsChanged(); documentService.emitLocalDocumentsChanged(); return created.id; @@ -171,20 +154,20 @@ export async function resolveRootDocumentId(options: ResolveRootDocumentOptions) } const normalizedTitle = normalizeDocumentTitle(options.title); + const localSnapshot = await createLocalDocumentSnapshot(normalizedTitle); const createdCloudDocument = await documentService.createCloudDocument( options.accessToken, - normalizedTitle + localSnapshot.id, + normalizedTitle, + localSnapshot.ydoc, + localSnapshot.meta.createdBy ?? null ); - const createdCloudDocumentMeta = { - ...createdCloudDocument.meta, - title: normalizedTitle, - }; try { await documentService.saveDocument( - createdCloudDocument.id, - createdCloudDocument.ydoc, - createdCloudDocumentMeta, + localSnapshot.id, + localSnapshot.ydoc, + createdCloudDocument.meta, { touchUpdatedAt: false } ); } catch (cacheError) { @@ -203,7 +186,7 @@ export async function resolveRootDocumentId(options: ResolveRootDocumentOptions) return mostRecentLocalDocument.id; } - return createLocalDocument(); + return persistLocalDocument(await createLocalDocumentSnapshot(options.title)); } } diff --git a/web/services/document.service.ts b/web/services/document.service.ts index 2f3dda1..8f0e7b3 100644 --- a/web/services/document.service.ts +++ b/web/services/document.service.ts @@ -28,14 +28,6 @@ interface ApiPage { last: boolean; } -interface BulkImportResponse { - imported: { - localId: string; - documentId: string; - title: string; - }[]; -} - interface ApiDocument { id: string; title: string; @@ -114,6 +106,11 @@ export class DocumentServiceApiError extends Error { } class DocumentService { + private normalizeCloudDocumentTitle(title: string | null | undefined): string { + const value = title?.trim(); + return value ? value : 'Untitled'; + } + public async loadDocument(id: string): Promise { const storedDoc = await indexedDBService.getDocument(id); @@ -461,17 +458,17 @@ class DocumentService { public async createCloudDocument( accessToken: string, + id: string, title = 'Untitled', - sourceLocalId?: string + ydoc?: Y.Doc, + createdBy?: string | null ): Promise<{ id: string; ydoc: Y.Doc; meta: DocumentMeta }> { - const ydoc = createYjsDoc(); + const documentYDoc = ydoc ?? createYjsDoc(); const payload = { + id, title, - yjsState: this.uint8ArrayToBase64(encodeYjsState(ydoc)), - icon: null, - coverImage: null, - createdBy: 'NextDocs User', - sourceLocalId, + yjsState: this.uint8ArrayToBase64(encodeYjsState(documentYDoc)), + createdBy: createdBy ?? 'NextDocs User', }; const body = await this.fetchApi('/api/v1/documents', { @@ -480,9 +477,15 @@ class DocumentService { body: JSON.stringify(payload), }); + if (body.id !== id) { + throw new Error( + `createCloudDocument: server returned ID "${body.id}" for requested ID "${id}".` + ); + } + return { id: body.id, - ydoc, + ydoc: documentYDoc, meta: this.toDocumentMeta(body), }; } @@ -561,30 +564,31 @@ class DocumentService { return indexedDBService.getAllGuestDocuments(); } - public async bulkImportLocalDocuments( + public async promoteGuestDocumentsToAccount( accessToken: string, docs: StoredDocument[] - ): Promise { - const payload = { - docs: docs.map((doc) => ({ - localId: doc.id, - title: doc.meta.title, - icon: doc.meta.icon, - coverImage: doc.meta.coverImage, - yjsState: this.uint8ArrayToBase64(doc.yjsState), - createdBy: doc.meta.createdBy, - })), - }; - - const body = await this.fetchApi('/api/v1/documents/bulk-import', { - method: 'POST', - accessToken, - body: JSON.stringify(payload), - }); + ): Promise { + const promotedIds = await Promise.all( + docs.map(async (doc) => { + const created = await this.createCloudDocument( + accessToken, + doc.id, + this.normalizeCloudDocumentTitle(doc.meta.title), + decodeYjsState(doc.yjsState), + doc.meta.createdBy ?? null + ); + + await this.saveDocument(doc.id, created.ydoc, created.meta, { + touchUpdatedAt: false, + }); + + return doc.id; + }) + ); this.emitCloudDocumentsChanged(); - - return body; + this.emitLocalDocumentsChanged(); + return promotedIds; } public async deleteLocalDocumentsByIds(ids: string[]): Promise { diff --git a/web/services/indexed-db.service.ts b/web/services/indexed-db.service.ts index 36f1e1f..11d8bff 100644 --- a/web/services/indexed-db.service.ts +++ b/web/services/indexed-db.service.ts @@ -28,6 +28,10 @@ class IndexedDBService { return this.currentUserId ? `nextdocs-db_${this.currentUserId}` : 'nextdocs-db'; } + public getUserId(): string | null { + return this.currentUserId; + } + public setUserId(userId: string | null): void { if (this.currentUserId === userId) { return; diff --git a/web/tests/unit/components/AppShell.test.tsx b/web/tests/unit/components/AppShell.test.tsx index 1946784..26b4321 100644 --- a/web/tests/unit/components/AppShell.test.tsx +++ b/web/tests/unit/components/AppShell.test.tsx @@ -104,7 +104,7 @@ describe('AppShell local guest document promotion', () => { deleteGuestDocumentsByIdsSpy = jest .spyOn(documentService, 'deleteGuestDocumentsByIds') .mockResolvedValue(undefined); - jest.spyOn(documentService, 'bulkImportLocalDocuments').mockResolvedValue({ imported: [] }); + jest.spyOn(documentService, 'promoteGuestDocumentsToAccount').mockResolvedValue([]); }); afterEach(() => { diff --git a/web/tests/unit/components/RootDocumentResolver.test.tsx b/web/tests/unit/components/RootDocumentResolver.test.tsx index f0c00cc..7539153 100644 --- a/web/tests/unit/components/RootDocumentResolver.test.tsx +++ b/web/tests/unit/components/RootDocumentResolver.test.tsx @@ -66,7 +66,7 @@ describe('RootDocumentResolver', () => { hasMore: false, }); (documentService.createCloudDocument as jest.Mock).mockResolvedValue({ - id: 'cloud-created-id', + id: 'generated-local-id', ydoc: new Y.Doc(), meta: defaultMeta, }); @@ -202,7 +202,7 @@ describe('RootDocumentResolver', () => { meta: localMeta, }); (documentService.createCloudDocument as jest.Mock).mockResolvedValue({ - id: 'cloud-from-local', + id: 'local-recent', ydoc: new Y.Doc(), meta: localMeta, }); @@ -210,20 +210,22 @@ describe('RootDocumentResolver', () => { render(); await waitFor(() => { - expect(mockReplace).toHaveBeenCalledWith('/doc/cloud-from-local'); + expect(mockReplace).toHaveBeenCalledWith('/doc/local-recent'); }); expect(documentService.createCloudDocument).toHaveBeenCalledWith( 'token-2', + 'local-recent', 'Recent local', - 'local-recent' + localYDoc, + null ); - expect(documentService.saveCloudDocument).toHaveBeenCalledWith( - 'cloud-from-local', + expect(documentService.saveDocument).toHaveBeenCalledWith( + 'local-recent', localYDoc, expect.objectContaining({ title: 'Recent local' }), - 'token-2' + { touchUpdatedAt: false } ); - expect(documentService.deleteDocument).toHaveBeenCalledWith('local-recent'); + expect(documentService.deleteDocument).not.toHaveBeenCalled(); }); }); diff --git a/web/tests/unit/hooks/useDocument.hook.test.tsx b/web/tests/unit/hooks/useDocument.hook.test.tsx index 8c4b3d2..0baea73 100644 --- a/web/tests/unit/hooks/useDocument.hook.test.tsx +++ b/web/tests/unit/hooks/useDocument.hook.test.tsx @@ -440,6 +440,48 @@ describe('useDocument', () => { consoleErrorSpy.mockRestore(); }); + it('should retry loading after document change events when migration finishes', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + const recoveredYdoc = new Y.Doc(); + const recoveredMeta = { + title: 'Recovered Cloud Copy', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-02T00:00:00.000Z', + }; + + (useAuth as jest.Mock).mockReturnValue({ + isAuthenticated: true, + accessToken: 'token-retry', + isInitializing: false, + }); + + getCloudDocumentSpy + .mockRejectedValueOnce(new TypeError('Failed to fetch')) + .mockResolvedValueOnce({ ydoc: recoveredYdoc, meta: recoveredMeta }); + loadDocumentSpy.mockResolvedValue(null); + + const { result } = renderHook(() => useDocument('cloud-retry-id'), { + wrapper: createWrapper(), + }); + + await waitFor(() => { + expect(result.current.errorState?.title).toBe('Document unavailable offline'); + }); + + await act(async () => { + window.dispatchEvent(new CustomEvent('cloud-documents-changed')); + }); + + await waitFor(() => { + expect(result.current.meta?.title).toBe('Recovered Cloud Copy'); + }); + + expect(getCloudDocumentSpy).toHaveBeenCalledTimes(2); + expect(result.current.errorState).toBeNull(); + + consoleErrorSpy.mockRestore(); + }); + it('should persist metadata locally while offline for authenticated users', async () => { const ydoc = new Y.Doc(); const meta = { diff --git a/web/tests/unit/lib/idb-isolation.util.test.ts b/web/tests/unit/lib/idb-isolation.util.test.ts index 5c5a3da..8c19ae6 100644 --- a/web/tests/unit/lib/idb-isolation.util.test.ts +++ b/web/tests/unit/lib/idb-isolation.util.test.ts @@ -6,6 +6,7 @@ jest.mock('../../../services/indexed-db.service', () => ({ clearAllDocuments: jest.fn(), wipeDatabase: jest.fn(), isAvailable: jest.fn().mockReturnValue(true), + getUserId: jest.fn().mockReturnValue('user-1'), setUserId: jest.fn(), }, })); @@ -19,11 +20,15 @@ const mockWipeDatabase = indexedDBService.wipeDatabase as jest.MockedFunction< const mockIsAvailable = indexedDBService.isAvailable as jest.MockedFunction< typeof indexedDBService.isAvailable >; +const mockGetUserId = indexedDBService.getUserId as jest.MockedFunction< + typeof indexedDBService.getUserId +>; describe('clearLocalUserData', () => { beforeEach(() => { jest.clearAllMocks(); mockIsAvailable.mockReturnValue(true); + mockGetUserId.mockReturnValue('user-1'); mockClearAllDocuments.mockReset(); // Use Reset to clear return values too mockWipeDatabase.mockReset(); mockClearAllDocuments.mockResolvedValue(undefined); @@ -71,6 +76,15 @@ describe('clearLocalUserData', () => { expect(mockWipeDatabase).not.toHaveBeenCalled(); }); + it('does not clear guest documents when there is no authenticated user context', async () => { + mockGetUserId.mockReturnValue(null); + + await clearLocalUserData(); + + expect(mockClearAllDocuments).not.toHaveBeenCalled(); + expect(mockWipeDatabase).not.toHaveBeenCalled(); + }); + it('falls back to wipeDatabase if clearAllDocuments rejects', async () => { mockClearAllDocuments.mockRejectedValue(new Error('QuotaExceededError')); mockWipeDatabase.mockResolvedValue(); diff --git a/web/tests/unit/lib/root-document.util.test.ts b/web/tests/unit/lib/root-document.util.test.ts index c2f32fe..25a8238 100644 --- a/web/tests/unit/lib/root-document.util.test.ts +++ b/web/tests/unit/lib/root-document.util.test.ts @@ -2,6 +2,10 @@ import * as Y from 'yjs'; import { resolveRootDocumentId } from '@/lib/root-document.util'; import { documentService } from '@/services/document.service'; +jest.mock('../../../lib/document-id.util', () => ({ + generateDocumentId: jest.fn(() => 'generated-local-id'), +})); + jest.mock('../../../services/document.service', () => ({ documentService: { getAllDocumentsMeta: jest.fn(), @@ -21,7 +25,7 @@ jest.mock('../../../services/document.service', () => ({ describe('resolveRootDocumentId', () => { const createdYDoc = new Y.Doc(); const createdMeta = { - title: '', + title: 'Untitled', createdAt: '2024-01-01T00:00:00.000Z', updatedAt: '2024-01-01T00:00:00.000Z', }; @@ -39,7 +43,11 @@ describe('resolveRootDocumentId', () => { hasMore: false, }); (documentService.createCloudDocument as jest.Mock).mockResolvedValue({ - id: 'cloud-created-id', + id: 'generated-local-id', + ydoc: createdYDoc, + meta: createdMeta, + }); + (documentService.createDocument as jest.Mock).mockResolvedValue({ ydoc: createdYDoc, meta: createdMeta, }); @@ -54,11 +62,17 @@ describe('resolveRootDocumentId', () => { isAuthenticated: true, accessToken: 'token-1', }) - ).resolves.toBe('cloud-created-id'); + ).resolves.toBe('generated-local-id'); - expect(documentService.createCloudDocument).toHaveBeenCalledWith('token-1', 'Untitled'); + expect(documentService.createCloudDocument).toHaveBeenCalledWith( + 'token-1', + 'generated-local-id', + 'Untitled', + expect.any(Y.Doc), + null + ); expect(documentService.saveDocument).toHaveBeenCalledWith( - 'cloud-created-id', + 'generated-local-id', createdYDoc, expect.objectContaining({ title: 'Untitled' }), { touchUpdatedAt: false } @@ -66,17 +80,32 @@ describe('resolveRootDocumentId', () => { }); it('uses a provided title when creating a new cloud root document', async () => { + (documentService.createCloudDocument as jest.Mock).mockResolvedValue({ + id: 'generated-local-id', + ydoc: createdYDoc, + meta: { + ...createdMeta, + title: 'Project kickoff', + }, + }); + await expect( resolveRootDocumentId({ isAuthenticated: true, accessToken: 'token-2', title: 'Project kickoff', }) - ).resolves.toBe('cloud-created-id'); + ).resolves.toBe('generated-local-id'); - expect(documentService.createCloudDocument).toHaveBeenCalledWith('token-2', 'Project kickoff'); + expect(documentService.createCloudDocument).toHaveBeenCalledWith( + 'token-2', + 'generated-local-id', + 'Project kickoff', + expect.any(Y.Doc), + null + ); expect(documentService.saveDocument).toHaveBeenCalledWith( - 'cloud-created-id', + 'generated-local-id', createdYDoc, expect.objectContaining({ title: 'Project kickoff' }), { touchUpdatedAt: false } @@ -90,11 +119,17 @@ describe('resolveRootDocumentId', () => { accessToken: 'token-3', title: ' ', }) - ).resolves.toBe('cloud-created-id'); + ).resolves.toBe('generated-local-id'); - expect(documentService.createCloudDocument).toHaveBeenCalledWith('token-3', 'Untitled'); + expect(documentService.createCloudDocument).toHaveBeenCalledWith( + 'token-3', + 'generated-local-id', + 'Untitled', + expect.any(Y.Doc), + null + ); expect(documentService.saveDocument).toHaveBeenCalledWith( - 'cloud-created-id', + 'generated-local-id', createdYDoc, expect.objectContaining({ title: 'Untitled' }), { touchUpdatedAt: false } diff --git a/web/tests/unit/services/document.service.test.ts b/web/tests/unit/services/document.service.test.ts index 63bb161..292ebb6 100644 --- a/web/tests/unit/services/document.service.test.ts +++ b/web/tests/unit/services/document.service.test.ts @@ -261,7 +261,7 @@ describe('document.service', () => { }); }); - describe('bulkImportLocalDocuments', () => { + describe('createCloudDocument', () => { let originalFetch: typeof globalThis.fetch; beforeEach(() => { @@ -272,48 +272,104 @@ describe('document.service', () => { (globalThis as typeof globalThis & { fetch: typeof fetch }).fetch = originalFetch; }); - it('should call backend and return imported mappings', async () => { + it('should send the requested client-generated id to the backend', async () => { const fetchMock = jest.fn().mockResolvedValue({ ok: true, json: async () => ({ success: true, data: { - imported: [{ localId: 'id-1', documentId: 'server-1', title: 'Doc 1' }], + id: 'id-1', + title: 'Doc 1', + yjsState: 'AQID', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', }, error: null, }), } as Response); (globalThis as typeof globalThis & { fetch: typeof fetch }).fetch = fetchMock as typeof fetch; - const { ydoc, meta } = await documentService.createDocument('Doc 1'); - await documentService.saveDocument('id-1', ydoc, meta); - const docs = await documentService.getAllLocalDocuments(); - - const result = await documentService.bulkImportLocalDocuments('access-token', docs); + const result = await documentService.createCloudDocument('access-token', 'id-1', 'Doc 1'); - expect(result.imported).toHaveLength(1); - expect(result.imported[0].localId).toBe('id-1'); + expect(result.id).toBe('id-1'); expect(fetchMock).toHaveBeenCalled(); + expect(fetchMock.mock.calls[0][1]?.body).toContain('"id":"id-1"'); }); - it('should throw when backend import fails', async () => { + it('should throw when backend returns a different id', async () => { const fetchMock = jest.fn().mockResolvedValue({ - ok: false, + ok: true, json: async () => ({ - success: false, - data: null, - error: 'Import failed', + success: true, + data: { + id: 'server-id', + title: 'Doc 1', + yjsState: 'AQID', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + }, + error: null, }), } as Response); (globalThis as typeof globalThis & { fetch: typeof fetch }).fetch = fetchMock as typeof fetch; + await expect( + documentService.createCloudDocument('access-token', 'id-1', 'Doc 1') + ).rejects.toThrow('server returned ID "server-id"'); + }); + }); + + describe('promoteGuestDocumentsToAccount', () => { + it('should cache promoted guest documents in the active user database', async () => { + const cloudYDoc = new Y.Doc(); + const cloudMeta = { + title: 'Doc 1', + createdAt: '2024-01-02T00:00:00.000Z', + updatedAt: '2024-01-02T00:00:00.000Z', + }; + const createCloudDocumentSpy = jest + .spyOn(documentService, 'createCloudDocument') + .mockResolvedValue({ + id: 'id-1', + ydoc: cloudYDoc, + meta: cloudMeta, + }); + const { ydoc, meta } = await documentService.createDocument('Doc 1'); await documentService.saveDocument('id-1', ydoc, meta); const docs = await documentService.getAllLocalDocuments(); + await indexedDBService.clearAllDocuments(); - await expect(documentService.bulkImportLocalDocuments('access-token', docs)).rejects.toThrow( - 'Import failed' + const result = await documentService.promoteGuestDocumentsToAccount('access-token', docs); + + expect(result).toEqual(['id-1']); + expect(createCloudDocumentSpy).toHaveBeenCalledWith( + 'access-token', + 'id-1', + 'Doc 1', + expect.any(Y.Doc), + null ); + const stored = await indexedDBService.getDocument('id-1'); + expect(stored?.meta).toEqual(cloudMeta); + + createCloudDocumentSpy.mockRestore(); + }); + + it('should propagate create failures during promotion', async () => { + const createCloudDocumentSpy = jest + .spyOn(documentService, 'createCloudDocument') + .mockRejectedValue(new Error('Create failed')); + + const { ydoc, meta } = await documentService.createDocument('Doc 1'); + await documentService.saveDocument('id-1', ydoc, meta); + const docs = await documentService.getAllLocalDocuments(); + + await expect( + documentService.promoteGuestDocumentsToAccount('access-token', docs) + ).rejects.toThrow('Create failed'); + + createCloudDocumentSpy.mockRestore(); }); });