mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(webapp): show how many times a public link has been opened (BEA-76) (#125)
Every /s/<token> hit was already recorded as a share-kind read, carrying
both a count and a timestamp — and then thrown away at the UI layer. The
Public links table showed only who shared a file and when, six inches
below a file header that already said "1 shared". Two personas filed it
independently on the same tour.
The number now rides the shares list:
* ReadLedger.ShareOpens(project) aggregates share-kind buckets per path,
all-time. Share-kind only is what makes Last mean *last opened* —
HeatEntry.LastRead is cross-kind, so a member viewing the file in the
hub would otherwise move the date.
* shareJSON takes the project's opens map, built ONCE per project by the
caller and indexed per row. Both callers — the project list and the
org-wide audit — hoist it above their loops; a per-share call would be
a full byKey scan per row.
* Counts, never openers. The share actor is token+"/"+IP, a public
credential joined to an IP, and it stays in the ledger. There is no
distinct-openers field, deliberately.
* Reads off means the keys are ABSENT, not zero: `0` would claim nobody
has opened a link on a hub that never looked.
shareDetail() is the leverage — the settings table, the org-wide audit and
the file page's share banner all render through it, so one string function
covers three surfaces. Once the row carried the receipt it truncated to
"3 op…", so the detail cell wraps instead of ellipsizing; the path keeps
its ellipsis, since it is a link with a tooltip and the column that can be
arbitrarily long.
Counted per FILE, not per link: heat is keyed by path, so two tokens on one
file report the same number. Worded that way in the section copy, alongside
the other honesty — opens are debounced visits, not requests.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
119d4abf79
commit
826d13795f
@@ -265,6 +265,7 @@ classDiagram
|
|||||||
-byKey, dirty, seen
|
-byKey, dirty, seen
|
||||||
+Record(...)
|
+Record(...)
|
||||||
+Heat(project, prefix, days)
|
+Heat(project, prefix, days)
|
||||||
|
+ShareOpens(project)
|
||||||
}
|
}
|
||||||
class ReadStat {
|
class ReadStat {
|
||||||
+Project +Path +Day +Kind +Actor +Count +Last
|
+Project +Path +Day +Kind +Actor +Count +Last
|
||||||
@@ -272,6 +273,10 @@ classDiagram
|
|||||||
class HeatEntry {
|
class HeatEntry {
|
||||||
+Human +Agent +Share +Readers +LastRead
|
+Human +Agent +Share +Readers +LastRead
|
||||||
}
|
}
|
||||||
|
class ShareOpen {
|
||||||
|
+Count +Last
|
||||||
|
}
|
||||||
|
note for ShareOpen "The receipt on a public link: share-kind buckets only, which is what makes Last mean last OPENED — HeatEntry.LastRead is cross-kind, so a member viewing the file in the hub would otherwise move the date. Counts, never openers: the share actor is token+IP. Keyed by path, so two tokens on one file report the same number. Callers build the map ONCE per project and index it; a per-share call is a full byKey scan per row"
|
||||||
|
|
||||||
class QuotaProvider {
|
class QuotaProvider {
|
||||||
<<interface>>
|
<<interface>>
|
||||||
@@ -371,6 +376,8 @@ classDiagram
|
|||||||
RemoteSource ..> sourcedOp : attribution comes from the journal key
|
RemoteSource ..> sourcedOp : attribution comes from the journal key
|
||||||
ReadLedger ..> ReadStat
|
ReadLedger ..> ReadStat
|
||||||
ReadLedger ..> HeatEntry
|
ReadLedger ..> HeatEntry
|
||||||
|
ReadLedger ..> ShareOpen
|
||||||
|
ShareDB ..> ShareOpen : shares list joins the open count per path
|
||||||
QuotaProvider <|.. UnlimitedQuota
|
QuotaProvider <|.. UnlimitedQuota
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|||||||
@@ -77,8 +77,12 @@ func (s *Server) handleOrgShares(w http.ResponseWriter, r *http.Request) {
|
|||||||
if !atLeast(s.projectPermOf(r, p), PermRead) {
|
if !atLeast(s.projectPermOf(r, p), PermRead) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
// One scan per visible project, zero per share — and after the
|
||||||
|
// permission check, since there is no reason to scan for a project
|
||||||
|
// the caller cannot see.
|
||||||
|
opens := s.Reads.ShareOpens(p.ID)
|
||||||
for _, sh := range s.Shares.List(p.ID) {
|
for _, sh := range s.Shares.List(p.ID) {
|
||||||
j := shareJSON(r, sh)
|
j := shareJSON(r, sh, opens)
|
||||||
j["project_name"] = p.Name
|
j["project_name"] = p.Name
|
||||||
out = append(out, j)
|
out = append(out, j)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -214,6 +214,13 @@ export interface ShareInfo {
|
|||||||
creator?: string;
|
creator?: string;
|
||||||
created?: string;
|
created?: string;
|
||||||
expires?: string;
|
expires?: string;
|
||||||
|
/* Share-link receipts (shareJSON, shares.go). Both keys are ABSENT — not
|
||||||
|
zero — when the hub has read telemetry off, which is why opens must be
|
||||||
|
tested with `=== undefined` and never with a falsy check: 0 is a real,
|
||||||
|
meaningful value ("minted, never opened"). Counted per FILE, not per
|
||||||
|
link, because heat is keyed by path. */
|
||||||
|
opens?: number;
|
||||||
|
last_opened?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
// GET/POST /api/admin/policy (handleAdminPolicy, admin.go)
|
// GET/POST /api/admin/policy (handleAdminPolicy, admin.go)
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ import { modalConfirm, modalPrompt } from "../modal";
|
|||||||
import { toast } from "../toast";
|
import { toast } from "../toast";
|
||||||
import { useHubRefresh, usePermissions, useShares } from "../hooks/useHub";
|
import { useHubRefresh, usePermissions, useShares } from "../hooks/useHub";
|
||||||
import { PROJECT_ICONS, ProjectIcon } from "./shell";
|
import { PROJECT_ICONS, ProjectIcon } from "./shell";
|
||||||
import { SharesTable } from "./SharesTable";
|
import { OPENS_NOTE, SharesTable } from "./SharesTable";
|
||||||
import { projColor } from "./ProjectNav";
|
import { projColor } from "./ProjectNav";
|
||||||
import { Button } from "@/components/ui/button";
|
import { Button } from "@/components/ui/button";
|
||||||
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
|
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
|
||||||
@@ -304,6 +304,9 @@ function PublicLinks({ project }: { project: Project }) {
|
|||||||
<CardTitle>Public links</CardTitle>
|
<CardTitle>Public links</CardTitle>
|
||||||
<CardDescription>
|
<CardDescription>
|
||||||
Files in this project that anyone with the URL can read — no account needed.
|
Files in this project that anyone with the URL can read — no account needed.
|
||||||
|
{/* Only when the hub actually measures opens: on a hub with read
|
||||||
|
telemetry off there is no number, so promising one would lie. */}
|
||||||
|
{(shares || []).some((s) => s.opens !== undefined) && <> {OPENS_NOTE}</>}
|
||||||
</CardDescription>
|
</CardDescription>
|
||||||
</CardHeader>
|
</CardHeader>
|
||||||
<Separator />
|
<Separator />
|
||||||
|
|||||||
@@ -3,7 +3,7 @@ import type { ShareInfo } from "../api/types";
|
|||||||
import { copyText } from "../util";
|
import { copyText } from "../util";
|
||||||
import { toast } from "../toast";
|
import { toast } from "../toast";
|
||||||
import { Icon } from "./shell";
|
import { Icon } from "./shell";
|
||||||
import { revokeShare, shareDetail } from "./SharesTable";
|
import { OPENS_NOTE, revokeShare, shareDetail } from "./SharesTable";
|
||||||
|
|
||||||
/* A file that is publicly reachable says so while you are reading it. The
|
/* A file that is publicly reachable says so while you are reading it. The
|
||||||
Share dialog used to be the only place the link — and its Revoke button —
|
Share dialog used to be the only place the link — and its Revoke button —
|
||||||
@@ -42,6 +42,9 @@ export function ShareBanner({
|
|||||||
<p className="sb-note">
|
<p className="sb-note">
|
||||||
<b>Anyone with this link can view this file</b> — no account needed. It always shows the
|
<b>Anyone with this link can view this file</b> — no account needed. It always shows the
|
||||||
latest version until you revoke it.
|
latest version until you revoke it.
|
||||||
|
{/* Same gate as the settings table: say nothing about opens on a hub
|
||||||
|
that does not measure them. */}
|
||||||
|
{shares.some((s) => s.opens !== undefined) && <> {OPENS_NOTE}</>}
|
||||||
</p>
|
</p>
|
||||||
{shares.map((s) => (
|
{shares.map((s) => (
|
||||||
<div className="sb-link" key={s.token}>
|
<div className="sb-link" key={s.token}>
|
||||||
|
|||||||
@@ -26,15 +26,37 @@ export function expiryLabel(expires?: string): string {
|
|||||||
return expires ? "expires " + new Date(expires).toLocaleDateString() : "no expiry";
|
return expires ? "expires " + new Date(expires).toLocaleDateString() : "no expiry";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The receipt: did anyone actually open this? The server already recorded
|
||||||
|
// every /s/<token> hit — this is the only place the number gets read back.
|
||||||
|
// `undefined` means the hub has read telemetry off, and then we say nothing
|
||||||
|
// at all; `0` is a real answer and gets one. Hence the explicit undefined
|
||||||
|
// check — `!s.opens` would collapse "not measured" into "never opened".
|
||||||
|
function opensLabel(s: ShareInfo): string | null {
|
||||||
|
if (s.opens === undefined) return null;
|
||||||
|
if (s.opens === 0) return "not opened yet";
|
||||||
|
const n = `${s.opens} open${s.opens === 1 ? "" : "s"}`;
|
||||||
|
return s.last_opened ? `${n} · last opened ${new Date(s.last_opened).toLocaleDateString()}` : n;
|
||||||
|
}
|
||||||
|
|
||||||
export function shareDetail(s: ShareInfo, showProject: boolean): string {
|
export function shareDetail(s: ShareInfo, showProject: boolean): string {
|
||||||
const bits: string[] = [];
|
const bits: string[] = [];
|
||||||
if (showProject && s.project_name) bits.push(s.project_name);
|
if (showProject && s.project_name) bits.push(s.project_name);
|
||||||
if (s.creator) bits.push("by " + s.creator);
|
if (s.creator) bits.push("by " + s.creator);
|
||||||
if (s.created) bits.push(new Date(s.created).toLocaleDateString());
|
if (s.created) bits.push(new Date(s.created).toLocaleDateString());
|
||||||
bits.push(expiryLabel(s.expires));
|
bits.push(expiryLabel(s.expires));
|
||||||
|
const opens = opensLabel(s);
|
||||||
|
if (opens) bits.push(opens);
|
||||||
return bits.join(" · ");
|
return bits.join(" · ");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The two honesties about the number, worded once per section rather than
|
||||||
|
// once per row: opens are debounced VISITS (readDebounce, reads.go), and the
|
||||||
|
// count is per FILE not per link (heat is keyed by path), so two tokens on
|
||||||
|
// one file report the same number.
|
||||||
|
export const OPENS_NOTE =
|
||||||
|
"Opens count how many times a file has been read through a public link. " +
|
||||||
|
"Repeat opens by the same reader within 10 minutes count once.";
|
||||||
|
|
||||||
export function SharesTable({
|
export function SharesTable({
|
||||||
shares,
|
shares,
|
||||||
onChanged,
|
onChanged,
|
||||||
|
|||||||
@@ -228,6 +228,14 @@ button, input, a.btn { font-family: inherit; }
|
|||||||
like the members table. At 186px it stole the room the row's own facts
|
like the members table. At 186px it stole the room the row's own facts
|
||||||
(who, when, whether it expires) needed, and they truncated to "no exp…". */
|
(who, when, whether it expires) needed, and they truncated to "no exp…". */
|
||||||
.shares-table .admin-table th:last-child, .shares-table .admin-table td:last-child { width: 110px; }
|
.shares-table .admin-table th:last-child, .shares-table .admin-table td:last-child { width: 110px; }
|
||||||
|
/* …and once the row also carries the link's open count (BEA-76), narrowing
|
||||||
|
the button column stopped being enough: the detail ran to "3 op…" and the
|
||||||
|
receipt — the whole point of showing it — was the part that fell off. So
|
||||||
|
this one cell wraps instead of ellipsizing. Nothing here is a fixed width
|
||||||
|
worth protecting, and a two-line detail on a narrow viewport beats a
|
||||||
|
truncated one at every width. The path keeps its ellipsis: it is a link
|
||||||
|
with a title tooltip, and it is the column that can be arbitrarily long. */
|
||||||
|
.shares-table .admin-table td .ai-tag { white-space: normal; overflow: visible; text-overflow: clip; }
|
||||||
|
|
||||||
/* ---- file-page "this file is public" banner ---- */
|
/* ---- file-page "this file is public" banner ---- */
|
||||||
.share-banner {
|
.share-banner {
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import (
|
|||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
|
"sync/atomic"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/runbear-io/beardrive/internal/journal"
|
"github.com/runbear-io/beardrive/internal/journal"
|
||||||
@@ -99,6 +100,12 @@ type ReadLedger struct {
|
|||||||
repo ReadRepo
|
repo ReadRepo
|
||||||
retention time.Duration
|
retention time.Duration
|
||||||
|
|
||||||
|
// scans counts ShareOpens passes over byKey. Tests assert one per
|
||||||
|
// project per list render — the "never one scan per share" rule is
|
||||||
|
// invisible in the response body, so this is the only thing that can
|
||||||
|
// catch the regression.
|
||||||
|
scans atomic.Int64
|
||||||
|
|
||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
byKey map[ReadStatKey]ReadStat
|
byKey map[ReadStatKey]ReadStat
|
||||||
dirty map[ReadStatKey]bool
|
dirty map[ReadStatKey]bool
|
||||||
@@ -272,6 +279,51 @@ func (l *ReadLedger) AgentHeat(project string, since time.Time) map[string]map[s
|
|||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ShareOpen is share-link consumption for one path: visits, and when.
|
||||||
|
type ShareOpen struct {
|
||||||
|
Count int64
|
||||||
|
Last time.Time
|
||||||
|
}
|
||||||
|
|
||||||
|
// ShareOpens aggregates share-kind reads per path for one project — the
|
||||||
|
// receipt a person who shared something actually wants. All-time, because a
|
||||||
|
// link's lifetime is the question a receipt answers.
|
||||||
|
//
|
||||||
|
// Share buckets only, and that is what makes Last mean *last opened*:
|
||||||
|
// HeatEntry.LastRead is cross-kind, so a member viewing the file in the hub
|
||||||
|
// would otherwise move the "opened through the link" date.
|
||||||
|
//
|
||||||
|
// Counts, never identities — the share actor is token+"/"+IP, a public
|
||||||
|
// credential joined to an IP, and it must not leave the ledger. There is
|
||||||
|
// deliberately no distinct-openers field.
|
||||||
|
//
|
||||||
|
// One byKey scan per project, never one per share: callers build this map
|
||||||
|
// once and index it, because byKey is the full map and a project with 40
|
||||||
|
// links would otherwise pay 40 full scans per list render.
|
||||||
|
func (l *ReadLedger) ShareOpens(project string) map[string]ShareOpen {
|
||||||
|
if l == nil {
|
||||||
|
return nil // reads disabled: absent, not zero
|
||||||
|
}
|
||||||
|
l.scans.Add(1)
|
||||||
|
out := map[string]ShareOpen{}
|
||||||
|
l.mu.Lock()
|
||||||
|
defer l.mu.Unlock()
|
||||||
|
for key, st := range l.byKey {
|
||||||
|
if key.Project != project || key.Kind != ReadKindShare {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
// No day filter: both the daily buckets and the folded Day == ""
|
||||||
|
// all-time row count.
|
||||||
|
e := out[key.Path]
|
||||||
|
e.Count += st.Count
|
||||||
|
if st.Last.After(e.Last) {
|
||||||
|
e.Last = st.Last
|
||||||
|
}
|
||||||
|
out[key.Path] = e
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
// Close flushes any pending buckets.
|
// Close flushes any pending buckets.
|
||||||
func (l *ReadLedger) Close() error {
|
func (l *ReadLedger) Close() error {
|
||||||
if l == nil {
|
if l == nil {
|
||||||
|
|||||||
@@ -146,12 +146,69 @@ func TestReadLedgerRetentionFold(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestShareOpens pins the receipt accessor: share buckets only (so Last
|
||||||
|
// really means *last opened*), all-time (a link's lifetime is the question),
|
||||||
|
// and counts with no trace of the actor, which is token+"/"+IP.
|
||||||
|
func TestShareOpens(t *testing.T) {
|
||||||
|
repo := newFileReadRepo(filepath.Join(t.TempDir(), "reads.json"))
|
||||||
|
old := time.Date(2026, 3, 1, 9, 0, 0, 0, time.UTC)
|
||||||
|
newer := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC)
|
||||||
|
if err := repo.PutBatch([]ReadStat{
|
||||||
|
// Two share buckets on one path, one of them the retention fold:
|
||||||
|
// all-time means both count, so no day filter may creep in.
|
||||||
|
{Project: "p-1", Path: "a.md", Day: "", Kind: ReadKindShare, Actor: "tok/1.2.3.4", Count: 2, Last: old},
|
||||||
|
{Project: "p-1", Path: "a.md", Day: "2026-03-02", Kind: ReadKindShare, Actor: "tok/5.6.7.8", Count: 1, Last: old.Add(time.Hour)},
|
||||||
|
// A NEWER human read of the same path. It must move neither the
|
||||||
|
// count nor the date — this is the assertion that pins the kind
|
||||||
|
// filter, since HeatEntry.LastRead would happily report it.
|
||||||
|
{Project: "p-1", Path: "a.md", Day: "2026-08-01", Kind: ReadKindHuman, Actor: "alice@x.io", Count: 40, Last: newer},
|
||||||
|
{Project: "p-1", Path: "a.md", Day: "2026-08-01", Kind: ReadKindAgent, Actor: "dev1", Count: 9, Last: newer},
|
||||||
|
// A share read in another project must not leak across.
|
||||||
|
{Project: "p-2", Path: "a.md", Day: "2026-03-02", Kind: ReadKindShare, Actor: "tok/9.9.9.9", Count: 7, Last: newer},
|
||||||
|
// A path with no share reads at all is simply absent from the map.
|
||||||
|
{Project: "p-1", Path: "b.md", Day: "2026-08-01", Kind: ReadKindHuman, Actor: "alice@x.io", Count: 3, Last: newer},
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
l, err := NewReadLedger(repo, 0)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
opens := l.ShareOpens("p-1")
|
||||||
|
if got := opens["a.md"]; got.Count != 3 {
|
||||||
|
t.Fatalf("a.md opens = %d, want 3 (share buckets only, all-time)", got.Count)
|
||||||
|
}
|
||||||
|
if got := opens["a.md"].Last; !got.Equal(old.Add(time.Hour)) {
|
||||||
|
t.Fatalf("a.md last opened = %s, want %s — a newer human read must not move it", got, old.Add(time.Hour))
|
||||||
|
}
|
||||||
|
if _, ok := opens["b.md"]; ok {
|
||||||
|
t.Fatal("a path read only by humans must not appear in the opens map")
|
||||||
|
}
|
||||||
|
if got := l.ShareOpens("p-2")["a.md"].Count; got != 7 {
|
||||||
|
t.Fatalf("p-2 a.md opens = %d, want 7", got)
|
||||||
|
}
|
||||||
|
// Enabled-but-empty is an empty map, not nil: nil is reserved for "reads
|
||||||
|
// are off", which is what the wire format turns into an absent key.
|
||||||
|
if m := l.ShareOpens("p-nothing"); m == nil {
|
||||||
|
t.Fatal("a project with no share reads must yield an empty map, not nil")
|
||||||
|
}
|
||||||
|
// Live recording lands in the same map, debounced to visits.
|
||||||
|
l.Record("p-1", "c.md", ReadKindShare, "tok2/1.1.1.1")
|
||||||
|
l.Record("p-1", "c.md", ReadKindShare, "tok2/1.1.1.1") // same opener, inside the window
|
||||||
|
if got := l.ShareOpens("p-1")["c.md"].Count; got != 1 {
|
||||||
|
t.Fatalf("c.md opens = %d, want 1 — repeat opens inside the debounce window are one visit", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestReadLedgerNil(t *testing.T) {
|
func TestReadLedgerNil(t *testing.T) {
|
||||||
var l *ReadLedger
|
var l *ReadLedger
|
||||||
l.Record("p-1", "a.md", ReadKindHuman, "x") // must not panic
|
l.Record("p-1", "a.md", ReadKindHuman, "x") // must not panic
|
||||||
if l.Heat("p-1", "", time.Time{}) != nil {
|
if l.Heat("p-1", "", time.Time{}) != nil {
|
||||||
t.Fatal("nil ledger heat should be nil")
|
t.Fatal("nil ledger heat should be nil")
|
||||||
}
|
}
|
||||||
|
if l.ShareOpens("p-1") != nil {
|
||||||
|
t.Fatal("nil ledger share opens should be nil — reads off means absent, not zero")
|
||||||
|
}
|
||||||
if err := l.Close(); err != nil {
|
if err := l.Close(); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -280,7 +280,8 @@ func (s *Server) handleShareCreate(v *volume, w http.ResponseWriter, r *http.Req
|
|||||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
writeJSON(w, shareJSON(r, sh))
|
// No opens: a freshly minted link has nothing to report.
|
||||||
|
writeJSON(w, shareJSON(r, sh, nil))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Server) handleShareList(v *volume, w http.ResponseWriter, r *http.Request) {
|
func (s *Server) handleShareList(v *volume, w http.ResponseWriter, r *http.Request) {
|
||||||
@@ -288,10 +289,12 @@ func (s *Server) handleShareList(v *volume, w http.ResponseWriter, r *http.Reque
|
|||||||
http.Error(w, "sharing is not enabled on this server", http.StatusNotFound)
|
http.Error(w, "sharing is not enabled on this server", http.StatusNotFound)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
shares := s.Shares.List(r.PathValue("project"))
|
project := r.PathValue("project")
|
||||||
|
shares := s.Shares.List(project)
|
||||||
|
opens := s.Reads.ShareOpens(project) // once, outside the loop — never per share
|
||||||
out := make([]map[string]any, 0, len(shares))
|
out := make([]map[string]any, 0, len(shares))
|
||||||
for _, sh := range shares {
|
for _, sh := range shares {
|
||||||
out = append(out, shareJSON(r, sh))
|
out = append(out, shareJSON(r, sh, opens))
|
||||||
}
|
}
|
||||||
writeJSON(w, map[string]any{"shares": out})
|
writeJSON(w, map[string]any{"shares": out})
|
||||||
}
|
}
|
||||||
@@ -359,10 +362,16 @@ func (s *Server) handleShareExpiry(w http.ResponseWriter, r *http.Request) {
|
|||||||
http.Error(w, "no such share", http.StatusNotFound)
|
http.Error(w, "no such share", http.StatusNotFound)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
writeJSON(w, shareJSON(r, updated))
|
// Same as create: an expiry edit is not the surface that reports opens.
|
||||||
|
writeJSON(w, shareJSON(r, updated, nil))
|
||||||
}
|
}
|
||||||
|
|
||||||
func shareJSON(r *http.Request, sh Share) map[string]any {
|
// shareJSON renders one link. opens is the project's share-open map from
|
||||||
|
// ReadLedger.ShareOpens, built ONCE by the caller and indexed here — passing
|
||||||
|
// nil means "not measured" (reads are off, or this is a single-share reply
|
||||||
|
// that has nothing to report yet), and then neither receipt key appears.
|
||||||
|
// Absent is not zero: `0` would be a lie on a hub with reads disabled.
|
||||||
|
func shareJSON(r *http.Request, sh Share, opens map[string]ShareOpen) map[string]any {
|
||||||
out := map[string]any{
|
out := map[string]any{
|
||||||
"token": sh.Token, "path": sh.Path, "project": sh.Project,
|
"token": sh.Token, "path": sh.Path, "project": sh.Project,
|
||||||
"url": requestBaseURL(r) + "/s/" + sh.Token, "created": sh.Created,
|
"url": requestBaseURL(r) + "/s/" + sh.Token, "created": sh.Created,
|
||||||
@@ -373,6 +382,16 @@ func shareJSON(r *http.Request, sh Share) map[string]any {
|
|||||||
if !sh.Expires.IsZero() {
|
if !sh.Expires.IsZero() {
|
||||||
out["expires"] = sh.Expires
|
out["expires"] = sh.Expires
|
||||||
}
|
}
|
||||||
|
if opens != nil {
|
||||||
|
// Keyed by path, not token: heat has no token dimension, so two
|
||||||
|
// links on one file report the same number. Documented, and asserted
|
||||||
|
// in shares_test.go so it can't regress into a silent wrong answer.
|
||||||
|
o := opens[sh.Path]
|
||||||
|
out["opens"] = o.Count
|
||||||
|
if o.Count > 0 {
|
||||||
|
out["last_opened"] = o.Last
|
||||||
|
}
|
||||||
|
}
|
||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -473,6 +473,167 @@ func TestShareDarkThemeIsLast(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// listShares reads the project's share list as the signed-in sharer.
|
||||||
|
func listShares(t *testing.T, srv *Server, h http.Handler, project string) []map[string]any {
|
||||||
|
t.Helper()
|
||||||
|
req := jsonReq(t, "GET", "/api/p/"+project+"/shares", nil)
|
||||||
|
authAs(t, srv, req)
|
||||||
|
rec := doHTTP(h, req)
|
||||||
|
if rec.Code != 200 {
|
||||||
|
t.Fatalf("list shares: %d %s", rec.Code, rec.Body)
|
||||||
|
}
|
||||||
|
var out struct {
|
||||||
|
Shares []map[string]any `json:"shares"`
|
||||||
|
}
|
||||||
|
if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
return out.Shares
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestShareOpensOnTheWire: the number was always recorded and always thrown
|
||||||
|
// away at the UI layer (BEA-76). It now rides the shares list — as a count,
|
||||||
|
// never as an opener.
|
||||||
|
func TestShareOpensOnTheWire(t *testing.T) {
|
||||||
|
srv, p, _, _, h := shareHub(t)
|
||||||
|
var err error
|
||||||
|
if srv.Reads, err = OpenReadLedger(filepath.Join(t.TempDir(), "reads.json"), 0); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
token, _ := authedShare(t, srv, h, p.ID, "wiki/report.html")
|
||||||
|
unopened, _ := authedShare(t, srv, h, p.ID, "wiki/notes.md")
|
||||||
|
|
||||||
|
// A freshly minted link reports zero — not absent. "Minted, never
|
||||||
|
// opened" is a real answer and the UI words it as "not opened yet".
|
||||||
|
byToken := map[string]map[string]any{}
|
||||||
|
for _, s := range listShares(t, srv, h, p.ID) {
|
||||||
|
byToken[s["token"].(string)] = s
|
||||||
|
}
|
||||||
|
if got, ok := byToken[unopened]["opens"]; !ok || got.(float64) != 0 {
|
||||||
|
t.Fatalf("unopened link opens = %v (present %v), want 0", got, ok)
|
||||||
|
}
|
||||||
|
if _, ok := byToken[unopened]["last_opened"]; ok {
|
||||||
|
t.Fatal("a never-opened link must carry no last_opened")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Two hits from one client inside the debounce window are one visit.
|
||||||
|
for i := 0; i < 2; i++ {
|
||||||
|
if rec := do(t, h, "GET", "/s/"+token, nil); rec.Code != 200 {
|
||||||
|
t.Fatalf("public fetch %d: %d %s", i, rec.Code, rec.Body)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
byToken = map[string]map[string]any{}
|
||||||
|
for _, s := range listShares(t, srv, h, p.ID) {
|
||||||
|
byToken[s["token"].(string)] = s
|
||||||
|
}
|
||||||
|
if got := byToken[token]["opens"].(float64); got != 1 {
|
||||||
|
t.Fatalf("opens = %v after two hits in the debounce window, want 1", got)
|
||||||
|
}
|
||||||
|
if _, ok := byToken[token]["last_opened"]; !ok {
|
||||||
|
t.Fatal("an opened link must carry last_opened")
|
||||||
|
}
|
||||||
|
|
||||||
|
// The actor is token+"/"+IP — a public credential joined to an IP. It
|
||||||
|
// must not appear anywhere in the response, in any shape.
|
||||||
|
req := jsonReq(t, "GET", "/api/p/"+p.ID+"/shares", nil)
|
||||||
|
authAs(t, srv, req)
|
||||||
|
body := doHTTP(h, req).Body.String()
|
||||||
|
for _, leak := range []string{token + "/", "192.0.2.1", "actor", "openers"} {
|
||||||
|
if strings.Contains(body, leak) {
|
||||||
|
t.Fatalf("shares response leaks %q: %s", leak, body)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Two tokens on one path report the SAME count: heat is keyed by path,
|
||||||
|
// not by token. Documented behavior — asserted so it cannot regress into
|
||||||
|
// a silently wrong per-link number.
|
||||||
|
second := Share{Token: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Project: p.ID,
|
||||||
|
Path: "wiki/report.html", Creator: "s@x.io", Created: time.Now().UTC()}
|
||||||
|
srv.Shares.mu.Lock()
|
||||||
|
srv.Shares.byToken[second.Token] = second
|
||||||
|
srv.Shares.mu.Unlock()
|
||||||
|
if err := srv.Shares.repo.Put(second); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
byToken = map[string]map[string]any{}
|
||||||
|
for _, s := range listShares(t, srv, h, p.ID) {
|
||||||
|
byToken[s["token"].(string)] = s
|
||||||
|
}
|
||||||
|
if a, b := byToken[token]["opens"], byToken[second.Token]["opens"]; a != b {
|
||||||
|
t.Fatalf("two links on one path report %v and %v; heat is keyed by path, so they must match", a, b)
|
||||||
|
}
|
||||||
|
|
||||||
|
// One byKey scan per project per render — never one per share. Three
|
||||||
|
// links are listed below; a per-share implementation would scan 3×.
|
||||||
|
before := srv.Reads.scans.Load()
|
||||||
|
listShares(t, srv, h, p.ID)
|
||||||
|
if got := srv.Reads.scans.Load() - before; got != 1 {
|
||||||
|
t.Fatalf("listing 3 shares performed %d ShareOpens scans, want exactly 1", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reads off: neither key, and no panic on the nil ledger. Absent means
|
||||||
|
// "not measured"; a 0 here would claim nobody has opened the link.
|
||||||
|
srv.Reads = nil
|
||||||
|
for _, s := range listShares(t, srv, h, p.ID) {
|
||||||
|
if _, ok := s["opens"]; ok {
|
||||||
|
t.Fatalf("reads disabled must omit opens entirely: %v", s)
|
||||||
|
}
|
||||||
|
if _, ok := s["last_opened"]; ok {
|
||||||
|
t.Fatalf("reads disabled must omit last_opened entirely: %v", s)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The org-wide audit table is the second caller of shareJSON, and the place
|
||||||
|
// the one-scan-per-project rule is easiest to break (it loops projects).
|
||||||
|
func TestOrgSharesCarryOpens(t *testing.T) {
|
||||||
|
h, srv, c, p := permHub(t)
|
||||||
|
var err error
|
||||||
|
if srv.Reads, err = OpenReadLedger(filepath.Join(t.TempDir(), "reads.json"), 0); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
for _, path := range []string{"a.md", "b.md", "c.md"} {
|
||||||
|
if _, err := srv.Shares.Create(p.ID, path, "alice@x.io", 0); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
srv.Reads.Record(p.ID, "a.md", ReadKindShare, "tok/203.0.113.7")
|
||||||
|
srv.Reads.Record(p.ID, "b.md", ReadKindHuman, "alice@x.io") // not an open
|
||||||
|
|
||||||
|
before := srv.Reads.scans.Load()
|
||||||
|
rec := doAs(t, h, "GET", "/api/orgs/"+p.Org+"/shares", nil, c["alice"])
|
||||||
|
if rec.Code != 200 {
|
||||||
|
t.Fatalf("org shares: %d %s", rec.Code, rec.Body)
|
||||||
|
}
|
||||||
|
if got := srv.Reads.scans.Load() - before; got != 1 {
|
||||||
|
t.Fatalf("org audit over 1 project with 3 shares scanned %d times, want 1", got)
|
||||||
|
}
|
||||||
|
var out struct {
|
||||||
|
Shares []struct {
|
||||||
|
Path string `json:"path"`
|
||||||
|
Opens *int64 `json:"opens"`
|
||||||
|
} `json:"shares"`
|
||||||
|
}
|
||||||
|
if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if len(out.Shares) != 3 {
|
||||||
|
t.Fatalf("want 3 org share rows, got %d", len(out.Shares))
|
||||||
|
}
|
||||||
|
for _, s := range out.Shares {
|
||||||
|
if s.Opens == nil {
|
||||||
|
t.Fatalf("%s carries no opens on a reads-enabled hub", s.Path)
|
||||||
|
}
|
||||||
|
want := int64(0)
|
||||||
|
if s.Path == "a.md" {
|
||||||
|
want = 1
|
||||||
|
}
|
||||||
|
if *s.Opens != want {
|
||||||
|
t.Fatalf("%s opens = %d, want %d (a human read is not an open)", s.Path, *s.Opens, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func jsonReq(t *testing.T, method, url string, body any) *http.Request {
|
func jsonReq(t *testing.T, method, url string, body any) *http.Request {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
var data []byte
|
var data []byte
|
||||||
|
|||||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
+15
-15
File diff suppressed because one or more lines are too long
@@ -5,8 +5,8 @@
|
|||||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||||
<title>BearDrive</title>
|
<title>BearDrive</title>
|
||||||
<link rel="icon" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32' fill='%23f5a623'><rect x='4' y='4' width='5.6' height='24'/><rect x='11.2' y='4' width='14.4' height='11.2'/><rect x='11.2' y='16.8' width='16.8' height='11.2'/></svg>">
|
<link rel="icon" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32' fill='%23f5a623'><rect x='4' y='4' width='5.6' height='24'/><rect x='11.2' y='4' width='14.4' height='11.2'/><rect x='11.2' y='16.8' width='16.8' height='11.2'/></svg>">
|
||||||
<script type="module" crossorigin src="/assets/index-Cmjeu7KJ.js"></script>
|
<script type="module" crossorigin src="/assets/index-CM4CrIy-.js"></script>
|
||||||
<link rel="stylesheet" crossorigin href="/assets/index-7wX--nX5.css">
|
<link rel="stylesheet" crossorigin href="/assets/index-C62PcDae.css">
|
||||||
</head>
|
</head>
|
||||||
<body>
|
<body>
|
||||||
<div id="root"></div>
|
<div id="root"></div>
|
||||||
|
|||||||
Reference in New Issue
Block a user