fix: implement document store isolation in tests

Author: isaaclins

excalidraw-server/stores/memory/documents.go

@@ -4,33 +4,46 @@ import (
 	"context"
 	"excalidraw-server/core"
 	"fmt"
+	"sync"
 
 	"github.com/oklog/ulid/v2"
 	"github.com/sirupsen/logrus"
 )
 
-var savedDocuments = make(map[string]core.Document)
-
 type documentStore struct {
+	mu        sync.RWMutex
+	documents map[string]core.Document
 }
 
 func NewDocumentStore() core.DocumentStore {
-	return &documentStore{}
+	return &documentStore{
+		documents: make(map[string]core.Document),
+	}
 }
 
 func (s *documentStore) FindID(ctx context.Context, id string) (*core.Document, error) {
 	log := logrus.WithField("document_id", id)
-	if val, ok := savedDocuments[id]; ok {
+
+	s.mu.RLock()
+	doc, ok := s.documents[id]
+	s.mu.RUnlock()
+
+	if ok {
 		log.Info("Document retrieved successfully")
-		return &val, nil
+		return &doc, nil
 	}
+
 	log.WithField("error", "document not found").Warn("Document with specified ID not found")
 	return nil, fmt.Errorf("document with id %s not found", id)
 }
 
 func (s *documentStore) Create(ctx context.Context, document *core.Document) (string, error) {
 	id := ulid.Make().String()
-	savedDocuments[id] = *document
+
+	s.mu.Lock()
+	s.documents[id] = *document
+	s.mu.Unlock()
+
 	log := logrus.WithFields(logrus.Fields{
 		"document_id": id,
 		"data_length": len(document.Data.Bytes()),

excalidraw-server/stores/memory/documents_test.go

@@ -351,10 +351,24 @@ func TestContextCancellation(t *testing.T) {
 }
 
 func TestStoreIsolation(t *testing.T) {
-	// NOTE: The current in-memory implementation uses a global map,
-	// so isolation between store instances is not supported.
-	// This test documents the current behavior.
-	// In a production system, you'd want each store instance to have its own map.
+	ctx := context.Background()
+
+	store1 := NewDocumentStore()
+	store2 := NewDocumentStore()
+
+	doc := &core.Document{Data: *bytes.NewBufferString("store1")}
+	id, err := store1.Create(ctx, doc)
+	if err != nil {
+		t.Fatalf("Create() failed in store1: %v", err)
+	}
+
+	// store1 should find the document
+	if _, err := store1.FindID(ctx, id); err != nil {
+		t.Fatalf("store1 should locate its document: %v", err)
+	}
 
-	t.Skip("Skipping test - memory store uses global state (by design for this simple implementation)")
+	// store2 should not see store1's document
+	if _, err := store2.FindID(ctx, id); err == nil {
+		t.Fatalf("store2 should not find document created by store1")
+	}
 }