mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-10 15:05:40 +00:00
bf5ab5b366
* chore: clear cosmetic staticcheck findings (TASK-764)
Apply zero-behavior-change fixes for 8 staticcheck findings on main:
- SA4023 cmd/pad/main.go:431 — drop always-true `if eventBus != nil`
guard. eventBus is wrapped in metrics.NewInstrumentedBus a few lines
above, which returns a concrete *InstrumentedBus that is never nil.
- SA1019 cmd/pad/main.go:3926 — replace deprecated strings.Title with
golang.org/x/text/cases.Title(language.English).String. golang.org/x/text
was already an indirect dep; now promoted to direct.
- SA4031 internal/server/handlers_changes.go:130 — delete dead
`if updatedItems == nil { ... }` block. make([]T, n) always returns
non-nil; the JSON marshalling already produced [] not null.
- SA9003 cmd/pad/init.go:351 — delete empty if branch and fold its
intent into the surrounding comment.
- SA9003 internal/server/handlers_dashboard.go:125 — replace empty
`if err == nil { ... }` branch with `_ = json.Unmarshal(...)` to
match the sibling settings parse and document the best-effort intent.
- SA4006 internal/cli/format.go:153 — drop the dead initial
`titlePart := item.Title` (overwritten in both branches below);
declare titlePart with `var` instead.
- SA4006 internal/store/workspaces.go:70 — drop the dead first call
to s.uniqueSlug; only the workspace-specific uniqueWorkspaceSlug
is meaningful (workspace slugs are globally unique, not workspace-
scoped like collection/item slugs).
- SA4000 internal/store/store_test.go:99 — remove always-true outer
`if idx := len(connStr) - len(connStr); idx >= 0` and unindent the
inner '?' query-string split.
go.mod side effects from `go mod tidy` under Go 1.26: golang.org/x/text
moves to direct (used directly now); pquerna/otp, prometheus/client_*
and trustelem/zxcvbn move from indirect to direct (they were already
used directly — Go 1.26's tidy correctly classifies them).
Verified:
- `go build ./...` clean
- `go vet ./...` clean
- `go test ./...` all pass (including the replaceDBName test path)
- `staticcheck -checks "SA1019,SA4000,SA4006,SA4023,SA4031,SA9003"` clean
except for handlers_dashboard.go:221 (SA4006, dashboard visibility-
filter dead block — handled in TASK-765)
Parent: PLAN-644.
* fix: clear SA5011 nil-deref in buildReconcileFindings (TASK-764)
extractItemStatus(item.Fields) on the first line of the function would
have panicked on a nil item before the `if item != nil && item.CodeContext
== nil` guard could fire. Staticcheck SA5011 flagged the inconsistency.
Drop the (item != nil) half of the guard — the function now documents
its non-nil contract in the doc comment. All callers (reconcile.go:204
plus three sites in cmd/pad/reconcile_test.go) already pass non-nil,
so this is documentation, not behaviour change.
Verified:
- `go build ./...` clean
- `go test ./cmd/pad/...` passes (the existing reconcile tests cover the
contract)
- `staticcheck -checks SA5011 ./...` clean
Parent: PLAN-644.
* chore: silence SA4017 false positive in watchCmd SSE loop (TASK-764)
cmd/pad/main.go SSE keepalive branch:
if strings.HasPrefix(line, ":") {
continue
}
Staticcheck SA4017 reports "HasPrefix doesn't have side effects and
its return value is ignored" — but the return value IS used as the
if condition. Two sibling strings.HasPrefix calls earlier in the same
for-loop body (matching "event: " and "data: " prefixes) are not
flagged, which strongly suggests an SSA-analysis quirk specific to
this branch rather than a real defect.
Suppress the finding with a //lint:ignore directive that explains
the false positive in-place. Rewriting to a different form (extract
to a bool var, comma-OK on a synthetic value, etc.) would be uglier
than the suppression comment.
Verified:
- `staticcheck -checks SA4017 ./...` clean
- `go build ./...` clean
Parent: PLAN-644.
* chore: delete dead code flagged by U1000 (TASK-764)
Pre-launch (no external contributors yet) — no consumer fork can be
relying on these unreferenced symbols, so we delete them rather than
carry the maintenance burden into v1.
## Helpers (14 functions, 1 type)
cmd/pad/main.go
- progressBar — never called
internal/cli/format.go
- stripHTMLTags — never called
internal/server/handlers_dashboard_test.go
- updateItem (test helper) — never called from any test
internal/server/handlers_items.go
- publishItemEvent — wrapper over publishItemEventWithName; all 5 call
sites use the *WithName variant directly.
- resolveRelationFields — never called.
- resolveRelationFieldFiltersForWorkspace, resolveRelationFieldFilters,
relationFilterKeys, resolveRelationFilterValue — closed loop of dead
helpers (each one only called by another dead one in the family).
- extractStatus — never called (cmd/pad/reconcile.go has its own copy).
internal/server/handlers_versions.go
- handleGetDiff (HTTP handler) — never wired into setupRouter.
- diffsToChanges, diffChange (type) — only used by handleGetDiff above.
- Removes now-unused imports `strconv` and `dmp` (sergi/go-diff).
internal/server/middleware_ratelimit.go
- writeTooManyRequests — never called; the live ratelimit middleware
uses a dedicated 429 path with Retry-After-Bucket headers.
internal/server/server.go
- guestVisibleItemIDs — never called. handlers_events.go had a
comment cross-reference; updated to drop the reference.
## Constants
internal/events/redis_bus.go
- reconnectDelay — never read.
internal/store/api_tokens.go
- defaultTokenExpiryDays — never read.
## Out of scope
The 5 unwired handlers in internal/server/handlers_documents.go are
left alone: they are the subject of TASK-769 (a product decision —
wire up vs. delete — that may want different treatment per handler).
The two SA4006/SA4010 findings on internal/server/handlers_dashboard.go
visibility-filter block are similarly left for TASK-765.
## Verified
- `go build ./...` clean
- `go vet ./...` clean
- `go test ./...` all pass
- `staticcheck -checks "SA*,U1000" ./...` clean except the two TASK-
765 / TASK-769 follow-ups noted above.
Parent: PLAN-644.
* docs: correct caller name in buildReconcileFindings doc (TASK-764)
Codex round 1 caught: the doc comment named the caller `reconcileSingle`
but the actual function is `reconcileItem` (cmd/pad/reconcile.go:204).
Fix the contract comment so it doesn't go stale on the first git blame.
146 lines
4.5 KiB
Go
146 lines
4.5 KiB
Go
package server
|
|
|
|
import (
|
|
"net/http"
|
|
"strconv"
|
|
"time"
|
|
)
|
|
|
|
// ChangesResponse is the response for GET /workspaces/{ws}/changes?since=<unix_ms>.
|
|
type ChangesResponse struct {
|
|
// Updated items (with full item data).
|
|
Updated []interface{} `json:"updated"`
|
|
// IDs of items that were deleted since the requested timestamp.
|
|
Deleted []string `json:"deleted"`
|
|
// Server timestamp at the time of this response (unix ms).
|
|
// Clients should use this as the `since` value for the next sync.
|
|
ServerTime int64 `json:"server_time"`
|
|
// Whether the collection metadata (counts, schemas) may have changed.
|
|
// True if any items were updated/deleted, signaling the client should
|
|
// also refresh collection metadata.
|
|
CollectionsChanged bool `json:"collections_changed"`
|
|
}
|
|
|
|
// handleGetChanges returns items modified since a given timestamp.
|
|
// GET /api/v1/workspaces/{ws}/changes?since=<unix_milliseconds>
|
|
//
|
|
// This is the incremental sync endpoint used by the frontend when the
|
|
// tab regains focus. Instead of re-fetching everything, the client sends
|
|
// the timestamp of its last successful sync and gets back only the delta.
|
|
func (s *Server) handleGetChanges(w http.ResponseWriter, r *http.Request) {
|
|
workspaceID, ok := s.getWorkspaceID(w, r)
|
|
if !ok {
|
|
return
|
|
}
|
|
|
|
sinceStr := r.URL.Query().Get("since")
|
|
if sinceStr == "" {
|
|
writeError(w, http.StatusBadRequest, "bad_request", "since query parameter is required (unix milliseconds)")
|
|
return
|
|
}
|
|
|
|
sinceMs, err := strconv.ParseInt(sinceStr, 10, 64)
|
|
if err != nil || sinceMs < 0 {
|
|
writeError(w, http.StatusBadRequest, "bad_request", "since must be a valid unix timestamp in milliseconds")
|
|
return
|
|
}
|
|
|
|
since := time.UnixMilli(sinceMs)
|
|
serverTime := time.Now().UnixMilli()
|
|
|
|
updated, deletedIDs, err := s.store.ItemsModifiedSince(workspaceID, since)
|
|
if err != nil {
|
|
writeInternalError(w, err)
|
|
return
|
|
}
|
|
|
|
// Filter by collection visibility so restricted members only see
|
|
// changes from collections they have access to.
|
|
visibleIDs, visErr := s.visibleCollectionIDs(r, workspaceID)
|
|
if visErr != nil {
|
|
writeInternalError(w, visErr)
|
|
return
|
|
}
|
|
|
|
// For guests with item-level grants, apply item-level filtering
|
|
// so they only see changes to items they actually have grants on.
|
|
fullCollIDs, grantedItemIDs, grantErr := s.guestResourceFilter(r, workspaceID)
|
|
if grantErr != nil {
|
|
writeInternalError(w, grantErr)
|
|
return
|
|
}
|
|
grantedItemSet := make(map[string]bool, len(grantedItemIDs))
|
|
for _, id := range grantedItemIDs {
|
|
grantedItemSet[id] = true
|
|
}
|
|
|
|
if visibleIDs != nil {
|
|
filtered := updated[:0]
|
|
allowedDeleted := deletedIDs[:0]
|
|
visibleSet := make(map[string]bool, len(visibleIDs))
|
|
for _, id := range visibleIDs {
|
|
visibleSet[id] = true
|
|
}
|
|
|
|
// For guests, build a set of full-collection-grant IDs to distinguish
|
|
// between full-collection access and item-only access.
|
|
fullCollSet := make(map[string]bool, len(fullCollIDs))
|
|
for _, id := range fullCollIDs {
|
|
fullCollSet[id] = true
|
|
}
|
|
|
|
for _, item := range updated {
|
|
if !visibleSet[item.CollectionID] {
|
|
continue
|
|
}
|
|
// For guests: if the collection is only visible via item grants,
|
|
// check the specific item is granted.
|
|
if len(grantedItemIDs) > 0 && !fullCollSet[item.CollectionID] && !grantedItemSet[item.ID] {
|
|
continue
|
|
}
|
|
filtered = append(filtered, item)
|
|
}
|
|
updated = filtered
|
|
// For deleted items, re-query with collection filter since we
|
|
// only have IDs. Fetch their collection_ids from the soft-deleted rows.
|
|
if len(deletedIDs) > 0 {
|
|
delItems, delErr := s.store.GetDeletedItemsWithCollection(workspaceID, deletedIDs)
|
|
if delErr != nil {
|
|
writeInternalError(w, delErr)
|
|
return
|
|
}
|
|
for _, item := range delItems {
|
|
if !visibleSet[item.CollectionID] {
|
|
continue
|
|
}
|
|
if len(grantedItemIDs) > 0 && !fullCollSet[item.CollectionID] && !grantedItemSet[item.ID] {
|
|
continue
|
|
}
|
|
allowedDeleted = append(allowedDeleted, item.ID)
|
|
}
|
|
deletedIDs = allowedDeleted
|
|
}
|
|
}
|
|
|
|
// Convert to interface slice for JSON marshaling. make() always
|
|
// returns a non-nil slice, which serialises to "[]" rather than
|
|
// "null", so we only need to guard deletedIDs (which can be nil
|
|
// when not initialised above).
|
|
updatedItems := make([]interface{}, len(updated))
|
|
for i, item := range updated {
|
|
updatedItems[i] = item
|
|
}
|
|
if deletedIDs == nil {
|
|
deletedIDs = []string{}
|
|
}
|
|
|
|
resp := ChangesResponse{
|
|
Updated: updatedItems,
|
|
Deleted: deletedIDs,
|
|
ServerTime: serverTime,
|
|
CollectionsChanged: len(updated) > 0 || len(deletedIDs) > 0,
|
|
}
|
|
|
|
writeJSON(w, http.StatusOK, resp)
|
|
}
|