diff --git a/.gitignore b/.gitignore index 3b02a10..4efea5c 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,11 @@ dist/ # Environment configuration .env +# Generated assets (run go generate to rebuild) +pkg/appview/licenses/spdx-licenses.json +pkg/appview/static/js/htmx.min.js +pkg/appview/static/js/lucide.min.js + # IDE .claude/ .vscode/ diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..015d22a --- /dev/null +++ b/Makefile @@ -0,0 +1,84 @@ +# ATCR Makefile +# Build targets for the ATProto Container Registry + +.PHONY: all build build-appview build-hold build-credential-helper build-oauth-helper \ + generate test test-race test-verbose lint clean help + +.DEFAULT_GOAL := help + +help: ## Show this help message + @echo "ATCR Build Targets:" + @echo "" + @awk 'BEGIN {FS = ":.*##"; printf ""} /^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-28s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST) + +all: generate build ## Generate assets and build all binaries (default) + +# Generated asset files +GENERATED_ASSETS = \ + pkg/appview/static/js/htmx.min.js \ + pkg/appview/static/js/lucide.min.js \ + pkg/appview/licenses/spdx-licenses.json + +generate: $(GENERATED_ASSETS) ## Run go generate to download vendor assets + +$(GENERATED_ASSETS): + @echo "→ Generating vendor assets and code..." + go generate ./... + +##@ Build Targets + +build: build-appview build-hold build-credential-helper ## Build all binaries + +build-appview: $(GENERATED_ASSETS) ## Build appview binary only + @echo "→ Building appview..." + @mkdir -p bin + go build -o bin/atcr-appview ./cmd/appview + +build-hold: $(GENERATED_ASSETS) ## Build hold binary only + @echo "→ Building hold..." + @mkdir -p bin + go build -o bin/atcr-hold ./cmd/hold + +build-credential-helper: $(GENERATED_ASSETS) ## Build credential helper only + @echo "→ Building credential helper..." + @mkdir -p bin + go build -o bin/docker-credential-atcr ./cmd/credential-helper + +build-oauth-helper: $(GENERATED_ASSETS) ## Build OAuth helper only + @echo "→ Building OAuth helper..." + @mkdir -p bin + go build -o bin/oauth-helper ./cmd/oauth-helper + +##@ Test Targets + +test: ## Run all tests + @echo "→ Running tests..." + go test -cover ./... + +test-race: ## Run tests with race detector + @echo "→ Running tests with race detector..." + go test -race ./... + +test-verbose: ## Run tests with verbose output + @echo "→ Running tests with verbose output..." + go test -v ./... + +##@ Quality Targets + +.PHONY: check-golangci-lint +check-golangci-lint: + @which golangci-lint > /dev/null || (echo "→ Installing golangci-lint..." && go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest) + +lint: check-golangci-lint ## Run golangci-lint + @echo "→ Running golangci-lint..." + golangci-lint run ./... + +##@ Utility Targets + +clean: ## Remove built binaries and generated assets + @echo "→ Cleaning build artifacts..." + rm -rf bin/ + rm -f pkg/appview/static/js/htmx.min.js + rm -f pkg/appview/static/js/lucide.min.js + rm -f pkg/appview/licenses/spdx-licenses.json + @echo "✓ Clean complete" diff --git a/pkg/appview/storage/context_test.go b/pkg/appview/storage/context_test.go index 92450c8..dce13c8 100644 --- a/pkg/appview/storage/context_test.go +++ b/pkg/appview/storage/context_test.go @@ -2,6 +2,7 @@ package storage import ( "context" + "sync" "testing" "atcr.io/pkg/atproto" @@ -9,20 +10,37 @@ import ( // Mock implementations for testing type mockDatabaseMetrics struct { + mu sync.Mutex pullCount int pushCount int } func (m *mockDatabaseMetrics) IncrementPullCount(did, repository string) error { + m.mu.Lock() + defer m.mu.Unlock() m.pullCount++ return nil } func (m *mockDatabaseMetrics) IncrementPushCount(did, repository string) error { + m.mu.Lock() + defer m.mu.Unlock() m.pushCount++ return nil } +func (m *mockDatabaseMetrics) getPullCount() int { + m.mu.Lock() + defer m.mu.Unlock() + return m.pullCount +} + +func (m *mockDatabaseMetrics) getPushCount() int { + m.mu.Lock() + defer m.mu.Unlock() + return m.pushCount +} + type mockReadmeCache struct{} func (m *mockReadmeCache) Get(ctx context.Context, url string) (string, error) { diff --git a/pkg/appview/storage/manifest_store.go b/pkg/appview/storage/manifest_store.go index 7401158..155a3d7 100644 --- a/pkg/appview/storage/manifest_store.go +++ b/pkg/appview/storage/manifest_store.go @@ -11,6 +11,7 @@ import ( "maps" "net/http" "strings" + "sync" "time" "atcr.io/pkg/atproto" @@ -22,6 +23,7 @@ import ( // It stores manifests in ATProto as records type ManifestStore struct { ctx *RegistryContext // Context with user/hold info + mu sync.RWMutex // Protects lastFetchedHoldDID lastFetchedHoldDID string // Hold DID from most recently fetched manifest (for pull) blobStore distribution.BlobStore // Blob store for fetching config during push } @@ -67,6 +69,7 @@ func (s *ManifestStore) Get(ctx context.Context, dgst digest.Digest, options ... // Store the hold DID for subsequent blob requests during pull // Prefer HoldDID (new format) with fallback to HoldEndpoint (legacy URL format) // The routing repository will cache this for concurrent blob fetches + s.mu.Lock() if manifestRecord.HoldDID != "" { // New format: DID reference (preferred) s.lastFetchedHoldDID = manifestRecord.HoldDID @@ -74,6 +77,7 @@ func (s *ManifestStore) Get(ctx context.Context, dgst digest.Digest, options ... // Legacy format: URL reference - convert to DID s.lastFetchedHoldDID = atproto.ResolveHoldDIDFromURL(manifestRecord.HoldEndpoint) } + s.mu.Unlock() var ociManifest []byte @@ -232,6 +236,8 @@ func digestToRKey(dgst digest.Digest) string { // GetLastFetchedHoldDID returns the hold DID from the most recently fetched manifest // This is used by the routing repository to cache the hold for blob requests func (s *ManifestStore) GetLastFetchedHoldDID() string { + s.mu.RLock() + defer s.mu.RUnlock() return s.lastFetchedHoldDID } diff --git a/pkg/appview/storage/manifest_store_test.go b/pkg/appview/storage/manifest_store_test.go index f83c262..05ff6ab 100644 --- a/pkg/appview/storage/manifest_store_test.go +++ b/pkg/appview/storage/manifest_store_test.go @@ -669,13 +669,13 @@ func TestManifestStore_Get_OnlyCountsGETRequests(t *testing.T) { if tt.expectPullIncrement { // Check that IncrementPullCount was called - if mockDB.pullCount == 0 { + if mockDB.getPullCount() == 0 { t.Error("Expected pull count to be incremented for GET request, but it wasn't") } } else { // Check that IncrementPullCount was NOT called - if mockDB.pullCount > 0 { - t.Errorf("Expected pull count NOT to be incremented for %s request, but it was (count=%d)", tt.httpMethod, mockDB.pullCount) + if mockDB.getPullCount() > 0 { + t.Errorf("Expected pull count NOT to be incremented for %s request, but it was (count=%d)", tt.httpMethod, mockDB.getPullCount()) } } }) diff --git a/pkg/appview/storage/profile_test.go b/pkg/appview/storage/profile_test.go index 890e449..548c6c3 100644 --- a/pkg/appview/storage/profile_test.go +++ b/pkg/appview/storage/profile_test.go @@ -219,6 +219,7 @@ func TestGetProfile(t *testing.T) { // Clear migration locks before each test migrationLocks = sync.Map{} + var mu sync.Mutex putRecordCalled := false var migrationRequest map[string]any @@ -232,8 +233,10 @@ func TestGetProfile(t *testing.T) { // PutRecord (migration) if r.Method == "POST" && strings.Contains(r.URL.Path, "putRecord") { + mu.Lock() putRecordCalled = true json.NewDecoder(r.Body).Decode(&migrationRequest) + mu.Unlock() w.WriteHeader(http.StatusOK) w.Write([]byte(`{"uri":"at://did:plc:test123/io.atcr.sailor.profile/self","cid":"bafytest"}`)) return @@ -270,12 +273,17 @@ func TestGetProfile(t *testing.T) { // Give goroutine time to execute time.Sleep(50 * time.Millisecond) - if !putRecordCalled { + mu.Lock() + called := putRecordCalled + request := migrationRequest + mu.Unlock() + + if !called { t.Error("Expected migration PutRecord to be called") } - if migrationRequest != nil { - recordData := migrationRequest["record"].(map[string]any) + if request != nil { + recordData := request["record"].(map[string]any) migratedHold := recordData["defaultHold"] if migratedHold != tt.expectedHoldDID { t.Errorf("Migrated defaultHold = %v, want %v", migratedHold, tt.expectedHoldDID) diff --git a/pkg/appview/storage/routing_repository.go b/pkg/appview/storage/routing_repository.go index 1d0bc44..ad6a4ff 100644 --- a/pkg/appview/storage/routing_repository.go +++ b/pkg/appview/storage/routing_repository.go @@ -7,6 +7,7 @@ package storage import ( "context" "log/slog" + "sync" "time" "github.com/distribution/distribution/v3" @@ -17,6 +18,7 @@ import ( type RoutingRepository struct { distribution.Repository Ctx *RegistryContext // All context and services (exported for token updates) + mu sync.Mutex // Protects manifestStore and blobStore manifestStore *ManifestStore // Cached manifest store instance blobStore *ProxyBlobStore // Cached blob store instance } @@ -31,35 +33,47 @@ func NewRoutingRepository(baseRepo distribution.Repository, ctx *RegistryContext // Manifests returns the ATProto-backed manifest service func (r *RoutingRepository) Manifests(ctx context.Context, options ...distribution.ManifestServiceOption) (distribution.ManifestService, error) { + r.mu.Lock() // Create or return cached manifest store if r.manifestStore == nil { // Ensure blob store is created first (needed for label extraction during push) + // Release lock while calling Blobs to avoid deadlock + r.mu.Unlock() blobStore := r.Blobs(ctx) + r.mu.Lock() - r.manifestStore = NewManifestStore(r.Ctx, blobStore) + // Double-check after reacquiring lock (another goroutine might have set it) + if r.manifestStore == nil { + r.manifestStore = NewManifestStore(r.Ctx, blobStore) + } } + manifestStore := r.manifestStore + r.mu.Unlock() // After any manifest operation, cache the hold DID for blob fetches // We use a goroutine to avoid blocking, and check after a short delay to allow the operation to complete go func() { time.Sleep(100 * time.Millisecond) // Brief delay to let manifest fetch complete - if holdDID := r.manifestStore.GetLastFetchedHoldDID(); holdDID != "" { + if holdDID := manifestStore.GetLastFetchedHoldDID(); holdDID != "" { // Cache for 10 minutes - should cover typical pull operations GetGlobalHoldCache().Set(r.Ctx.DID, r.Ctx.Repository, holdDID, 10*time.Minute) slog.Debug("Cached hold DID", "component", "storage/routing", "did", r.Ctx.DID, "repo", r.Ctx.Repository, "hold", holdDID) } }() - return r.manifestStore, nil + return manifestStore, nil } // Blobs returns a proxy blob store that routes to external hold service // The registry (AppView) NEVER stores blobs locally - all blobs go through hold service func (r *RoutingRepository) Blobs(ctx context.Context) distribution.BlobStore { + r.mu.Lock() // Return cached blob store if available if r.blobStore != nil { + blobStore := r.blobStore + r.mu.Unlock() slog.Debug("Returning cached blob store", "component", "storage/blobs", "did", r.Ctx.DID, "repo", r.Ctx.Repository) - return r.blobStore + return blobStore } // For pull operations, check if we have a cached hold DID from a recent manifest fetch @@ -85,7 +99,9 @@ func (r *RoutingRepository) Blobs(ctx context.Context) distribution.BlobStore { // Create and cache proxy blob store r.blobStore = NewProxyBlobStore(r.Ctx) - return r.blobStore + blobStore := r.blobStore + r.mu.Unlock() + return blobStore } // Tags returns the tag service diff --git a/pkg/appview/templates/components/head.html b/pkg/appview/templates/components/head.html index 2107bf0..5957327 100644 --- a/pkg/appview/templates/components/head.html +++ b/pkg/appview/templates/components/head.html @@ -12,11 +12,11 @@ - - + + - - + + diff --git a/pkg/appview/ui.go b/pkg/appview/ui.go index 8841ba9..3dab4d3 100644 --- a/pkg/appview/ui.go +++ b/pkg/appview/ui.go @@ -12,6 +12,9 @@ import ( "atcr.io/pkg/appview/licenses" ) +//go:generate curl -fsSL -o static/js/htmx.min.js https://unpkg.com/htmx.org@2.0.8/dist/htmx.min.js +//go:generate curl -fsSL -o static/js/lucide.min.js https://unpkg.com/lucide@latest/dist/umd/lucide.min.js + //go:embed templates/**/*.html var templatesFS embed.FS diff --git a/pkg/atproto/generate.go b/pkg/atproto/generate.go index 9b2c917..c29fd79 100644 --- a/pkg/atproto/generate.go +++ b/pkg/atproto/generate.go @@ -35,6 +35,4 @@ func main() { fmt.Printf("Failed to generate CBOR encoders: %v\n", err) os.Exit(1) } - - fmt.Println("Generated CBOR encoders in pkg/atproto/cbor_gen.go") }