small fix for null columns and background pds inserts on push

This commit is contained in:
Evan Jarrett
2026-06-13 19:50:29 -05:00
parent 6758996300
commit 2eea5ff885
3 changed files with 96 additions and 8 deletions
+7 -7
View File
@@ -145,7 +145,7 @@ func SearchRepositories(db DBTX, query string, limit, offset int, currentUserDID
COALESCE(m.artifact_type, 'container-image'),
COALESCE((SELECT tag FROM tags WHERE did = m.did AND repository = m.repository ORDER BY created_at DESC LIMIT 1), ''),
COALESCE(m.digest, ''),
MAX(rs.last_push, m.created_at),
MAX(COALESCE(rs.last_push, m.created_at), m.created_at),
COALESCE(rp.avatar_cid, '')
FROM matching_repos mr
JOIN manifests m ON mr.latest_id = m.id
@@ -153,7 +153,7 @@ func SearchRepositories(db DBTX, query string, limit, offset int, currentUserDID
JOIN repo_stats ON m.did = repo_stats.did AND m.repository = repo_stats.repository
LEFT JOIN repository_stats rs ON m.did = rs.did AND m.repository = rs.repository
LEFT JOIN repo_pages rp ON m.did = rp.did AND m.repository = rp.repository
ORDER BY MAX(rs.last_push, m.created_at) DESC
ORDER BY MAX(COALESCE(rs.last_push, m.created_at), m.created_at) DESC
LIMIT ? OFFSET ?
`
@@ -2253,7 +2253,7 @@ func GetRepoCards(db DBTX, limit int, currentUserDID string, sortOrder RepoCardS
var orderBy string
switch sortOrder {
case SortByLastUpdate:
orderBy = "MAX(rs.last_push, m.created_at) DESC"
orderBy = "MAX(COALESCE(rs.last_push, m.created_at), m.created_at) DESC"
default: // SortByScore
orderBy = "(COALESCE(rs.pull_count, 0) + COALESCE((SELECT COUNT(*) FROM stars WHERE owner_did = m.did AND repository = m.repository), 0) * 10) DESC, m.created_at DESC"
}
@@ -2279,7 +2279,7 @@ func GetRepoCards(db DBTX, limit int, currentUserDID string, sortOrder RepoCardS
COALESCE(m.artifact_type, 'container-image'),
COALESCE((SELECT tag FROM tags WHERE did = m.did AND repository = m.repository ORDER BY created_at DESC LIMIT 1), ''),
COALESCE(m.digest, ''),
MAX(rs.last_push, m.created_at),
MAX(COALESCE(rs.last_push, m.created_at), m.created_at),
COALESCE(rp.avatar_cid, '')
FROM latest_manifests lm
JOIN manifests m ON lm.latest_id = m.id
@@ -2358,7 +2358,7 @@ func GetUserRepoCards(db DBTX, userDID string, currentUserDID string) ([]RepoCar
COALESCE(m.artifact_type, 'container-image'),
COALESCE((SELECT tag FROM tags WHERE did = m.did AND repository = m.repository ORDER BY created_at DESC LIMIT 1), ''),
COALESCE(m.digest, ''),
MAX(rs.last_push, m.created_at),
MAX(COALESCE(rs.last_push, m.created_at), m.created_at),
COALESCE(rp.avatar_cid, '')
FROM latest_manifests lm
JOIN manifests m ON lm.latest_id = m.id
@@ -2366,7 +2366,7 @@ func GetUserRepoCards(db DBTX, userDID string, currentUserDID string) ([]RepoCar
LEFT JOIN repository_stats rs ON m.did = rs.did AND m.repository = rs.repository
LEFT JOIN repo_pages rp ON m.did = rp.did AND m.repository = rp.repository
WHERE ` + activeTakedownClause("m") + `
ORDER BY MAX(rs.last_push, m.created_at) DESC
ORDER BY MAX(COALESCE(rs.last_push, m.created_at), m.created_at) DESC
`
rows, err := db.Query(query, userDID, currentUserDID, currentUserDID, currentUserDID)
@@ -2444,7 +2444,7 @@ func GetStarredRepoCards(db DBTX, starrerDID string, currentUserDID string) ([]R
COALESCE(m.artifact_type, 'container-image'),
COALESCE((SELECT tag FROM tags WHERE did = m.did AND repository = m.repository ORDER BY created_at DESC LIMIT 1), ''),
COALESCE(m.digest, ''),
MAX(rs.last_push, m.created_at),
MAX(COALESCE(rs.last_push, m.created_at), m.created_at),
COALESCE(rp.avatar_cid, ''),
st.starred_at
FROM latest_manifests lm
+78
View File
@@ -1909,3 +1909,81 @@ func TestGetUserRepositories_Empty(t *testing.T) {
t.Errorf("expected nil slice for user with no repos, got %#v", repos)
}
}
// TestGetRepoCards_NullLastPushStillSortsByCreatedAt is a regression test for
// the "What's New" NULL-poison bug: a repo whose repository_stats row exists
// but has last_push = NULL (e.g. created by a pull when the push-notify never
// recorded a push) must still appear and sort by its manifest created_at.
//
// SQLite's scalar 2-arg max() returns NULL if any argument is NULL, so the old
// `MAX(rs.last_push, m.created_at)` produced a NULL sort key for such repos,
// dropping them to the bottom (and out of the limited "What's New" list) and
// rendering a blank "last updated" timestamp. The fix wraps last_push in
// COALESCE so it falls back to created_at.
func TestGetRepoCards_NullLastPushStillSortsByCreatedAt(t *testing.T) {
d := setupBatchTestDB(t)
createBatchTestUser(t, d, "did:plc:alice")
// A public hold so the manifests are accessible to anonymous viewers.
if _, err := d.Exec(`
INSERT INTO hold_captain_records (hold_did, owner_did, public, allow_all_crew)
VALUES ('did:web:hold', 'did:plc:alice', 1, 0)
`); err != nil {
t.Fatalf("seed captain: %v", err)
}
now := time.Now()
old := now.Add(-240 * time.Hour) // 10 days ago
// "fresh": recent push, but its stats row has last_push = NULL (pulled,
// never push-recorded). "stale": older, with a real last_push.
if _, err := BatchInsertManifests(d, []Manifest{
{DID: "did:plc:alice", Repository: "fresh", Digest: "sha256:fresh", HoldEndpoint: "did:web:hold", SchemaVersion: 2, MediaType: "application/vnd.oci.image.manifest.v1+json", ArtifactType: "container-image", CreatedAt: now},
{DID: "did:plc:alice", Repository: "stale", Digest: "sha256:stale", HoldEndpoint: "did:web:hold", SchemaVersion: 2, MediaType: "application/vnd.oci.image.manifest.v1+json", ArtifactType: "container-image", CreatedAt: old},
}); err != nil {
t.Fatalf("insert manifests: %v", err)
}
// fresh: pull-only stats row, last_push explicitly NULL (the bug trigger).
if _, err := d.Exec(`
INSERT INTO repository_stats (did, repository, pull_count, last_pull, push_count, last_push)
VALUES ('did:plc:alice', 'fresh', 2, ?, 0, NULL)
`, now.Format(time.RFC3339)); err != nil {
t.Fatalf("seed fresh stats: %v", err)
}
// stale: has a genuine last_push, older than fresh's created_at.
if _, err := d.Exec(`
INSERT INTO repository_stats (did, repository, pull_count, last_pull, push_count, last_push)
VALUES ('did:plc:alice', 'stale', 0, NULL, 1, ?)
`, old.Format(time.RFC3339)); err != nil {
t.Fatalf("seed stale stats: %v", err)
}
cards, err := GetRepoCards(d, 18, "", SortByLastUpdate)
if err != nil {
t.Fatalf("GetRepoCards: %v", err)
}
if len(cards) != 2 {
t.Fatalf("expected 2 cards, got %d: %#v", len(cards), cards)
}
// fresh (recent created_at) must sort above stale (old last_push), despite
// fresh's last_push being NULL.
if cards[0].Repository != "fresh" {
t.Errorf("expected 'fresh' first (sorted by created_at), got order %q, %q", cards[0].Repository, cards[1].Repository)
}
// And its "last updated" must be populated from created_at, not blank.
var fresh *RepoCardData
for i := range cards {
if cards[i].Repository == "fresh" {
fresh = &cards[i]
}
}
if fresh == nil {
t.Fatal("'fresh' repo missing from What's New")
}
if fresh.LastUpdated.IsZero() {
t.Error("expected 'fresh' LastUpdated to fall back to created_at, got zero time")
}
}
+11 -1
View File
@@ -2,11 +2,13 @@
package oci
import (
"context"
"encoding/json"
"fmt"
"log/slog"
"net/http"
"strings"
"time"
"atcr.io/pkg/atproto"
"atcr.io/pkg/hold/pds"
@@ -189,7 +191,15 @@ func (h *XRPCHandler) HandleAbortUpload(w http.ResponseWriter, r *http.Request)
// For pulls: Just increments stats (no layer records or posts)
// Always increments stats (pull or push counts)
func (h *XRPCHandler) HandleNotifyManifest(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// Detach from the request context for the PDS writes below. AppView sends
// this notify as a best-effort fire-and-forget call (with its own 30s
// timeout), and an upstream proxy may 504 around the 10s mark. If we wrote
// layer/config/stats records on r.Context(), a client disconnect or proxy
// timeout cancels them mid-flight — leaving the manifest with missing
// records ("getb tx, context canceled"). Once we start writing to the
// embedded PDS we must finish; cap with a timeout so it can't run unbounded.
ctx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), 2*time.Minute)
defer cancel()
// Validate service token (same auth as blob:write endpoints)
validatedUser, err := pds.ValidateBlobWriteAccess(r, h.pds, h.httpClient)