diff --git a/pkg/appview/db/queries.go b/pkg/appview/db/queries.go index f2571f7..ef63217 100644 --- a/pkg/appview/db/queries.go +++ b/pkg/appview/db/queries.go @@ -1088,6 +1088,34 @@ func IsManifestTagged(db *sql.DB, did, repository, digest string) (bool, error) return count > 0, nil } +// GetManifestTags retrieves all tags for a manifest +func GetManifestTags(db *sql.DB, did, repository, digest string) ([]string, error) { + rows, err := db.Query(` + SELECT tag FROM tags + WHERE did = ? AND repository = ? AND digest = ? + ORDER BY tag + `, did, repository, digest) + if err != nil { + return nil, err + } + defer rows.Close() + + var tags []string + for rows.Next() { + var tag string + if err := rows.Scan(&tag); err != nil { + return nil, err + } + tags = append(tags, tag) + } + + if err := rows.Err(); err != nil { + return nil, err + } + + return tags, nil +} + // BackfillState represents the backfill progress type BackfillState struct { StartCursor int64 diff --git a/pkg/appview/handlers/images.go b/pkg/appview/handlers/images.go index 2c8d0d5..2798d27 100644 --- a/pkg/appview/handlers/images.go +++ b/pkg/appview/handlers/images.go @@ -2,6 +2,7 @@ package handlers import ( "database/sql" + "encoding/json" "fmt" "net/http" "strings" @@ -75,6 +76,7 @@ func (h *DeleteManifestHandler) ServeHTTP(w http.ResponseWriter, r *http.Request repo := chi.URLParam(r, "repository") digest := chi.URLParam(r, "digest") + confirmed := r.URL.Query().Get("confirm") == "true" // Check if manifest is tagged tagged, err := db.IsManifestTagged(h.DB, user.DID, repo, digest) @@ -83,8 +85,21 @@ func (h *DeleteManifestHandler) ServeHTTP(w http.ResponseWriter, r *http.Request return } - if tagged { - http.Error(w, "Cannot delete tagged manifest", http.StatusBadRequest) + // If tagged and not confirmed, return tag list and require confirmation + if tagged && !confirmed { + tags, err := db.GetManifestTags(h.DB, user.DID, repo, digest) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + json.NewEncoder(w).Encode(map[string]interface{}{ + "error": "confirmation_required", + "message": "This manifest has associated tags that will also be deleted", + "tags": tags, + }) return } @@ -99,6 +114,31 @@ func (h *DeleteManifestHandler) ServeHTTP(w http.ResponseWriter, r *http.Request apiClient := session.APIClient() pdsClient := atproto.NewClientWithIndigoClient(user.PDSEndpoint, user.DID, apiClient) + // If tagged and confirmed, delete all tags first + if tagged && confirmed { + tags, err := db.GetManifestTags(h.DB, user.DID, repo, digest) + if err != nil { + http.Error(w, fmt.Sprintf("Failed to get tags: %v", err), http.StatusInternalServerError) + return + } + + // Delete each tag from PDS and database + for _, tag := range tags { + // Delete from PDS + tagRKey := fmt.Sprintf("%s:%s", repo, tag) + if err := pdsClient.DeleteRecord(r.Context(), atproto.TagCollection, tagRKey); err != nil { + http.Error(w, fmt.Sprintf("Failed to delete tag '%s' from PDS: %v", tag, err), http.StatusInternalServerError) + return + } + + // Delete from cache + if err := db.DeleteTag(h.DB, user.DID, repo, tag); err != nil { + http.Error(w, fmt.Sprintf("Failed to delete tag '%s' from cache: %v", tag, err), http.StatusInternalServerError) + return + } + } + } + // Compute rkey for manifest record (digest without "sha256:" prefix) rkey := strings.TrimPrefix(digest, "sha256:") diff --git a/pkg/appview/static/js/app.js b/pkg/appview/static/js/app.js index daa4fb9..d95ecfe 100644 --- a/pkg/appview/static/js/app.js +++ b/pkg/appview/static/js/app.js @@ -305,3 +305,114 @@ document.addEventListener('DOMContentLoaded', () => { } } }); + +// Delete manifest with confirmation for tagged manifests +async function deleteManifest(repository, digest, sanitizedId) { + try { + // First, try to delete without confirmation + const response = await fetch(`/api/images/${repository}/manifests/${digest}`, { + method: 'DELETE', + credentials: 'include', + }); + + if (response.status === 409) { + // Manifest has tags, need confirmation + const data = await response.json(); + showManifestDeleteModal(repository, digest, sanitizedId, data.tags); + } else if (response.ok) { + // Successfully deleted + removeManifestElement(sanitizedId); + } else { + // Other error + const errorText = await response.text(); + alert(`Failed to delete manifest: ${errorText}`); + } + } catch (err) { + console.error('Error deleting manifest:', err); + alert(`Error deleting manifest: ${err.message}`); + } +} + +// Show the confirmation modal for deleting a tagged manifest +function showManifestDeleteModal(repository, digest, sanitizedId, tags) { + const modal = document.getElementById('manifest-delete-modal'); + const tagsList = document.getElementById('manifest-delete-tags'); + const confirmBtn = document.getElementById('confirm-manifest-delete-btn'); + + // Clear and populate tags list + tagsList.innerHTML = ''; + tags.forEach(tag => { + const li = document.createElement('li'); + li.textContent = tag; + tagsList.appendChild(li); + }); + + // Set up confirm button click handler + confirmBtn.onclick = () => confirmManifestDelete(repository, digest, sanitizedId); + + // Show modal + modal.style.display = 'flex'; +} + +// Close the manifest delete confirmation modal +function closeManifestDeleteModal() { + const modal = document.getElementById('manifest-delete-modal'); + modal.style.display = 'none'; +} + +// Confirm and execute manifest deletion with all tags +async function confirmManifestDelete(repository, digest, sanitizedId) { + const confirmBtn = document.getElementById('confirm-manifest-delete-btn'); + const originalText = confirmBtn.textContent; + + try { + // Disable button and show loading state + confirmBtn.disabled = true; + confirmBtn.textContent = 'Deleting...'; + + // Delete with confirmation + const response = await fetch(`/api/images/${repository}/manifests/${digest}?confirm=true`, { + method: 'DELETE', + credentials: 'include', + }); + + if (response.ok) { + // Successfully deleted + closeManifestDeleteModal(); + removeManifestElement(sanitizedId); + // Also remove any tag elements that were deleted + location.reload(); // Reload to refresh the tags list + } else { + // Error + const errorText = await response.text(); + alert(`Failed to delete manifest: ${errorText}`); + confirmBtn.disabled = false; + confirmBtn.textContent = originalText; + } + } catch (err) { + console.error('Error deleting manifest:', err); + alert(`Error deleting manifest: ${err.message}`); + confirmBtn.disabled = false; + confirmBtn.textContent = originalText; + } +} + +// Remove a manifest element from the DOM +function removeManifestElement(sanitizedId) { + const element = document.getElementById(`manifest-${sanitizedId}`); + if (element) { + element.remove(); + } +} + +// Close modal when clicking outside +document.addEventListener('DOMContentLoaded', () => { + const modal = document.getElementById('manifest-delete-modal'); + if (modal) { + modal.addEventListener('click', (e) => { + if (e.target === modal) { + closeManifestDeleteModal(); + } + }); + } +}); diff --git a/pkg/appview/storage/context_test.go b/pkg/appview/storage/context_test.go index 683450c..92450c8 100644 --- a/pkg/appview/storage/context_test.go +++ b/pkg/appview/storage/context_test.go @@ -8,13 +8,18 @@ import ( ) // Mock implementations for testing -type mockDatabaseMetrics struct{} +type mockDatabaseMetrics struct { + pullCount int + pushCount int +} func (m *mockDatabaseMetrics) IncrementPullCount(did, repository string) error { + m.pullCount++ return nil } func (m *mockDatabaseMetrics) IncrementPushCount(did, repository string) error { + m.pushCount++ return nil } @@ -96,7 +101,7 @@ func TestRegistryContext_ReadmeCacheInterface(t *testing.T) { } // Test that interface methods are callable - content, err := ctx.ReadmeCache.Get(nil, "https://example.com/README.md") + content, err := ctx.ReadmeCache.Get(context.Background(), "https://example.com/README.md") if err != nil { t.Errorf("Unexpected error: %v", err) } diff --git a/pkg/appview/storage/manifest_store.go b/pkg/appview/storage/manifest_store.go index 499ad62..21896dd 100644 --- a/pkg/appview/storage/manifest_store.go +++ b/pkg/appview/storage/manifest_store.go @@ -86,12 +86,16 @@ func (s *ManifestStore) Get(ctx context.Context, dgst digest.Digest, options ... } // Track pull count (increment asynchronously to avoid blocking the response) + // Only count GET requests (actual downloads), not HEAD requests (existence checks) if s.ctx.Database != nil { - go func() { - if err := s.ctx.Database.IncrementPullCount(s.ctx.DID, s.ctx.Repository); err != nil { - slog.Warn("Failed to increment pull count", "did", s.ctx.DID, "repository", s.ctx.Repository, "error", err) - } - }() + // Check HTTP method from context (distribution library stores it as "http.request.method") + if method, ok := ctx.Value("http.request.method").(string); ok && method == "GET" { + go func() { + if err := s.ctx.Database.IncrementPullCount(s.ctx.DID, s.ctx.Repository); err != nil { + slog.Warn("Failed to increment pull count", "did", s.ctx.DID, "repository", s.ctx.Repository, "error", err) + } + }() + } } // Parse the manifest based on media type diff --git a/pkg/appview/storage/manifest_store_test.go b/pkg/appview/storage/manifest_store_test.go index 77938db..8177a8e 100644 --- a/pkg/appview/storage/manifest_store_test.go +++ b/pkg/appview/storage/manifest_store_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "atcr.io/pkg/atproto" "github.com/distribution/distribution/v3" @@ -605,6 +606,82 @@ func TestManifestStore_Get_HoldDIDTracking(t *testing.T) { } } +// TestManifestStore_Get_OnlyCountsGETRequests verifies that HEAD requests don't increment pull count +func TestManifestStore_Get_OnlyCountsGETRequests(t *testing.T) { + ociManifest := []byte(`{"schemaVersion":2}`) + + tests := []struct { + name string + httpMethod string + expectPullIncrement bool + }{ + { + name: "GET request increments pull count", + httpMethod: "GET", + expectPullIncrement: true, + }, + { + name: "HEAD request does not increment pull count", + httpMethod: "HEAD", + expectPullIncrement: false, + }, + { + name: "POST request does not increment pull count", + httpMethod: "POST", + expectPullIncrement: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == atproto.SyncGetBlob { + w.Write(ociManifest) + return + } + w.Write([]byte(`{ + "uri": "at://did:plc:test123/io.atcr.manifest/abc123", + "value": { + "$type":"io.atcr.manifest", + "holdDid":"did:web:hold01.atcr.io", + "mediaType":"application/vnd.oci.image.manifest.v1+json", + "manifestBlob":{"ref":{"$link":"bafytest"},"size":100} + } + }`)) + })) + defer server.Close() + + client := atproto.NewClient(server.URL, "did:plc:test123", "token") + mockDB := &mockDatabaseMetrics{} + ctx := mockRegistryContext(client, "myapp", "did:web:hold01.atcr.io", "did:plc:test123", "test.handle", mockDB) + store := NewManifestStore(ctx, nil) + + // Create a context with the HTTP method stored (as distribution library does) + testCtx := context.WithValue(context.Background(), "http.request.method", tt.httpMethod) + + _, err := store.Get(testCtx, "sha256:abc123") + if err != nil { + t.Fatalf("Get() error = %v", err) + } + + // Wait for async goroutine to complete (metrics are incremented asynchronously) + time.Sleep(50 * time.Millisecond) + + if tt.expectPullIncrement { + // Check that IncrementPullCount was called + if mockDB.pullCount == 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) + } + } + }) + } +} + // TestManifestStore_Put tests storing manifests func TestManifestStore_Put(t *testing.T) { ociManifest := []byte(`{ diff --git a/pkg/appview/templates/pages/install.html b/pkg/appview/templates/pages/install.html index 93ba56b..1832987 100644 --- a/pkg/appview/templates/pages/install.html +++ b/pkg/appview/templates/pages/install.html @@ -81,14 +81,14 @@ chmod +x install.sh

You can also use docker login with your ATProto app password:

    -
  1. Generate an app password in your ATProto account settings
    2. +
  2. Generate an app password at bsky.app/settings/app-passwords
  3. Run: docker login {{ .RegistryURL }}
  4. Enter your handle as username
  5. Enter your app password
- Note: App passwords are available in your Bluesky account settings under "App Passwords". + Note: Create an app password at bsky.app/settings/app-passwords.
diff --git a/pkg/appview/templates/pages/repository.html b/pkg/appview/templates/pages/repository.html index 643dfe9..58c5e24 100644 --- a/pkg/appview/templates/pages/repository.html +++ b/pkg/appview/templates/pages/repository.html @@ -212,10 +212,7 @@ {{ if $.IsOwner }} {{ end }} @@ -259,6 +256,123 @@ + + + + + {{ end }} diff --git a/pkg/appview/templates/pages/settings.html b/pkg/appview/templates/pages/settings.html index 5f6537c..a4d1df5 100644 --- a/pkg/appview/templates/pages/settings.html +++ b/pkg/appview/templates/pages/settings.html @@ -83,7 +83,7 @@
- Fallback: Use app-password with docker login {{ .RegistryURL }} for quick start (no device tracking) + Fallback: Use app password with docker login {{ .RegistryURL }} for quick start (no device tracking)
@@ -109,9 +109,6 @@ - - -