diff --git a/.agents/skills/jaws/SKILL.md b/.agents/skills/jaws/SKILL.md index 17d07b53..83c67a5f 100644 --- a/.agents/skills/jaws/SKILL.md +++ b/.agents/skills/jaws/SKILL.md @@ -109,10 +109,33 @@ These are the two usual building blocks for widget handlers passed to `$.Button` - The root dot **must** be comparable at runtime and equal to itself: `ui.NewTemplate` returns a value, so the dot is part of the widget the container widgets use as a map key. A slice, map, func or NaN-bearing dot makes the widget unusable as a container child. -- Implementing `JawsGetTag(tag.Context) any` does **not** fix a non-comparable dot — it +- Implementing `JawsGetTag() any` does **not** fix a non-comparable dot — it resolves the *tag*, not the widget's comparability. A non-comparable dot is unsupported. Always use the Template itself as a value; taking its address is unsupported because it changes container reuse to pointer identity. `ui.Handler` is the arbitrary-dot exception. +- `JawsGetTag` is the canonical public accessor for an object's tags, and application code may + call it directly. Callers needing flattened, validated keys pass the object to + `tag.TagExpand`. It takes no context argument: a tag value expands the same way regardless + of which request or goroutine expands it. +- `Element.ApplyGetter` invokes `JawsGetTag` to obtain a tag candidate, then expands that + candidate for registration. This expansion may invoke `JawsGetTag` again when the candidate + is itself a `tag.TagGetter` or contains one. Standard getter-backed widgets invoke + `ApplyGetter` once during initial render, but there is no `JawsGetTag` call-count guarantee: + `tag.TagExpand`, dirtying, broadcasts, and application code may make further calls. +- Except for an explicitly documented initialization phase that returns nil, a `tag.TagGetter` + must be idempotent in tag identity. After its first non-nil result, every call must return a + value that `tag.TagExpand` expands to the same set of keys. Previously returned containers + must continue expanding to the key set they produced when returned and must be treated as + read-only. Fresh containers and equivalent representations are allowed. Non-idempotent + `tag.TagGetter` implementations are unsupported. +- JaWS does not serialize `JawsGetTag` calls. A getter used concurrently must synchronize its + state and safely publish any returned containers. +- `ui.JsVar` is the one in-tree initialization case: `JawsGetTag` returns nil before its first + render initializes the dirty tag, and that nil is not a dirty target. A getter in its nil phase + is therefore not usable as a tag: passing a not-yet-rendered `JsVar` as a tag to another widget + expands to no keys, so that widget registers under nothing and a later dirty of the JsVar never + reaches it. Render the JsVar first, or tag the other widget with an independent value. `ui.Object` + propagates the phase of any chained getter that has one. - `tag.TagExpand` rejects exactly these as tags: `string`, `bool`, `int`/`int8`/`int16`/`int32`/`int64`, `uint`/`uint8`/`uint16`/`uint32`/`uint64`, `float32`/`float64`, `template.HTML`, `template.HTMLAttr`, `jid.Jid` and `key.Key`. It is a switch on exact types, so aliases of a rejected type are rejected, @@ -198,12 +221,14 @@ For clickable content rendering: ## Dirtying rules -- Prefer `Request.Dirty(...)` when in request context. -- Avoid `Jaws.Dirty(...)` unless necessary; its tag expansion runs with nil request context. +- `Request.Dirty` and `Jaws.Dirty` are equivalent: both expand through `Jaws.MustTagExpand` and + dirty every matching element across all live Requests. `Request.Dirty` is *not* scoped to its + own Request. - Dirty only precise tags whose output depends on the changed state. - Avoid broad model-level dirty tags when finer-grained element-level tags are practical. - For broad refreshes, attach a shared dependency tag to all relevant elements and dirty that shared tag instead of enumerating many element tags. -- `Request.Dirty` runs the tag list through `tag.TagExpand`, which has a hard cap of 100 expanded entries and returns `tag.ErrTooManyTags` above that. When a mutation might touch more items than that, prefer the shared group tag over enumerating individual item tags. +- `Request.Dirty` runs the tag list through `Jaws.MustTagExpand`, which has a hard cap of 100 expanded entries. Above that, expansion fails with `tag.ErrTooManyTags`: with a `Jaws.Logger` configured the error is logged and the partial expansion is still applied, and without one the call panics before anything is dirtied. When a mutation might touch more items than that, prefer the shared group tag over enumerating individual item tags. +- `Request.TagsOf(elem)` reports every tag actually registered on an element, including tags added separately from the UI object — use it when a dirty target seems not to reach an element. - Redundant-update filtering is asymmetric: input widgets (`InputText`, `InputBool`, `InputFloat`, `InputDate`) compare the new getter output against a stored `Last` value and skip `SetValue` when unchanged, but `HTMLInner`-backed widgets (spans, divs, buttons) do not — `JawsUpdate` unconditionally calls `SetInner`. For HTML-inner widgets, ensure dirty scope matches fields that actually changed, otherwise unrelated status/label spans will re-render (and lose selection, transitions, etc.) on every event. Usually the mutation code already knows what it changed and can dirty accordingly; fall back to snapshot-and-diff only when outcomes are hard to predict up front (e.g. flood-fill or win-condition checks) and the snapshot is cheap. ## HTML safety rules diff --git a/broadcast.go b/broadcast.go index ad990d6c..a95ba58c 100644 --- a/broadcast.go +++ b/broadcast.go @@ -37,10 +37,10 @@ import ( // Dest is expanded into tags. Plain strings and [Jid] values are illegal tag // types; use [tag.Tag], a domain tag, or an [Element] method instead. // -// A [wire.Message.Dest] that cannot be expanded into tags (an illegal tag type) -// is reported through [Jaws.MustLog], which panics when no [Jaws.Logger] is -// set; with a Logger the error is logged and the message is sent to the -// destinations that did expand. +// That expansion runs through [Jaws.MustTagExpand], which reports a failure such as an +// illegal tag type through [Jaws.MustLog]: that panics when no [Jaws.Logger] is set, +// while with a Logger the error is logged and the message is sent to the destinations +// that did expand. func (jw *Jaws) Broadcast(msg wire.Message) { switch msg.What { case what.Replace: @@ -61,9 +61,7 @@ func (jw *Jaws) Broadcast(msg wire.Message) { return } default: - expanded, err := tag.TagExpand(nil, msg.Dest) - jw.MustLog(err) - expanded = jw.dropNonComparableTags(expanded) + expanded := jw.dropNonComparableTags(jw.MustTagExpand(msg.Dest)) switch len(expanded) { case 0: // no tags, so no requests will match @@ -99,10 +97,8 @@ func (jw *Jaws) dropNonComparableTags(tags []any) []any { // setDirty marks all Elements that have one or more of the given tags as dirty. func (jw *Jaws) setDirty(tags []any) { jw.mu.Lock() - // Release the lock with defer so it is freed even if a map insert panics: a tag - // that passed the static comparability check in ensureUsableTag can still be - // non-comparable at runtime (a comparable struct holding e.g. a func in an - // interface field) and panic when used as a map key here. + // Public paths validate keys through TagExpand; the deferred unlock is + // defense-in-depth if an internal caller violates that invariant. defer jw.mu.Unlock() for _, tagValue := range tags { jw.dirtOrder++ @@ -112,23 +108,12 @@ func (jw *Jaws) setDirty(tags []any) { // Dirty marks all [Element] values that have one or more of the given tags as dirty. // -// If any tag implements [tag.TagGetter] it is called with a nil [Request]; prefer -// [Request.Dirty], which avoids this. A tag that is not hashable panics the calling -// goroutine, but the panic is contained there and the [Jaws.Serve] loop is -// unaffected. [Request.Dirty] behaves the same here. +// The tags are expanded through [Jaws.MustTagExpand]: with a [Jaws.Logger] configured +// an expansion error is logged and the partial result is still applied, while without +// one the call panics before anything is marked dirty. [Request.Dirty] is equivalent; +// both mark matching Elements on every live [Request]. func (jw *Jaws) Dirty(dirtyTags ...any) { - // A non-hashable tag panics here in the caller's goroutine rather than being - // logged-and-dropped the way Broadcast handles a bad wire.Message.Dest: Broadcast - // hashes the destination in the Serve goroutine, where a panic would crash the - // process, whereas this hashing happens in setDirty under the caller's goroutine - // with the lock released on panic via defer, so Serve is unaffected. - // - // Use TagExpand+MustLog rather than MustTagExpand: with a nil Context the latter - // panics on an illegal tag even in production, unlike Request.Dirty and Broadcast. - // Log and continue with the partial result. - expanded, err := tag.TagExpand(nil, dirtyTags) - jw.MustLog(err) - jw.setDirty(expanded) + jw.setDirty(jw.MustTagExpand(dirtyTags)) } // dirtPair pairs a dirty tag with its insertion-order rank, used by sortedDirtTags diff --git a/contracts.go b/contracts.go index e7857124..7641ccd1 100644 --- a/contracts.go +++ b/contracts.go @@ -23,15 +23,6 @@ type Container interface { JawsContains(elem *Element) (contents []UI) } -// InitHandler allows initializing UI getters and setters before their use. -// -// You can of course initialize them in the call from the template engine, -// but at that point you don't have access to the [Element], [Request.Context] -// or [Request.Session]. -type InitHandler interface { - JawsInit(elem *Element) (err error) -} - // Logger is satisfied by a [*log/slog.Logger] via its Info, Warn and Error methods. type Logger interface { Info(msg string, args ...any) diff --git a/element.go b/element.go index 93ee0014..7689da06 100644 --- a/element.go +++ b/element.go @@ -110,6 +110,9 @@ func (elem *Element) Tag(tags ...any) { } // HasTag returns true if this Element has the given tag. +// +// It reports false for a deleted Element. tagValue is not expanded; see +// [Request.HasTag]. func (elem *Element) HasTag(tagValue any) bool { return !elem.deleted.Load() && elem.Request.HasTag(elem, tagValue) } @@ -475,34 +478,34 @@ func (elem *Element) ApplyParams(params []any) (attrs []template.HTMLAttr) { // ApplyGetter examines getter and resolves its tag candidate. // -// If getter implements [tag.TagGetter], the candidate is its returned value; -// otherwise the candidate is getter itself. TagGetter values, supported tag -// slices and runtime-comparable candidates are passed to [Element.Tag] for normal -// validation. Other non-comparable candidates are not automatically tagged, -// matching [ParseParams]. +// If getter implements [tag.TagGetter], the candidate is the value returned by +// [tag.TagGetter.JawsGetTag]; otherwise the candidate is getter itself. Eligible +// candidates — TagGetter values, supported tag slices and runtime-comparable +// values — are passed to [Element.Tag] for normal expansion and validation. +// That expansion may invoke JawsGetTag again when the candidate is itself a +// TagGetter or contains one. Other non-comparable candidates are not automatically +// tagged, matching [ParseParams]. // // If getter is an [InputHandler], [ClickHandler], [ContextMenuHandler] or // [InitialHTMLAttrHandler], relevant values are added to the [Element]. // -// Finally, if getter is an [InitHandler], its JawsInit -// function is called. -// -// Returns the tag that was added (nil if none was added, whether because getter -// was nil or its candidate was not usable as a tag), any initial HTML attrs -// provided by InitialHTMLAttrHandler, and any error returned from JawsInit() if it -// was called. +// The returned value is the candidate that was passed for tagging, not confirmation +// that every expanded key was registered; it is nil if getter was nil or its candidate +// was not usable as a tag. Callers may retain it for later dirtying; what makes that +// safe is [tag.TagGetter]'s idempotent tag identity. attrs holds any initial HTML +// attributes provided by InitialHTMLAttrHandler. // // If the [Element] is already frozen and getter is an event handler, the handler // is not added: in production with a [Jaws.Logger] configured this is logged and -// tag and init processing still occur, while debug builds and servers without a -// Logger panic via reportMisuse, aborting before tag and init processing. A -// non-event-handler getter never calls reportMisuse, so its tag and init -// processing always occur. -func (elem *Element) ApplyGetter(getter any) (tagValue any, attrs []template.HTMLAttr, err error) { +// initial-attribute and tag processing still occur, while debug builds and servers +// without a Logger panic via reportMisuse, aborting before them. A non-event-handler +// getter never calls reportMisuse, so its initial-attribute and tag processing always +// occur. +func (elem *Element) ApplyGetter(getter any) (tagValue any, attrs []template.HTMLAttr) { if getter != nil { tagValue = getter if tagger, ok := getter.(tag.TagGetter); ok { - tagValue = tagger.JawsGetTag(elem.Request) + tagValue = tagger.JawsGetTag() } if _, ok := getter.(InputHandler); ok { elem.appendHandlers(getter) @@ -521,9 +524,6 @@ func (elem *Element) ApplyGetter(getter any) (tagValue any, attrs []template.HTM } else { tagValue = nil } - if initer, ok := getter.(InitHandler); ok { - err = initer.JawsInit(elem) - } } return } diff --git a/element_test.go b/element_test.go index 6628f00e..b8627800 100644 --- a/element_test.go +++ b/element_test.go @@ -28,23 +28,12 @@ type testUi struct { updateCalled int32 getCalled int32 setCalled int32 - initCalled int32 - initError error s string renderFn func(elem *Element, w io.Writer, params []any) error updateFn func(elem *Element) } -// JawsInit implements InitHandler. -func (tss *testUi) JawsInit(elem *Element) (err error) { - atomic.AddInt32(&tss.initCalled, 1) - return tss.initError -} - -var ( - _ UI = (*testUi)(nil) - _ InitHandler = (*testUi)(nil) -) +var _ UI = (*testUi)(nil) func (tss *testUi) JawsGet(elem *Element) string { atomic.AddInt32(&tss.getCalled, 1) @@ -73,11 +62,9 @@ func (tss *testUi) JawsUpdate(elem *Element) { } } -type testApplyGetterAll struct { - initErr error -} +type testApplyGetterAll struct{} -func (a testApplyGetterAll) JawsGetTag(tag.Context) any { return tag.Tag("tg") } +func (a testApplyGetterAll) JawsGetTag() any { return tag.Tag("tg") } func (a testApplyGetterAll) JawsClick(elem *Element, click Click) error { return ErrEventUnhandled } @@ -90,13 +77,9 @@ func (a testApplyGetterAll) JawsSet(elem *Element, value string) error { return ErrEventUnhandled } -func (a testApplyGetterAll) JawsInit(elem *Element) error { - return a.initErr -} - type testNilTagGetter struct{} -func (testNilTagGetter) JawsGetTag(tag.Context) any { return nil } +func (testNilTagGetter) JawsGetTag() any { return nil } type testReentrantDebugTag struct { rq *Request @@ -620,7 +603,7 @@ func TestElement_HandlersFrozenAfterRender(t *testing.T) { } assertHandlerMutationFrozen(t, e, func() { e.AddHandlers(testClickHandler{}) }) assertHandlerMutationFrozen(t, e, func() { e.ApplyParams([]any{testEventHandler{}}) }) - assertHandlerMutationFrozen(t, e, func() { _, _, _ = e.ApplyGetter(testClickHandler{}) }) + assertHandlerMutationFrozen(t, e, func() { _, _ = e.ApplyGetter(testClickHandler{}) }) } func TestElement_FreezeSealsHandlers(t *testing.T) { @@ -661,9 +644,7 @@ func TestElement_UnrenderedAcceptsHandlers(t *testing.T) { click := &testClickCounter{wantName: "name"} e.AddHandlers(testEventHandler{}) e.ApplyParams([]any{testContextMenuHandler{}}) - if _, _, err := e.ApplyGetter(click); err != nil { - t.Fatal(err) - } + e.ApplyGetter(click) if got := len(e.handlers); got != 3 { t.Fatalf("expected 3 handlers, got %d", got) } @@ -811,25 +792,18 @@ func TestElement_ApplyGetterDebugBranches(t *testing.T) { defer rq.Close() elem := rq.NewElement(&testUi{}) - if gotTag, attrs, err := elem.ApplyGetter(nil); gotTag != nil || err != nil || len(attrs) != 0 { - t.Fatalf("unexpected %v %v %#v", gotTag, err, attrs) + if gotTag, attrs := elem.ApplyGetter(nil); gotTag != nil || len(attrs) != 0 { + t.Fatalf("unexpected %v %#v", gotTag, attrs) } ag := testApplyGetterAll{} - gotTags, attrs, err := elem.ApplyGetter(ag) - if err != nil { - t.Fatalf("unexpected error %v", err) - } + gotTags, attrs := elem.ApplyGetter(ag) if len(attrs) != 0 { t.Fatalf("expected no attrs, got %#v", attrs) } if !elem.HasTag(tag.Tag("tg")) { t.Fatalf("missing Tag('tg') in %#v", gotTags) } - agErr := testApplyGetterAll{initErr: tag.ErrNotComparable} - if _, _, err := elem.ApplyGetter(agErr); err != tag.ErrNotComparable { - t.Fatalf("expected init err, got %v", err) - } } type testClickHandler struct{} @@ -955,16 +929,13 @@ func TestElement_ApplyGetter(t *testing.T) { e := rq.NewElement(tss) var tch testClickHandler - gotTag, attrs, err := e.ApplyGetter(tch) + gotTag, attrs := e.ApplyGetter(tch) if gotTag != tch { t.Errorf("tag was %#v", gotTag) } if len(attrs) != 0 { t.Fatalf("expected no attrs, got %#v", attrs) } - if err != nil { - t.Error(err) - } is.Equal(len(e.handlers), 1) if !e.HasTag(tch) { t.Fatal("expected comparable click handler to be tagged") @@ -977,9 +948,7 @@ func TestElement_ApplyGetter_NonComparableHandler(t *testing.T) { e := rq.NewElement(&testUi{s: "foo"}) tch := testNonComparableClickHandler{names: []string{"name"}} - if _, _, err := e.ApplyGetter(tch); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + e.ApplyGetter(tch) if len(e.handlers) != 1 { t.Fatalf("expected 1 handler, got %d", len(e.handlers)) } @@ -1004,10 +973,7 @@ func TestElement_ApplyGetter_NonComparableHandler_NilLogger(t *testing.T) { rq := jw.NewRequest(httptest.NewRequest(http.MethodGet, "/", nil)) e := rq.NewElement(&testUi{s: "x"}) tch := testNonComparableClickHandler{names: []string{"name"}} - gotTag, _, err := e.ApplyGetter(tch) - if err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + gotTag, _ := e.ApplyGetter(tch) if gotTag != nil { t.Fatalf("expected declined non-comparable candidate to return a nil tag, got %#v", gotTag) } @@ -1034,9 +1000,7 @@ func TestElement_ApplyGetter_NonComparableHandler_NoLog(t *testing.T) { rq := jw.NewRequest(httptest.NewRequest(http.MethodGet, "/", nil)) e := rq.NewElement(&testUi{s: "x"}) tch := testNonComparableClickHandler{names: []string{"name"}} - if _, _, err := e.ApplyGetter(tch); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + e.ApplyGetter(tch) if strings.Contains(buf.String(), "not usable as tag") { t.Fatalf("expected no not-usable-as-tag log, got %q", buf.String()) } @@ -1047,16 +1011,13 @@ func TestElement_ApplyGetter_NilTagGetter(t *testing.T) { defer rq.Close() e := rq.NewElement(&testUi{s: "foo"}) - gotTag, attrs, err := e.ApplyGetter(testNilTagGetter{}) + gotTag, attrs := e.ApplyGetter(testNilTagGetter{}) if gotTag != nil { t.Fatalf("expected nil tag, got %#v", gotTag) } if len(attrs) != 0 { t.Fatalf("expected no attrs, got %#v", attrs) } - if err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } if got := rq.TagsOf(e); len(got) != 0 { t.Fatalf("expected nil tag getter to not tag element, got %v", got) } @@ -1192,10 +1153,7 @@ func TestElement_ApplyGetter_InitialHTMLAttrAndClickHandler(t *testing.T) { called: &called, attr: `data-attr="ok"`, } - _, attrs, err := e.ApplyGetter(h) - if err != nil { - t.Fatal(err) - } + _, attrs := e.ApplyGetter(h) if len(attrs) != 1 || attrs[0] != `data-attr="ok"` { t.Fatalf("unexpected attrs from ApplyGetter: %#v", attrs) } @@ -1223,9 +1181,7 @@ func TestElement_ApplyGetter_InputHandlerAutoTag(t *testing.T) { e := rq.NewElement(testDivWidget{inner: "x"}) h := testEventHandler{} - if _, _, err := e.ApplyGetter(h); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + e.ApplyGetter(h) if len(e.handlers) != 1 { t.Fatalf("expected 1 handler, got %d", len(e.handlers)) } @@ -1243,9 +1199,7 @@ func TestElement_ApplyGetter_ContextMenuHandlerAutoTag(t *testing.T) { e := rq.NewElement(testDivWidget{inner: "x"}) h := testContextMenuHandler{} - if _, _, err := e.ApplyGetter(h); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + e.ApplyGetter(h) if len(e.handlers) != 1 { t.Fatalf("expected 1 handler, got %d", len(e.handlers)) } @@ -1263,9 +1217,7 @@ func TestElement_ApplyGetter_InputHandlerNonComparableNoAutoTag(t *testing.T) { e := rq.NewElement(testDivWidget{inner: "x"}) h := testNonComparableEventHandler{names: []string{"name"}} - if _, _, err := e.ApplyGetter(h); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + e.ApplyGetter(h) if len(e.handlers) != 1 { t.Fatalf("expected 1 handler, got %d", len(e.handlers)) } @@ -1283,9 +1235,7 @@ func TestElement_ApplyGetter_ContextMenuHandlerNonComparableNoAutoTag(t *testing e := rq.NewElement(testDivWidget{inner: "x"}) h := testNonComparableContextMenuHandler{names: []string{"name"}} - if _, _, err := e.ApplyGetter(h); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + e.ApplyGetter(h) if len(e.handlers) != 1 { t.Fatalf("expected 1 handler, got %d", len(e.handlers)) } @@ -1297,25 +1247,6 @@ func TestElement_ApplyGetter_ContextMenuHandlerNonComparableNoAutoTag(t *testing } } -func TestElement_JawsInit(t *testing.T) { - is := newTestHelper(t) - rq := newTestRequest(t) - defer rq.Close() - - tss := &testUi{s: "foo"} - tss.initError = tag.ErrNotComparable - e := rq.NewElement(tss) - - gotTag, _, err := e.ApplyGetter(tss) - is.Equal(atomic.LoadInt32(&tss.initCalled), int32(1)) - if gotTag != tss { - t.Errorf("tag was %#v", gotTag) - } - if err != tag.ErrNotComparable { - t.Error(err) - } -} - // nonReflexiveUI is a comparable UI value that is not equal to itself when it holds // NaN; it backs TestNewErrUnusableUI. type nonReflexiveUI struct{ f float64 } diff --git a/eventhandler_test.go b/eventhandler_test.go index 922b62a7..02e77007 100644 --- a/eventhandler_test.go +++ b/eventhandler_test.go @@ -37,16 +37,14 @@ func (t *testJawsEvent) JawsInput(elem *Element, value string) (err error) { return } -func (t *testJawsEvent) JawsGetTag(tag.Context) (tagValue any) { +func (t *testJawsEvent) JawsGetTag() (tagValue any) { return t.tagValue } func (t *testJawsEvent) JawsRender(elem *Element, w io.Writer, params []any) (err error) { - var tagValue any - if tagValue, _, err = elem.ApplyGetter(t); err == nil { - _, _ = fmt.Fprint(w, params) - t.msgCh <- fmt.Sprintf("JawsRender(%d)%#v", elem.jid, tagValue) - } + tagValue, _ := elem.ApplyGetter(t) + _, _ = fmt.Fprint(w, params) + t.msgCh <- fmt.Sprintf("JawsRender(%d)%#v", elem.jid, tagValue) return } @@ -716,9 +714,7 @@ func Test_CallEventHandlers_ClickOnlyHandlerViaApplyGetter(t *testing.T) { elem := rq.NewElement(testDivWidget{inner: "x"}) clickCounter := &testClickCounter{wantName: "name"} - if _, _, err := elem.ApplyGetter(clickCounter); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + elem.ApplyGetter(clickCounter) err := CallEventHandlers(elem.UI(), elem, what.Click, "1 2 5 name") if err != nil { @@ -766,9 +762,7 @@ func Test_CallEventHandlers_ContextMenuOnlyHandlerViaApplyGetter(t *testing.T) { elem := rq.NewElement(testDivWidget{inner: "x"}) counter := &testContextMenuCounter{wantName: "name"} - if _, _, err := elem.ApplyGetter(counter); err != nil { - t.Fatalf("ApplyGetter returned error: %v", err) - } + elem.ApplyGetter(counter) err := CallEventHandlers(elem.UI(), elem, what.ContextMenu, "10 20 5 name") if err != nil { diff --git a/examples/minesweeper/main.go b/examples/minesweeper/main.go index 9f5b040d..f60ea4ac 100644 --- a/examples/minesweeper/main.go +++ b/examples/minesweeper/main.go @@ -14,7 +14,6 @@ import ( "github.com/linkdata/jaws" "github.com/linkdata/jaws/lib/bind" - "github.com/linkdata/jaws/lib/tag" "github.com/linkdata/jaws/lib/ui" ) @@ -138,12 +137,13 @@ func (c *Cell) syncPresentation(elem *jaws.Element, view cellView) { // JawsGetTag returns the cell's per-cell dirty identity. // // It deliberately returns ONLY the cell, not the shared board tag. Because *Cell -// is a [tag.TagGetter], returning the board tag here would make Request.Dirty(c) -// tag-expand to include it, so dirtying one cell would re-render every cell. The -// shared board tag is registered separately via [Cell.BoardTag] (passed to the -// cell's Button), so the element listens to both while a single-cell dirty stays -// scoped to just that cell. Board-wide refreshes dirty &g.cells directly. -func (c *Cell) JawsGetTag(_ tag.Context) any { +// is a [github.com/linkdata/jaws/lib/tag.TagGetter], returning the board tag here +// would make Request.Dirty(c) tag-expand to include it, so dirtying one cell would +// re-render every cell. The shared board tag is registered separately via +// [Cell.BoardTag] (passed to the cell's Button), so the element listens to both while +// a single-cell dirty stays scoped to just that cell. Board-wide refreshes dirty +// &g.cells directly. +func (c *Cell) JawsGetTag() any { return c } @@ -508,8 +508,8 @@ func run(listenAndServe func(addr string, handler http.Handler) error) error { // One board is shared by every visitor: this is a single, collaborative // game that all connected browsers see and play simultaneously. That is a // deliberate choice for this demo. For per-user state instead, create the - // game inside the handler (or in a JawsInit) keyed off the request/session, - // e.g. via jw.Session, rather than binding one board for the whole server. + // game inside the handler keyed off the request/session, e.g. via + // jw.Session, rather than binding one board for the whole server. board := newGame(10, 10, 15) mux := http.NewServeMux() diff --git a/examples/minesweeper/main_benchmark_test.go b/examples/minesweeper/main_benchmark_test.go index 59c5efe2..a42f232d 100644 --- a/examples/minesweeper/main_benchmark_test.go +++ b/examples/minesweeper/main_benchmark_test.go @@ -55,7 +55,7 @@ func BenchmarkSingleCellDirtyFanout(b *testing.B) { // resolve each to its registered elements (Request.GetElements is the same // tagMap lookup makeUpdateList performs). The sum is the number of element // re-renders the toggle would drive. - expanded, err := jawstag.TagExpand(nil, g.toggleFlag(cell)) + expanded, err := jawstag.TagExpand(g.toggleFlag(cell)) if err != nil { b.Fatal(err) } diff --git a/examples/minesweeper/main_test.go b/examples/minesweeper/main_test.go index e04e7b9d..511497cf 100644 --- a/examples/minesweeper/main_test.go +++ b/examples/minesweeper/main_test.go @@ -356,7 +356,7 @@ func TestGameStatusAndStatsHelpers(t *testing.T) { if got := statusGetter.JawsGet(nil); got != statusTests[0].want { t.Fatalf("StatusSpan getter = %q, want %q", got, statusTests[0].want) } - statusTags, err := jawstag.TagExpand(nil, statusTagger.JawsGetTag(nil)) + statusTags, err := jawstag.TagExpand(statusTagger.JawsGetTag()) if err != nil { t.Fatal(err) } @@ -380,7 +380,7 @@ func TestGameStatusAndStatsHelpers(t *testing.T) { if got := statsGetter.JawsGet(nil); got != wantStats { t.Fatalf("StatsSpan getter = %q, want %q", got, wantStats) } - statsTags, err := jawstag.TagExpand(nil, statsTagger.JawsGetTag(nil)) + statsTags, err := jawstag.TagExpand(statsTagger.JawsGetTag()) if err != nil { t.Fatal(err) } @@ -576,7 +576,7 @@ func TestSingleCellDirtyStaysScopedToOneCell(t *testing.T) { g := newGame(3, 3, 1) cell := g.cells[0][0] - flagTags, err := jawstag.TagExpand(nil, g.toggleFlag(cell)) + flagTags, err := jawstag.TagExpand(g.toggleFlag(cell)) if err != nil { t.Fatal(err) } @@ -589,7 +589,7 @@ func TestSingleCellDirtyStaysScopedToOneCell(t *testing.T) { // reports changed scalars and appends &g.cells. g2 := newGame(3, 3, 1) _ = g2.clickCell(g2.cells[0][0]) - resetTags, err := jawstag.TagExpand(nil, g2.reset()) + resetTags, err := jawstag.TagExpand(g2.reset()) if err != nil { t.Fatal(err) } diff --git a/jaws_test.go b/jaws_test.go index 70f7e9d4..c96c1f39 100644 --- a/jaws_test.go +++ b/jaws_test.go @@ -38,7 +38,7 @@ import ( type testBroadcastTagGetter struct{} -func (testBroadcastTagGetter) JawsGetTag(tag.Context) any { +func (testBroadcastTagGetter) JawsGetTag() any { return tag.Tag("expanded") } diff --git a/lib/bind/bind_test.go b/lib/bind/bind_test.go index d973f732..2f4cb340 100644 --- a/lib/bind/bind_test.go +++ b/lib/bind/bind_test.go @@ -12,7 +12,6 @@ import ( "github.com/linkdata/deadlock" "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) // TestBinder_ConcurrentAccess backs the concurrency-safety documented on @@ -190,7 +189,7 @@ func testBind_Hook_Success[T comparable](t *testing.T, testval T) { if calls1 != 1 { t.Error(calls1) } - tags1 := tag.MustTagExpand(nil, bind1) + tags1 := mustExpand(t, bind1) if !reflect.DeepEqual(tags1, []any{&val}) { t.Error(tags1) } @@ -215,7 +214,7 @@ func testBind_Hook_Success[T comparable](t *testing.T, testval T) { if calls2 != 1 { t.Error(calls2) } - tags2 := tag.MustTagExpand(nil, bind2) + tags2 := mustExpand(t, bind2) if !reflect.DeepEqual(tags2, []any{&val}) { t.Error(tags2) } @@ -292,7 +291,7 @@ func testBind_Hook_Set[T comparable](t *testing.T, testval T) { if calls1 != 2 { t.Error(calls1) } - tags1 := tag.MustTagExpand(nil, bind1) + tags1 := mustExpand(t, bind1) if !reflect.DeepEqual(tags1, []any{&val}) { t.Error(tags1) } @@ -310,7 +309,7 @@ func testBind_Hook_Set[T comparable](t *testing.T, testval T) { if calls2 != 0 { t.Error(calls2) } - tags2 := tag.MustTagExpand(nil, bind2) + tags2 := mustExpand(t, bind2) if !reflect.DeepEqual(tags2, []any{&val}) { t.Error(tags2) } @@ -338,7 +337,7 @@ func testBind_Hook_Get[T comparable](t *testing.T, testval T) { if calls1 != 1 { t.Error(calls1) } - tags1 := tag.MustTagExpand(nil, bind1) + tags1 := mustExpand(t, bind1) if !reflect.DeepEqual(tags1, []any{&val}) { t.Error(tags1) } @@ -359,7 +358,7 @@ func testBind_Hook_Get[T comparable](t *testing.T, testval T) { if calls2 != 0 { t.Error(calls2) } - tags2 := tag.MustTagExpand(nil, bind2) + tags2 := mustExpand(t, bind2) if !reflect.DeepEqual(tags2, []any{&val}) { t.Error(tags2) } @@ -460,7 +459,7 @@ func TestBind_Hook_Clicked_binding(t *testing.T) { if gotClick.Name != "save" || gotClick.X != 1 || gotClick.Y != 2 { t.Error(gotClick) } - tags := tag.MustTagExpand(nil, bind) + tags := mustExpand(t, bind) if !reflect.DeepEqual(tags, []any{&val}) { t.Error(tags) } @@ -527,7 +526,7 @@ func TestBind_Hook_Clicked_bindingHook(t *testing.T) { if err := bindWithSuccess.(jaws.ClickHandler).JawsClick(nil, jaws.Click{Name: "x"}); !errors.Is(err, jaws.ErrEventUnhandled) { t.Fatal(err) } - tags := tag.MustTagExpand(nil, clickBind2) + tags := mustExpand(t, clickBind2) if !reflect.DeepEqual(tags, []any{&val}) { t.Error(tags) } @@ -833,7 +832,7 @@ func TestBind_GetHTML_Default(t *testing.T) { if got, want := MakeHTMLGetter(bind1).JawsGetHTML(nil), template.HTML("12"); got != want { t.Fatalf("want %q got %q", want, got) } - if tags := tag.MustTagExpand(nil, bind1); !reflect.DeepEqual(tags, []any{&val1}) { + if tags := mustExpand(t, bind1); !reflect.DeepEqual(tags, []any{&val1}) { t.Fatal(tags) } @@ -843,7 +842,7 @@ func TestBind_GetHTML_Default(t *testing.T) { if got, want := MakeHTMLGetter(bind2).JawsGetHTML(nil), template.HTML("<span>"); got != want { t.Fatalf("want %q got %q", want, got) } - if tags := tag.MustTagExpand(nil, bind2); !reflect.DeepEqual(tags, []any{&val2}) { + if tags := mustExpand(t, bind2); !reflect.DeepEqual(tags, []any{&val2}) { t.Fatal(tags) } bind3 := bind2.Success(func() {}) @@ -860,7 +859,7 @@ func TestBind_Hook_Format_escapedSprintf(t *testing.T) { if got, want := MakeHTMLGetter(bind).JawsGetHTML(nil), template.HTML("v=<span>"); got != want { t.Fatalf("want %q got %q", want, got) } - if tags := tag.MustTagExpand(nil, bind); !reflect.DeepEqual(tags, []any{&val}) { + if tags := mustExpand(t, bind); !reflect.DeepEqual(tags, []any{&val}) { t.Fatal(tags) } @@ -878,7 +877,7 @@ func TestBind_Hook_Format_usesFormatter(t *testing.T) { if got, want := MakeHTMLGetter(bind).JawsGetHTML(nil), template.HTML("<fmt!:<&>>"); got != want { t.Fatalf("want %q got %q", want, got) } - if tags := tag.MustTagExpand(nil, bind); !reflect.DeepEqual(tags, []any{&val}) { + if tags := mustExpand(t, bind); !reflect.DeepEqual(tags, []any{&val}) { t.Fatal(tags) } } @@ -891,7 +890,7 @@ func TestBind_Hook_Format_timeTimeUsesFormatter(t *testing.T) { if got, want := MakeHTMLGetter(bind).JawsGetHTML(nil), template.HTML("<2026-04-20>"); got != want { t.Fatalf("want %q got %q", want, got) } - if tags := tag.MustTagExpand(nil, bind); !reflect.DeepEqual(tags, []any{&val}) { + if tags := mustExpand(t, bind); !reflect.DeepEqual(tags, []any{&val}) { t.Fatal(tags) } } @@ -978,7 +977,7 @@ func TestBind_Hook_GetHTML(t *testing.T) { if !getHTML2CurrentOK { t.Fatal("GetHTML second hook current binder mismatch") } - if tags := tag.MustTagExpand(nil, getHTML2); !reflect.DeepEqual(tags, []any{&val}) { + if tags := mustExpand(t, getHTML2); !reflect.DeepEqual(tags, []any{&val}) { t.Fatal(tags) } } @@ -1018,7 +1017,7 @@ func TestBind_Hook_InitialHTMLAttr(t *testing.T) { if !secondPrevOK { t.Fatal("InitialHTMLAttr second hook previous binder mismatch") } - if tags := tag.MustTagExpand(nil, second); !reflect.DeepEqual(tags, []any{&val}) { + if tags := mustExpand(t, second); !reflect.DeepEqual(tags, []any{&val}) { t.Fatal(tags) } } diff --git a/lib/bind/binder.go b/lib/bind/binder.go index b722eb11..89ad3ee3 100644 --- a/lib/bind/binder.go +++ b/lib/bind/binder.go @@ -301,7 +301,7 @@ func (b *binder[T]) JawsSet(elem *jaws.Element, value T) (err error) { return } -func (b *binder[T]) JawsGetTag(tag.Context) any { +func (b *binder[T]) JawsGetTag() any { return b.ptr } diff --git a/lib/bind/getter.go b/lib/bind/getter.go index b6219642..e1549ba3 100644 --- a/lib/bind/getter.go +++ b/lib/bind/getter.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) // ErrValueNotSettable is returned by read-only adapters when [Setter.JawsSet] @@ -29,7 +28,7 @@ func (s getterStatic[T]) JawsGet(elem *jaws.Element) T { return s.v } -func (s getterStatic[T]) JawsGetTag(tag.Context) any { +func (s getterStatic[T]) JawsGetTag() any { return nil } diff --git a/lib/bind/getter_test.go b/lib/bind/getter_test.go index 646e3a86..9e4918aa 100644 --- a/lib/bind/getter_test.go +++ b/lib/bind/getter_test.go @@ -32,7 +32,7 @@ func TestMakeGetter_GetterPassThroughAndTag(t *testing.T) { if got := g.JawsGet(nil); got != "x" { t.Fatalf("unexpected getter value %q", got) } - if gotTag := g.(tag.TagGetter).JawsGetTag(nil); gotTag != nil { + if gotTag := g.(tag.TagGetter).JawsGetTag(); gotTag != nil { t.Fatalf("expected nil tag, got %#v", gotTag) } diff --git a/lib/bind/htmlgetterfunc.go b/lib/bind/htmlgetterfunc.go index 38ddef05..4fe7d302 100644 --- a/lib/bind/htmlgetterfunc.go +++ b/lib/bind/htmlgetterfunc.go @@ -2,6 +2,7 @@ package bind import ( "html/template" + "slices" "github.com/linkdata/jaws" "github.com/linkdata/jaws/lib/tag" @@ -18,13 +19,17 @@ func (g *htmlGetterFunc) JawsGetHTML(elem *jaws.Element) template.HTML { return g.fn(elem) } -func (g *htmlGetterFunc) JawsGetTag(tag.Context) any { +func (g *htmlGetterFunc) JawsGetTag() any { return g.tags } // HTMLGetterFunc wraps fn as an [HTMLGetter]. // // Optional tags are exposed through [tag.TagGetter]. +// +// The top-level slots of tags are copied, so the caller may reuse or modify the slice +// it passed. Nested containers and reference-backed tag values are not copied; keeping +// those stable remains the caller's obligation. func HTMLGetterFunc(fn func(elem *jaws.Element) (tmpl template.HTML), tags ...any) HTMLGetter { - return &htmlGetterFunc{fn: fn, tags: tags} + return &htmlGetterFunc{fn: fn, tags: slices.Clone(tags)} } diff --git a/lib/bind/htmlgetterfunc_test.go b/lib/bind/htmlgetterfunc_test.go index aa85f98f..e8d7ba0d 100644 --- a/lib/bind/htmlgetterfunc_test.go +++ b/lib/bind/htmlgetterfunc_test.go @@ -17,7 +17,21 @@ func TestHTMLGetterFunc(t *testing.T) { if s := hg.JawsGetHTML(nil); s != "foo" { t.Error(s) } - if got := tag.MustTagExpand(nil, hg); !reflect.DeepEqual(got, []any{tt}) { + if got := mustExpand(t, hg); !reflect.DeepEqual(got, []any{tt}) { t.Error(got) } } + +// TestHTMLGetterFunc_SnapshotsTagSlots is the regression for issue #217: the +// constructor took the caller's variadic slice header, so reusing that slice after +// construction retagged a live getter. Only the top-level slots are copied, which is +// what this asserts. +func TestHTMLGetterFunc_SnapshotsTagSlots(t *testing.T) { + tags := []any{tag.Tag("original")} + hg := HTMLGetterFunc(func(*jaws.Element) template.HTML { return "x" }, tags...) + tags[0] = tag.Tag("reused") + + if got := mustExpand(t, hg); !reflect.DeepEqual(got, []any{tag.Tag("original")}) { + t.Fatalf("getter tags = %#v, want the construction-time snapshot", got) + } +} diff --git a/lib/bind/makehtmlgetter.go b/lib/bind/makehtmlgetter.go index 23c7126c..8d8d3ea9 100644 --- a/lib/bind/makehtmlgetter.go +++ b/lib/bind/makehtmlgetter.go @@ -6,7 +6,6 @@ import ( "html/template" "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) type htmlGetter struct{ v template.HTML } @@ -15,7 +14,7 @@ func (g htmlGetter) JawsGetHTML(elem *jaws.Element) template.HTML { return g.v } -func (g htmlGetter) JawsGetTag(tag.Context) any { +func (g htmlGetter) JawsGetTag() any { return nil } @@ -25,7 +24,7 @@ func (g htmlStringerGetter) JawsGetHTML(elem *jaws.Element) template.HTML { return template.HTML(html.EscapeString(g.sg.String())) // #nosec G203 } -func (g htmlStringerGetter) JawsGetTag(tag.Context) any { +func (g htmlStringerGetter) JawsGetTag() any { return g.sg } @@ -41,7 +40,7 @@ func (g htmlGetterString) JawsGetHTML(elem *jaws.Element) template.HTML { return template.HTML(html.EscapeString(g.sg.JawsGet(elem))) // #nosec G203 } -func (g htmlGetterString) JawsGetTag(tag.Context) any { +func (g htmlGetterString) JawsGetTag() any { return g.sg } diff --git a/lib/bind/makehtmlgetter_test.go b/lib/bind/makehtmlgetter_test.go index 6302879f..7218d831 100644 --- a/lib/bind/makehtmlgetter_test.go +++ b/lib/bind/makehtmlgetter_test.go @@ -107,7 +107,7 @@ func Test_MakeHTMLGetter(t *testing.T) { if txt := got.JawsGetHTML(nil); txt != tt.out { t.Errorf("MakeHTMLGetter(%s).JawsGetHTML() = %v, want %v", tt.name, txt, tt.out) } - if gotTag := got.(tag.TagGetter).JawsGetTag(nil); gotTag != tt.wantTag { + if gotTag := got.(tag.TagGetter).JawsGetTag(); gotTag != tt.wantTag { t.Errorf("MakeHTMLGetter(%s).JawsGetTag() = %v, want %v", tt.name, gotTag, tt.wantTag) } }) diff --git a/lib/bind/mustexpand_test.go b/lib/bind/mustexpand_test.go new file mode 100644 index 00000000..5ec3716b --- /dev/null +++ b/lib/bind/mustexpand_test.go @@ -0,0 +1,20 @@ +package bind + +import ( + "testing" + + "github.com/linkdata/jaws/lib/tag" +) + +// mustExpand expands v and fails the test on an expansion error. +// +// It replaces constructing a *jaws.Jaws purely to reach Jaws.MustTagExpand: these +// tests only need the expanded keys, not error logging. +func mustExpand(t *testing.T, v any) []any { + t.Helper() + expanded, err := tag.TagExpand(v) + if err != nil { + t.Fatalf("TagExpand(%#v) error = %v", v, err) + } + return expanded +} diff --git a/lib/bind/setter.go b/lib/bind/setter.go index b8f92522..50e98829 100644 --- a/lib/bind/setter.go +++ b/lib/bind/setter.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) // Setter exposes and updates a value for a [jaws.Element]. @@ -22,7 +21,7 @@ func (setterReadOnly[T]) JawsSet(elem *jaws.Element, value T) error { return ErrValueNotSettable } -func (s setterReadOnly[T]) JawsGetTag(tag.Context) any { +func (s setterReadOnly[T]) JawsGetTag() any { return s.Getter } @@ -38,7 +37,7 @@ func (s setterStatic[T]) JawsGet(elem *jaws.Element) T { return s.v } -func (s setterStatic[T]) JawsGetTag(tag.Context) any { +func (s setterStatic[T]) JawsGetTag() any { return nil } diff --git a/lib/bind/setter_test.go b/lib/bind/setter_test.go index 94dc540f..2cba7002 100644 --- a/lib/bind/setter_test.go +++ b/lib/bind/setter_test.go @@ -26,7 +26,7 @@ func Test_makeSetter(t *testing.T) { if s := setter1.JawsGet(nil); s != testStringGetterText { t.Error(s) } - if gotTag := setter1.(tag.TagGetter).JawsGetTag(nil); gotTag != tsg { + if gotTag := setter1.(tag.TagGetter).JawsGetTag(); gotTag != tsg { t.Error(gotTag) } @@ -37,7 +37,7 @@ func Test_makeSetter(t *testing.T) { if s := setter2.JawsGet(nil); s != "quux" { t.Error(s) } - if gotTag := setter2.(tag.TagGetter).JawsGetTag(nil); gotTag != nil { + if gotTag := setter2.(tag.TagGetter).JawsGetTag(); gotTag != nil { t.Error(gotTag) } } diff --git a/lib/bind/setterfloat64.go b/lib/bind/setterfloat64.go index 525caeb7..fd9a2e53 100644 --- a/lib/bind/setterfloat64.go +++ b/lib/bind/setterfloat64.go @@ -6,7 +6,6 @@ import ( "math" "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) var ( @@ -113,7 +112,7 @@ func (s setterFloat64[T]) JawsSet(elem *jaws.Element, value float64) (err error) return } -func (s setterFloat64[T]) JawsGetTag(tag.Context) any { +func (s setterFloat64[T]) JawsGetTag() any { return s.Setter } @@ -130,7 +129,7 @@ func (setterFloat64ReadOnly[T]) JawsSet(elem *jaws.Element, value float64) error return ErrValueNotSettable } -func (s setterFloat64ReadOnly[T]) JawsGetTag(tag.Context) any { +func (s setterFloat64ReadOnly[T]) JawsGetTag() any { return s.Getter } @@ -146,7 +145,7 @@ func (s setterFloat64Static[T]) JawsGet(elem *jaws.Element) float64 { return s.v } -func (s setterFloat64Static[T]) JawsGetTag(tag.Context) any { +func (s setterFloat64Static[T]) JawsGetTag() any { return nil } diff --git a/lib/bind/setterfloat64_test.go b/lib/bind/setterfloat64_test.go index 77bef080..c98d06d5 100644 --- a/lib/bind/setterfloat64_test.go +++ b/lib/bind/setterfloat64_test.go @@ -78,7 +78,7 @@ func Test_makeSetterFloat64_int(t *testing.T) { t.Error(x) } tg := gotS.(tag.TagGetter) - if x := tg.JawsGetTag(nil); x != tsint { + if x := tg.JawsGetTag(); x != tsint { t.Error(x) } } @@ -337,7 +337,7 @@ func Test_makeSetterFloat64ReadOnly_int(t *testing.T) { t.Error(x) } tg := gotS.(tag.TagGetter) - if x := tg.JawsGetTag(nil); x != tgint { + if x := tg.JawsGetTag(); x != tgint { t.Error(x) } } @@ -353,7 +353,7 @@ func Test_makeSetterFloat64Static_int(t *testing.T) { t.Error(x) } tg := gotS.(tag.TagGetter) - if x := tg.JawsGetTag(nil); x != nil { + if x := tg.JawsGetTag(); x != nil { t.Error(x) } } diff --git a/lib/bind/stringgetterfunc.go b/lib/bind/stringgetterfunc.go index 20e92b80..78288e8c 100644 --- a/lib/bind/stringgetterfunc.go +++ b/lib/bind/stringgetterfunc.go @@ -1,8 +1,9 @@ package bind import ( + "slices" + "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) type stringGetterFunc struct { @@ -14,13 +15,17 @@ func (g *stringGetterFunc) JawsGet(elem *jaws.Element) string { return g.fn(elem) } -func (g *stringGetterFunc) JawsGetTag(tag.Context) any { +func (g *stringGetterFunc) JawsGetTag() any { return g.tags } // StringGetterFunc wraps fn as a [Getter] for string values. // -// Optional tags are exposed through [tag.TagGetter]. +// Optional tags are exposed through [github.com/linkdata/jaws/lib/tag.TagGetter]. +// +// The top-level slots of tags are copied, so the caller may reuse or modify the slice +// it passed. Nested containers and reference-backed tag values are not copied; keeping +// those stable remains the caller's obligation. func StringGetterFunc(fn func(elem *jaws.Element) (s string), tags ...any) Getter[string] { - return &stringGetterFunc{fn: fn, tags: tags} + return &stringGetterFunc{fn: fn, tags: slices.Clone(tags)} } diff --git a/lib/bind/stringgetterfunc_test.go b/lib/bind/stringgetterfunc_test.go index 66e15f87..a8a6b3a7 100644 --- a/lib/bind/stringgetterfunc_test.go +++ b/lib/bind/stringgetterfunc_test.go @@ -16,7 +16,19 @@ func TestStringGetterFunc(t *testing.T) { if s := sg.JawsGet(nil); s != "foo" { t.Error(s) } - if got := tag.MustTagExpand(nil, sg); !reflect.DeepEqual(got, []any{tt}) { + if got := mustExpand(t, sg); !reflect.DeepEqual(got, []any{tt}) { t.Error(got) } } + +// TestStringGetterFunc_SnapshotsTagSlots is the regression for issue #217; see +// TestHTMLGetterFunc_SnapshotsTagSlots for the shape of the bug. +func TestStringGetterFunc_SnapshotsTagSlots(t *testing.T) { + tags := []any{tag.Tag("original")} + sg := StringGetterFunc(func(*jaws.Element) string { return "x" }, tags...) + tags[0] = tag.Tag("reused") + + if got := mustExpand(t, sg); !reflect.DeepEqual(got, []any{tag.Tag("original")}) { + t.Fatalf("getter tags = %#v, want the construction-time snapshot", got) + } +} diff --git a/lib/bind/testsetter_test.go b/lib/bind/testsetter_test.go index 385f810e..b052930a 100644 --- a/lib/bind/testsetter_test.go +++ b/lib/bind/testsetter_test.go @@ -3,7 +3,6 @@ package bind import ( "github.com/linkdata/deadlock" "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) // testSetter is a minimal getter/setter fixture for the bind tests. It only @@ -49,6 +48,6 @@ func (ts *testSetter[T]) JawsSet(elem *jaws.Element, value T) (err error) { type selfTagger struct{} -func (st *selfTagger) JawsGetTag(tag.Context) any { +func (st *selfTagger) JawsGetTag() any { return st } diff --git a/lib/tag/context.go b/lib/tag/context.go deleted file mode 100644 index c50ead1b..00000000 --- a/lib/tag/context.go +++ /dev/null @@ -1,26 +0,0 @@ -package tag - -import ( - "context" - "net/http" -) - -// Context is the request state made available while expanding tags. -type Context interface { - // Initial returns the Request's initial HTTP request, or nil. - Initial() (r *http.Request) - // Get returns the JaWS session value for the key, or nil. - Get(key string) any - // Set sets the JaWS session value for the key. - Set(key string, value any) - // Context returns the Request's context. - Context() (ctx context.Context) - // Log sends an error to the Logger set in the Jaws. - // Has no effect if the err is nil or the Logger is nil. - // Returns err. - Log(err error) error - // MustLog sends an error to the Logger set in the Jaws or - // panics with the given error if no Logger is set. - // Has no effect if the err is nil. - MustLog(err error) -} diff --git a/lib/tag/doc.go b/lib/tag/doc.go index 1bde56e3..f859f9d0 100644 --- a/lib/tag/doc.go +++ b/lib/tag/doc.go @@ -5,4 +5,9 @@ // tags, including values whose static type is comparable but whose runtime // contents are not, and otherwise admissible values containing NaN that do not // equal themselves. +// +// [TagGetter] defines the contract an object implements to report its own tags, +// including the idempotent tag identity every implementation owes while its tags are +// registered on a live element. Expansion takes no request context: a tag value +// expands the same way regardless of which request or goroutine expands it. package tag diff --git a/lib/tag/errnotusableastag.go b/lib/tag/errnotusableastag.go index 739c2e56..451f703a 100644 --- a/lib/tag/errnotusableastag.go +++ b/lib/tag/errnotusableastag.go @@ -25,9 +25,9 @@ func (e errNotUsableAsTag) Error() (s string) { } s += "not usable as tag" if e.tagGetterType != nil { - return s + fmt.Sprintf("; found nested TagGetter at %s (%s); hint: implement JawsGetTag(tag.Context) on this type to delegate to that value, or pass that nested TagGetter directly", e.tagGetterPath, e.tagGetterType) + return s + fmt.Sprintf("; found nested TagGetter at %s (%s); hint: implement JawsGetTag() on this type to delegate to that value, or pass that nested TagGetter directly", e.tagGetterPath, e.tagGetterType) } - return s + "; found no nested TagGetter; hint: use a comparable tag value that equals itself, or implement JawsGetTag(tag.Context) and return one" + return s + "; found no nested TagGetter; hint: use a comparable tag value that equals itself, or implement JawsGetTag() and return one" } func (errNotUsableAsTag) Is(target error) bool { diff --git a/lib/tag/errnotusableastag_test.go b/lib/tag/errnotusableastag_test.go index 3b84fc7d..71438722 100644 --- a/lib/tag/errnotusableastag_test.go +++ b/lib/tag/errnotusableastag_test.go @@ -7,7 +7,7 @@ import ( type testFindTagGetter struct{} -func (testFindTagGetter) JawsGetTag(Context) any { +func (testFindTagGetter) JawsGetTag() any { return Tag("tg") } diff --git a/lib/tag/example_test.go b/lib/tag/example_test.go index ad4e1c69..84d815bc 100644 --- a/lib/tag/example_test.go +++ b/lib/tag/example_test.go @@ -11,13 +11,13 @@ type exampleItem struct { Name string } -func (item *exampleItem) JawsGetTag(tag.Context) any { +func (item *exampleItem) JawsGetTag() any { return item } func ExampleTagExpand_tagGetter() { item := &exampleItem{Name: "row"} - tags, err := tag.TagExpand(nil, []any{item, tag.Tag("list")}) + tags, err := tag.TagExpand([]any{item, tag.Tag("list")}) if err != nil { panic(err) } @@ -26,8 +26,30 @@ func ExampleTagExpand_tagGetter() { // Output: 2 true true } +// ExampleTagGetter shows the two supported ways to read an object's tags: +// JawsGetTag directly, which returns the raw value, and TagExpand, which flattens and +// validates it into keys. Both are stable for as long as the getter is idempotent. +func ExampleTagGetter() { + group := &exampleItem{Name: "group"} + item := &exampleItem{Name: "row"} + + // JawsGetTag is the canonical public accessor and may be called directly. + fmt.Println(item.JawsGetTag() == item) + + // TagExpand is how to obtain flattened, validated keys. + keys, err := tag.TagExpand([]any{item, group}) + if err != nil { + panic(err) + } + fmt.Println(len(keys), keys[0] == item, keys[1] == group) + + // Output: + // true + // 2 true true +} + func ExampleTagExpand_errorsIs() { - _, err := tag.TagExpand(nil, []int{1}) + _, err := tag.TagExpand([]int{1}) fmt.Println(errors.Is(err, tag.ErrNotUsableAsTag)) fmt.Println(errors.Is(err, tag.ErrNotComparable)) diff --git a/lib/tag/function_tagger_test.go b/lib/tag/function_tagger_test.go index f33ea0d1..b6d1b449 100644 --- a/lib/tag/function_tagger_test.go +++ b/lib/tag/function_tagger_test.go @@ -6,10 +6,10 @@ import ( "testing" ) -type testFunctionTagGetter func(Context) any +type testFunctionTagGetter func() any -func (fn testFunctionTagGetter) JawsGetTag(ctx Context) any { - return fn(ctx) +func (fn testFunctionTagGetter) JawsGetTag() any { + return fn() } func TestTagExpandDoesNotConflateDistinctFunctionTagGetters(t *testing.T) { @@ -18,7 +18,7 @@ func TestTagExpandDoesNotConflateDistinctFunctionTagGetters(t *testing.T) { getters := make([]testFunctionTagGetter, len(next)) for i := range next { i := i - getters[i] = func(Context) any { return next[i] } + getters[i] = func() any { return next[i] } } leafGetter := getters[0] rootGetter := getters[1] @@ -28,7 +28,7 @@ func TestTagExpandDoesNotConflateDistinctFunctionTagGetters(t *testing.T) { t.Skipf("compiler emitted distinct code pointers %#x and %#x", rootPtr, leafPtr) } - got, err := TagExpand(nil, rootGetter) + got, err := TagExpand(rootGetter) if err != nil { t.Fatal(err) } @@ -37,9 +37,9 @@ func TestTagExpandDoesNotConflateDistinctFunctionTagGetters(t *testing.T) { func TestTagExpandRecursiveFunctionTagGetterHitsDepthLimit(t *testing.T) { var recursive testFunctionTagGetter - recursive = func(Context) any { return recursive } + recursive = func() any { return recursive } - result, err := TagExpand(nil, recursive) + result, err := TagExpand(recursive) if !errors.Is(err, ErrTooManyTags) { t.Fatalf("TagExpand() error = %v, want %v", err, ErrTooManyTags) } @@ -49,7 +49,7 @@ func TestTagExpandRecursiveFunctionTagGetterHitsDepthLimit(t *testing.T) { } func TestSameActiveNodeDoesNotIdentifyFunctions(t *testing.T) { - fn := testFunctionTagGetter(func(Context) any { return Tag("leaf") }) + fn := testFunctionTagGetter(func() any { return Tag("leaf") }) if sameActiveNode(fn, fn) { t.Fatal("function code pointers do not identify function values") } diff --git a/lib/tag/nonreflexive_test.go b/lib/tag/nonreflexive_test.go index 92f93a9c..9dd55e69 100644 --- a/lib/tag/nonreflexive_test.go +++ b/lib/tag/nonreflexive_test.go @@ -30,7 +30,7 @@ func TestTagExpandRejectsNonReflexiveTags(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := TagExpand(nil, tt.tag) + result, err := TagExpand(tt.tag) if !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("TagExpand() error = %v, want %v", err, ErrNotUsableAsTag) } @@ -75,7 +75,7 @@ func TestNonReflexiveKindsAcceptFiniteTags(t *testing.T) { } for _, tag := range tests { - result, err := TagExpand(nil, tag) + result, err := TagExpand(tag) if err != nil { t.Fatalf("TagExpand(%#v) error = %v", tag, err) } diff --git a/lib/tag/tag.go b/lib/tag/tag.go index 7409bd07..86c76b68 100644 --- a/lib/tag/tag.go +++ b/lib/tag/tag.go @@ -134,15 +134,15 @@ func hasNonNilTag(tags []any) bool { return false } -func expand(depth int, ctx Context, tag any, result []any, active []any) ([]any, error) { +func expand(depth int, tagValue any, result []any, active []any) ([]any, error) { if depth > maxTagDepth || len(result) > maxTagCount { return result, ErrTooManyTags } - switch data := tag.(type) { + switch data := tagValue.(type) { case nil: return result, nil case Tag: - return appendUniqueTag(result, tag) + return appendUniqueTag(result, tagValue) case []Tag: if result == nil && len(data) > 0 { result = make([]any, 0, len(data)) @@ -158,7 +158,7 @@ func expand(depth int, ctx Context, tag any, result []any, active []any) ([]any, if idx := findActiveIndex(active, data); idx >= 0 { return addActiveTags(result, active[idx:]) } - return expand(depth+1, ctx, data.JawsGetTag(ctx), result, append(active, data)) + return expand(depth+1, data.JawsGetTag(), result, append(active, data)) case []any: if !hasNonNilTag(data) { return result, nil @@ -172,7 +172,7 @@ func expand(depth int, ctx Context, tag any, result []any, active []any) ([]any, active = append(active, data) var err error for _, v := range data { - if result, err = expand(depth+1, ctx, v, result, active); err != nil { + if result, err = expand(depth+1, v, result, active); err != nil { return result, err } } @@ -181,16 +181,16 @@ func expand(depth int, ctx Context, tag any, result []any, active []any) ([]any, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, key.Key, float32, float64, bool: - return result, errIllegalTagType{tag: tag} + return result, errIllegalTagType{tag: tagValue} default: return addTag(result, data) } } -// TagExpand expands tag into a flat list of unique, usable tag keys. +// TagExpand expands tagValue into a flat list of unique, usable tag keys. // -// tag may be nil, a [Tag], a slice of tags, a [TagGetter] or another value that is -// comparable at runtime and equals itself. The predeclared string, bool, signed +// tagValue may be nil, a [Tag], a slice of tags, a [TagGetter] or another value that +// is comparable at runtime and equals itself. The predeclared string, bool, signed // integer, unsigned integer other than uintptr, and floating-point types are // rejected with [ErrIllegalTagType], as are [template.HTML], [template.HTMLAttr], // [jid.Jid] and [key.Key]. This catches common accidental tags. An expanded key @@ -203,10 +203,20 @@ func expand(depth int, ctx Context, tag any, result []any, active []any) ([]any, // value is not usable as a tag key, result is nil and err matches // [ErrNotUsableAsTag]. // -// Expansion reads tag and any values returned by [TagGetter.JawsGetTag] by -// reference, so tag and those values must not be mutated concurrently with the -// call. -func TagExpand(ctx Context, tag any) (result []any, err error) { +// A single call may invoke a [TagGetter] more than once when the same value is +// reached at several points in the graph, and the same value may be expanded again by +// any later call. The [TagGetter] values emitted when a cycle is closed become keys +// themselves. Implementations must therefore satisfy [TagGetter]'s idempotent tag +// identity requirement. +// +// Expansion reads tagValue and any values returned by [TagGetter.JawsGetTag] by +// reference and never writes to them, so those values must not be mutated +// concurrently with the call. +// +// Errors are returned rather than logged. Use +// [github.com/linkdata/jaws.Jaws.MustTagExpand] to report them through a configured +// logger instead. +func TagExpand(tagValue any) (result []any, err error) { // ensureUsableTag rejects tags that are not comparable at runtime, so the // existing == tag dedup in appendUniqueTag does not panic on them. recover // stays as a defense-in-depth net: should a non-comparable value ever reach @@ -215,11 +225,11 @@ func TagExpand(ctx Context, tag any) (result []any, err error) { // anything else. defer func() { if r := recover(); r != nil { - result, err = recoverComparabilityPanic(r, tag) + result, err = recoverComparabilityPanic(r, tagValue) } }() var activeArr [12]any - return expand(0, ctx, tag, nil, activeArr[:0]) + return expand(0, tagValue, nil, activeArr[:0]) } // recoverComparabilityPanic maps a panic recovered from tag expansion to a @@ -235,20 +245,3 @@ func recoverComparabilityPanic(r any, tag any) (result []any, err error) { } panic(r) } - -// MustTagExpand calls [TagExpand] and either logs or panics if expansion fails. -// -// On a non-nil ctx, expansion errors are passed to [Context.MustLog] (which logs -// them, or panics if no Logger is set); MustTagExpand then returns the partial -// result from [TagExpand]. A nil ctx always panics on error. -func MustTagExpand(ctx Context, tag any) []any { - result, err := TagExpand(ctx, tag) - if err != nil { - if ctx != nil { - ctx.MustLog(err) - } else { - panic(err) - } - } - return result -} diff --git a/lib/tag/tag_benchmark_test.go b/lib/tag/tag_benchmark_test.go index 222c15f4..95b0d43d 100644 --- a/lib/tag/tag_benchmark_test.go +++ b/lib/tag/tag_benchmark_test.go @@ -4,7 +4,7 @@ import "testing" type benchSelfTagger struct{} -func (t *benchSelfTagger) JawsGetTag(Context) any { +func (t *benchSelfTagger) JawsGetTag() any { return t } @@ -12,7 +12,7 @@ type benchChainTagger struct { next any } -func (t *benchChainTagger) JawsGetTag(Context) any { +func (t *benchChainTagger) JawsGetTag() any { return t.next } @@ -20,21 +20,21 @@ type benchSliceTagger struct { tags []any } -func (t *benchSliceTagger) JawsGetTag(Context) any { +func (t *benchSliceTagger) JawsGetTag() any { return t.tags } -type benchFunctionTagger func(Context) any +type benchFunctionTagger func() any -func (fn benchFunctionTagger) JawsGetTag(ctx Context) any { - return fn(ctx) +func (fn benchFunctionTagger) JawsGetTag() any { + return fn() } -func benchFunctionLeaf(Context) any { +func benchFunctionLeaf() any { return Tag("leaf") } -func benchFunctionRoot(Context) any { +func benchFunctionRoot() any { return benchFunctionTagger(benchFunctionLeaf) } @@ -51,7 +51,7 @@ func benchmarkTagExpandCase(b *testing.B, tag any) { b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - got, err := TagExpand(nil, tag) + got, err := TagExpand(tag) if err != nil { b.Fatal(err) } diff --git a/lib/tag/tag_test.go b/lib/tag/tag_test.go index cd7c59cb..15c0738d 100644 --- a/lib/tag/tag_test.go +++ b/lib/tag/tag_test.go @@ -1,31 +1,28 @@ package tag import ( - "context" "errors" "fmt" "html/template" - "net/http" "reflect" "runtime" "strings" "sync/atomic" "testing" - "github.com/linkdata/deadlock" "github.com/linkdata/jaws/lib/jid" "github.com/linkdata/jaws/lib/key" ) type testSelfTagger struct{} -func (tt *testSelfTagger) JawsGetTag(Context) any { +func (tt *testSelfTagger) JawsGetTag() any { return tt } type testBadTagGetter []int -func (tt testBadTagGetter) JawsGetTag(Context) any { +func (tt testBadTagGetter) JawsGetTag() any { return tt } @@ -35,19 +32,19 @@ func (testStringTag) String() string { return "str" } type testNestedTagGetter struct{} -func (testNestedTagGetter) JawsGetTag(Context) any { +func (testNestedTagGetter) JawsGetTag() any { return Tag("nested") } type testSelfSliceTagger struct{} -func (tt *testSelfSliceTagger) JawsGetTag(Context) any { +func (tt *testSelfSliceTagger) JawsGetTag() any { return []any{tt} } type testSelfSliceExtraTagger struct{} -func (tt *testSelfSliceExtraTagger) JawsGetTag(Context) any { +func (tt *testSelfSliceExtraTagger) JawsGetTag() any { return []any{tt, Tag("extra")} } @@ -56,7 +53,7 @@ type testMutualSliceTagger struct { name string } -func (tt *testMutualSliceTagger) JawsGetTag(Context) any { +func (tt *testMutualSliceTagger) JawsGetTag() any { return []any{tt.next} } @@ -67,7 +64,7 @@ func (tt *testMutualSliceTagger) JawsGetTag(Context) any { // be treated as distinct active nodes. type testCapTagger []int -func (c testCapTagger) JawsGetTag(Context) any { +func (c testCapTagger) JawsGetTag() any { if cap(c) > len(c) { return []any{Tag("outer"), c[:len(c):len(c)]} } @@ -87,7 +84,7 @@ type testDeepTagGetter struct { next any } -func (tt testDeepTagGetter) JawsGetTag(Context) any { +func (tt testDeepTagGetter) JawsGetTag() any { return tt.next } @@ -324,16 +321,16 @@ func TestTagExpand(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assertTagSetEqual(t, MustTagExpand(nil, tt.tag), tt.want) + got, err := TagExpand(tt.tag) + if err != nil { + t.Fatalf("TagExpand(%#v) error = %v", tt.tag, err) + } + assertTagSetEqual(t, got, tt.want) }) } } -func TestTagExpand_IllegalTypesPanic(t *testing.T) { - if !deadlock.Debug { - t.Log("skipped, not debugging") - return - } +func TestTagExpand_IllegalTypes(t *testing.T) { tags := []any{ string("string"), template.HTML("template.HTML"), @@ -358,21 +355,16 @@ func TestTagExpand_IllegalTypesPanic(t *testing.T) { } for _, tag := range tags { t.Run(fmt.Sprintf("%T", tag), func(t *testing.T) { - defer func() { - x := recover() - e, ok := x.(error) - if !ok { - t.FailNow() - } - if !(errors.Is(e, ErrIllegalTagType) || errors.Is(e, ErrNotComparable)) { - t.FailNow() - } - if !strings.Contains(e.Error(), fmt.Sprintf("%T", tag)) { - t.FailNow() - } - }() - MustTagExpand(nil, tag) - t.FailNow() + _, err := TagExpand(tag) + if err == nil { + t.Fatalf("TagExpand(%T) accepted an illegal tag type", tag) + } + if !(errors.Is(err, ErrIllegalTagType) || errors.Is(err, ErrNotComparable)) { + t.Fatalf("TagExpand(%T) error = %v, want ErrIllegalTagType or ErrNotComparable", tag, err) + } + if !strings.Contains(err.Error(), fmt.Sprintf("%T", tag)) { + t.Fatalf("TagExpand(%T) error %q does not name the offending type", tag, err) + } }) } } @@ -380,7 +372,7 @@ func TestTagExpand_IllegalTypesPanic(t *testing.T) { func TestTagExpand_SelfReferentialSliceStopsRecursing(t *testing.T) { tags := []any{nil} tags[0] = tags - got, err := TagExpand(nil, tags) + got, err := TagExpand(tags) if err != nil { t.Fatal(err) } @@ -401,7 +393,7 @@ func TestTagExpand_AliasedSliceViews(t *testing.T) { all[1] = all all[2] = Tag("last") - got, err := TagExpand(nil, outer) + got, err := TagExpand(outer) if err != nil { t.Fatal(err) } @@ -421,37 +413,29 @@ func TestTagExpand_CapacityDependentSliceGetter(t *testing.T) { backing := make(testCapTagger, 2) outer := backing[:1] // len 1, cap 2: yields Tag("outer") and a cap-1 inner view - got, err := TagExpand(nil, outer) + got, err := TagExpand(outer) if err != nil { t.Fatal(err) } assertTagSetEqual(t, got, []any{Tag("outer"), Tag("inner")}) } -func TestTagExpand_TooManyTagsPanic(t *testing.T) { +func TestTagExpand_TooManyTags(t *testing.T) { tags := make([]any, 101) for i := range tags { tags[i] = Tag(fmt.Sprintf("t%d", i)) } - defer func() { - x := recover() - e, ok := x.(error) - if !ok { - t.Fatal("expected error, got", x) - } - if !errors.Is(e, ErrTooManyTags) { - t.Errorf("recovered error = %v, want %v", e, ErrTooManyTags) - } - if e.Error() != "too many tags" { - t.Errorf("ErrTooManyTags.Error() = %q", e.Error()) - } - }() - MustTagExpand(nil, tags) - t.FailNow() + _, err := TagExpand(tags) + if !errors.Is(err, ErrTooManyTags) { + t.Fatalf("TagExpand error = %v, want %v", err, ErrTooManyTags) + } + if err.Error() != "too many tags" { + t.Errorf("ErrTooManyTags.Error() = %q", err.Error()) + } } func TestTagExpand_TagGetterNonComparable(t *testing.T) { - _, err := TagExpand(nil, testBadTagGetter{1}) + _, err := TagExpand(testBadTagGetter{1}) if !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("expected ErrNotUsableAsTag, got %v", err) } @@ -469,7 +453,7 @@ func TestTagExpand_TagGetterNonComparable(t *testing.T) { // map key. Tag expansion must reject even a single such tag with // ErrNotUsableAsTag rather than deferring that panic to jw.dirty or rq.tagMap. func TestTagExpand_RuntimeNonComparable(t *testing.T) { - if _, err := TagExpand(nil, testRuntimeNonComparable{v: func() {}}); !errors.Is(err, ErrNotUsableAsTag) { + if _, err := TagExpand(testRuntimeNonComparable{v: func() {}}); !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("expected ErrNotUsableAsTag, got %v", err) } } @@ -482,7 +466,7 @@ func TestTagExpand_RuntimeNonComparable(t *testing.T) { func TestTagExpand_MultiRuntimeNonComparable(t *testing.T) { a := testRuntimeNonComparable{v: func() {}} b := testRuntimeNonComparable{v: func() {}} - result, err := TagExpand(nil, []any{a, b}) + result, err := TagExpand([]any{a, b}) if !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("expected ErrNotUsableAsTag, got %v", err) } @@ -496,7 +480,7 @@ func TestTagExpand_MultiRuntimeNonComparable(t *testing.T) { // ErrNotUsableAsTag and not leak the partial result accumulated before the // rejection. func TestTagExpand_ValidThenRuntimeNonComparable(t *testing.T) { - result, err := TagExpand(nil, []any{Tag("a"), testRuntimeNonComparable{v: func() {}}}) + result, err := TagExpand([]any{Tag("a"), testRuntimeNonComparable{v: func() {}}}) if !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("expected ErrNotUsableAsTag, got %v", err) } @@ -509,7 +493,7 @@ func TestTagExpand_ValidThenRuntimeNonComparable(t *testing.T) { // comparable ([1]any) but whose element holds a non-comparable value (a func). // Comparing it panics, so expansion must reject it with ErrNotUsableAsTag. func TestTagExpand_RuntimeNonComparableArray(t *testing.T) { - if _, err := TagExpand(nil, [1]any{func() {}}); !errors.Is(err, ErrNotUsableAsTag) { + if _, err := TagExpand([1]any{func() {}}); !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("expected ErrNotUsableAsTag, got %v", err) } } @@ -530,12 +514,12 @@ func TestTagExpand_RepanicsOtherPanics(t *testing.T) { t.Fatalf("unexpected panic value %v", r) } }() - _, _ = TagExpand(nil, testPanicTagGetter{}) + _, _ = TagExpand(testPanicTagGetter{}) } type testPanicTagGetter struct{} -func (testPanicTagGetter) JawsGetTag(Context) any { panic("boom") } +func (testPanicTagGetter) JawsGetTag() any { panic("boom") } // uncomparablePanic returns a real "comparing uncomparable type" runtime panic // value by comparing two non-comparable interface values. @@ -582,7 +566,7 @@ func TestTagExpand_NotUsableAsTag_WithNestedTagGetterHint(t *testing.T) { Setter: testNestedTagGetter{}, Vals: []int{1}, } - _, err := TagExpand(nil, tag) + _, err := TagExpand(tag) if !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("expected ErrNotUsableAsTag, got %v", err) } @@ -592,13 +576,13 @@ func TestTagExpand_NotUsableAsTag_WithNestedTagGetterHint(t *testing.T) { if !strings.Contains(err.Error(), "found nested TagGetter at Setter") { t.Fatalf("expected nested TagGetter search result in error text, got %q", err.Error()) } - if !strings.Contains(err.Error(), "implement JawsGetTag(tag.Context)") { + if !strings.Contains(err.Error(), "implement JawsGetTag()") { t.Fatalf("expected remediation hint in error text, got %q", err.Error()) } } func TestTagExpand_NotUsableAsTag_NoNestedTagGetterHint(t *testing.T) { - _, err := TagExpand(nil, map[int]int{1: 1}) + _, err := TagExpand(map[int]int{1: 1}) if !errors.Is(err, ErrNotUsableAsTag) { t.Fatalf("expected ErrNotUsableAsTag, got %v", err) } @@ -614,7 +598,7 @@ func TestTagExpand_NotUsableAsTag_NoNestedTagGetterHint(t *testing.T) { } func TestTagExpand_IllegalTagTypeError(t *testing.T) { - _, err := TagExpand(nil, "plain-string") + _, err := TagExpand("plain-string") if !errors.Is(err, ErrIllegalTagType) { t.Fatalf("expected ErrIllegalTagType, got %v", err) } @@ -651,7 +635,7 @@ func TestTagExpand_IllegalTypesAsErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := TagExpand(nil, tt.tag) + _, err := TagExpand(tt.tag) if !errors.Is(err, tt.wantErr) { t.Fatalf("TagExpand(%T): got %v want %v", tt.tag, err, tt.wantErr) } @@ -659,36 +643,8 @@ func TestTagExpand_IllegalTypesAsErrors(t *testing.T) { } } -type mustLogContext struct { - ctx context.Context - err error -} - -func (ctx *mustLogContext) Initial() *http.Request { - return nil -} - -func (ctx *mustLogContext) Get(string) any { - return nil -} - -func (ctx *mustLogContext) Set(key string, value any) {} - -func (ctx *mustLogContext) Context() context.Context { - return ctx.ctx -} - -func (ctx *mustLogContext) Log(err error) error { - ctx.err = err - return err -} - -func (ctx *mustLogContext) MustLog(err error) { - ctx.err = err -} - func TestTagExpand_TagGetterRecurses(t *testing.T) { - got, err := TagExpand(nil, testNestedTagGetter{}) + got, err := TagExpand(testNestedTagGetter{}) if err != nil { t.Fatal(err) } @@ -697,7 +653,7 @@ func TestTagExpand_TagGetterRecurses(t *testing.T) { func TestTagExpand_TagGetterSelfInSlice(t *testing.T) { self := &testSelfSliceTagger{} - got, err := TagExpand(nil, self) + got, err := TagExpand(self) if err != nil { t.Fatal(err) } @@ -706,7 +662,7 @@ func TestTagExpand_TagGetterSelfInSlice(t *testing.T) { func TestTagExpand_TagGetterSelfAndExtraInSlice(t *testing.T) { self := &testSelfSliceExtraTagger{} - got, err := TagExpand(nil, self) + got, err := TagExpand(self) if err != nil { t.Fatal(err) } @@ -717,21 +673,23 @@ func TestTagExpand_TagGetterMutualCycleExpandsToCycleMembers(t *testing.T) { a := &testMutualSliceTagger{name: "a"} b := &testMutualSliceTagger{name: "b", next: a} a.next = b - got, err := TagExpand(nil, a) + got, err := TagExpand(a) if err != nil { t.Fatal(err) } assertTagSetEqual(t, got, []any{a, b}) } -func TestMustTagExpand_UsesContextMustLog(t *testing.T) { - ctx := &mustLogContext{ctx: t.Context()} - got := MustTagExpand(ctx, "plain-string") +// TagExpand returns expansion errors directly. Jaws.MustTagExpand's logging and panic +// behavior is covered by tagexpand_test.go in the root package; importing jaws here +// would make tag depend on its own consumer. +func TestTagExpand_IllegalTagTypeIsReturnedNotLogged(t *testing.T) { + got, err := TagExpand("plain-string") if got != nil { - t.Fatalf("MustTagExpand returned %#v, want nil", got) + t.Fatalf("TagExpand returned %#v, want nil", got) } - if !errors.Is(ctx.err, ErrIllegalTagType) { - t.Fatalf("expected ErrIllegalTagType, got %v", ctx.err) + if !errors.Is(err, ErrIllegalTagType) { + t.Fatalf("expected ErrIllegalTagType, got %v", err) } } @@ -799,7 +757,7 @@ func TestTagExpand_TooDeepAndTooManySliceTags(t *testing.T) { for range 11 { nested = testDeepTagGetter{next: nested} } - if _, err := TagExpand(nil, nested); !errors.Is(err, ErrTooManyTags) { + if _, err := TagExpand(nested); !errors.Is(err, ErrTooManyTags) { t.Fatalf("TagExpand(deep) = %v, want %v", err, ErrTooManyTags) } @@ -807,7 +765,7 @@ func TestTagExpand_TooDeepAndTooManySliceTags(t *testing.T) { for i := range tags { tags[i] = Tag(fmt.Sprintf("t%d", i)) } - if _, err := TagExpand(nil, tags); !errors.Is(err, ErrTooManyTags) { + if _, err := TagExpand(tags); !errors.Is(err, ErrTooManyTags) { t.Fatalf("TagExpand([]Tag) = %v, want %v", err, ErrTooManyTags) } } @@ -821,7 +779,7 @@ func TestTagExpand_PartialResultOnCountLimit(t *testing.T) { for i := range tags { tags[i] = Tag(fmt.Sprintf("t%d", i)) } - result, err := TagExpand(nil, tags) + result, err := TagExpand(tags) if !errors.Is(err, ErrTooManyTags) { t.Fatalf("TagExpand([]Tag) error = %v, want %v", err, ErrTooManyTags) } diff --git a/lib/tag/taggetter.go b/lib/tag/taggetter.go index fa14e397..16b933a9 100644 --- a/lib/tag/taggetter.go +++ b/lib/tag/taggetter.go @@ -1,10 +1,32 @@ package tag -// TagGetter exposes dynamic tags during [TagExpand]. +// TagGetter exposes an object's lazily resolved tags to [TagExpand]. +// +// JawsGetTag is the canonical public accessor for an object's tags, and application +// code may call it directly. Callers needing flattened, validated keys should pass the +// object to [TagExpand] rather than interpret the raw return value. +// +// [github.com/linkdata/jaws.Element.ApplyGetter] invokes JawsGetTag to obtain a tag +// candidate, then expands that candidate for registration. This expansion may invoke +// JawsGetTag again when the candidate is itself a TagGetter or contains one. Standard +// getter-backed widgets invoke ApplyGetter once during an Element's initial render. +// [TagExpand], dirtying, broadcasts, and application code may make further calls; +// there is no call-count guarantee. +// +// Except for an explicitly documented initialization phase that returns nil, a +// TagGetter must be idempotent in tag identity. After its first non-nil result, every +// call must return a value that [TagExpand] expands to the same set of keys. Previously +// returned containers must continue expanding to the key set they produced when +// returned and must be treated as read-only. Fresh containers and equivalent +// representations are allowed. Non-idempotent TagGetter implementations are +// unsupported. +// +// JaWS does not serialize JawsGetTag calls. A getter used concurrently must synchronize +// its state and safely publish any returned containers. +// +// [github.com/linkdata/jaws.Request.TagsOf] reports every tag actually registered on an +// Element, including tags added separately from the UI object. type TagGetter interface { - // JawsGetTag returns the dynamic tag or tags for the implementing object. - // - // ctx may be nil — [TagExpand] is routinely called with a nil [Context] — so - // implementations must not dereference it unconditionally. - JawsGetTag(ctx Context) any + // JawsGetTag returns the tag or tags for the implementing object. + JawsGetTag() any } diff --git a/lib/tag/taggetter_test.go b/lib/tag/taggetter_test.go new file mode 100644 index 00000000..bf0a91f9 --- /dev/null +++ b/lib/tag/taggetter_test.go @@ -0,0 +1,90 @@ +package tag + +import ( + "testing" +) + +// freshSliceTagger returns an equal but freshly allocated container on every call, +// which the TagGetter contract explicitly permits: only the expanded key set has to +// stay the same. +type freshSliceTagger struct { + keys []Tag +} + +func (g *freshSliceTagger) JawsGetTag() any { + out := make([]any, 0, len(g.keys)) + for _, k := range g.keys { + out = append(out, k) + } + return out +} + +func TestTagGetter_FreshContainersExpandToTheSameKeySet(t *testing.T) { + g := &freshSliceTagger{keys: []Tag{Tag("a"), Tag("b")}} + first, err := TagExpand(g) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, first, []any{Tag("a"), Tag("b")}) + + second, err := TagExpand(g) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, second, first) +} + +// sharedLeafTagger lets one getter be reached from two points of a single graph. +type sharedLeafTagger struct { + key Tag +} + +func (g *sharedLeafTagger) JawsGetTag() any { return g.key } + +// TestTagGetter_RepeatedGraphOccurrenceDeduplicates asserts the observable outcome of a +// getter reached more than once in one expansion: a single deduplicated key set. It +// deliberately does not assert how many times JawsGetTag was called, since the contract +// makes no such guarantee and a future per-expansion cache must stay compliant. +func TestTagGetter_RepeatedGraphOccurrenceDeduplicates(t *testing.T) { + leaf := &sharedLeafTagger{key: Tag("leaf")} + got, err := TagExpand([]any{leaf, Tag("other"), []any{leaf}}) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, got, []any{Tag("leaf"), Tag("other")}) +} + +func TestTagGetter_StableNestedAndCyclicGettersRepeatIdentically(t *testing.T) { + t.Run("nested", func(t *testing.T) { + inner := &sharedLeafTagger{key: Tag("inner")} + outer := testDeepTagGetter{next: []any{Tag("outer"), inner}} + first, err := TagExpand(outer) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, first, []any{Tag("outer"), Tag("inner")}) + second, err := TagExpand(outer) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, second, first) + }) + + t.Run("mutual cycle", func(t *testing.T) { + a := &testMutualSliceTagger{name: "a"} + b := &testMutualSliceTagger{name: "b", next: a} + a.next = b + // A closed cycle yields the participating getters as keys, so repeating the + // expansion must produce that same pair. + first, err := TagExpand(a) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, first, []any{a, b}) + second, err := TagExpand(a) + if err != nil { + t.Fatal(err) + } + assertTagSetEqual(t, second, first) + }) +} diff --git a/lib/ui/README.md b/lib/ui/README.md index 92a26dc3..107d0409 100644 --- a/lib/ui/README.md +++ b/lib/ui/README.md @@ -191,10 +191,7 @@ func NewArticle(inner any) *Article { } func (w *Article) JawsRender(e *jaws.Element, wr io.Writer, params []any) error { - _, getterAttrs, err := e.ApplyGetter(w.HTMLGetter) - if err != nil { - return err - } + _, getterAttrs := e.ApplyGetter(w.HTMLGetter) attrs := append(e.ApplyParams(params), getterAttrs...) return htmlio.WriteHTMLInner(wr, e.Jid(), "article", "", w.HTMLGetter.JawsGetHTML(e), attrs...) } diff --git a/lib/ui/clickable_test.go b/lib/ui/clickable_test.go index ad91f126..4d4c6e39 100644 --- a/lib/ui/clickable_test.go +++ b/lib/ui/clickable_test.go @@ -44,7 +44,7 @@ func TestClickable_ForwardsClickAndGetterBehavior(t *testing.T) { if !ok { t.Fatalf("%T does not implement tag.TagGetter", handler) } - if got, want := tagGetter.JawsGetTag(rq), any(inner); got != want { + if got, want := tagGetter.JawsGetTag(), any(inner); got != want { t.Fatalf("want tag %#v got %#v", want, got) } @@ -65,7 +65,7 @@ func TestClickable_TagIsNilWhenInnerHTMLHasNoTag(t *testing.T) { if !ok { t.Fatalf("%T does not implement tag.TagGetter", handler) } - if gotTag := tagGetter.JawsGetTag(nil); gotTag != nil { + if gotTag := tagGetter.JawsGetTag(); gotTag != nil { t.Fatalf("expected nil tag, got %#v", gotTag) } } diff --git a/lib/ui/containerhelper.go b/lib/ui/containerhelper.go index 2d372768..39b69703 100644 --- a/lib/ui/containerhelper.go +++ b/lib/ui/containerhelper.go @@ -57,51 +57,50 @@ func NewContainerHelper(c jaws.Container) ContainerHelper { // [jaws.Container.JawsContains]. func (u *ContainerHelper) RenderContainer(elem *jaws.Element, w io.Writer, outerHTMLTag string, params []any) (err error) { var getterAttrs []template.HTMLAttr - if u.tag, getterAttrs, err = elem.ApplyGetter(u.Container); err == nil { - attrs := append(elem.ApplyParams(params), getterAttrs...) - b := elem.Jid().AppendStartTagAttr(nil, outerHTMLTag) - b = htmlio.AppendAttrs(b, attrs) - b = append(b, '>') - _, err = w.Write(b) - if err == nil { - var contents []*jaws.Element - // Validate every child before creating any Element: an unusable child (one - // that is not comparable at runtime, or not equal to itself) terminates the - // Request, and the rest must not be rendered. Keeping unusable children out of - // u.contents also stops a later reconcile pool build from hashing one. - children := u.Container.JawsContains(elem) - if !cancelUnusableChildren(elem, children) { - for _, childUI := range children { - childElem := elem.Request.NewElement(childUI) - contents = append(contents, childElem) - if err = childElem.JawsRender(w, nil); err != nil { - break - } + u.tag, getterAttrs = elem.ApplyGetter(u.Container) + attrs := append(elem.ApplyParams(params), getterAttrs...) + b := elem.Jid().AppendStartTagAttr(nil, outerHTMLTag) + b = htmlio.AppendAttrs(b, attrs) + b = append(b, '>') + _, err = w.Write(b) + if err == nil { + var contents []*jaws.Element + // Validate every child before creating any Element: an unusable child (one + // that is not comparable at runtime, or not equal to itself) terminates the + // Request, and the rest must not be rendered. Keeping unusable children out of + // u.contents also stops a later reconcile pool build from hashing one. + children := u.Container.JawsContains(elem) + if !cancelUnusableChildren(elem, children) { + for _, childUI := range children { + childElem := elem.Request.NewElement(childUI) + contents = append(contents, childElem) + if err = childElem.JawsRender(w, nil); err != nil { + break } } - // Always emit the closing tag, even on a child-render error, to balance - // the start tag already written above; leaving it unclosed would be - // worse for any partial output. The original err is preserved (err2 is - // only adopted when err is nil). - b = b[:0] - b = append(b, "') - if _, err2 := w.Write(b); err == nil { - err = err2 - } - // Commit the rendered children only on full success. Any failure — a - // child render or the closing-tag write above — deletes the child - // Elements created during this render; otherwise they leak in the - // Request registry, since RequestWriter.NewUI deletes only the parent - // Element on a failed render. - if err == nil { - u.mu.Lock() - u.contents = contents - u.mu.Unlock() - } else { - deleteOwnedElements(elem.Request, contents) - } + } + // Always emit the closing tag, even on a child-render error, to balance + // the start tag already written above; leaving it unclosed would be + // worse for any partial output. The original err is preserved (err2 is + // only adopted when err is nil). + b = b[:0] + b = append(b, "') + if _, err2 := w.Write(b); err == nil { + err = err2 + } + // Commit the rendered children only on full success. Any failure — a + // child render or the closing-tag write above — deletes the child + // Elements created during this render; otherwise they leak in the + // Request registry, since RequestWriter.NewUI deletes only the parent + // Element on a failed render. + if err == nil { + u.mu.Lock() + u.contents = contents + u.mu.Unlock() + } else { + deleteOwnedElements(elem.Request, contents) } } return diff --git a/lib/ui/html_widgets.go b/lib/ui/html_widgets.go index d62d3f68..e9851ba0 100644 --- a/lib/ui/html_widgets.go +++ b/lib/ui/html_widgets.go @@ -1,7 +1,6 @@ package ui import ( - "html/template" "io" "github.com/linkdata/jaws" @@ -20,11 +19,9 @@ type HTMLInner struct { } func (u *HTMLInner) renderInner(elem *jaws.Element, w io.Writer, htmlTag, htmlType string, params []any) (err error) { - var getterAttrs []template.HTMLAttr - if _, getterAttrs, err = elem.ApplyGetter(u.HTMLGetter); err == nil { - attrs := append(elem.ApplyParams(params), getterAttrs...) - err = htmlio.WriteHTMLInner(w, elem.Jid(), htmlTag, htmlType, u.HTMLGetter.JawsGetHTML(elem), attrs...) - } + _, getterAttrs := elem.ApplyGetter(u.HTMLGetter) + attrs := append(elem.ApplyParams(params), getterAttrs...) + err = htmlio.WriteHTMLInner(w, elem.Jid(), htmlTag, htmlType, u.HTMLGetter.JawsGetHTML(elem), attrs...) return } diff --git a/lib/ui/html_widgets_test.go b/lib/ui/html_widgets_test.go index 1d028ed7..638f90c8 100644 --- a/lib/ui/html_widgets_test.go +++ b/lib/ui/html_widgets_test.go @@ -9,7 +9,6 @@ import ( "github.com/linkdata/jaws" "github.com/linkdata/jaws/lib/bind" "github.com/linkdata/jaws/lib/named" - "github.com/linkdata/jaws/lib/tag" ) func TestHTMLWidgets_ConstructorsAndRender(t *testing.T) { @@ -56,26 +55,18 @@ func TestHTMLWidgets_StringGetterInnerHTMLIsEscaped(t *testing.T) { mustMatch(t, `^
<b>x</b>
$`, got) } -func TestHTMLInner_RenderInnerApplyGetterError(t *testing.T) { +// TestHTMLInner_RenderInnerWriteError covers renderInner's error return, whose sole +// failure source is the writer because ApplyGetter has no error result. +func TestHTMLInner_RenderInnerWriteError(t *testing.T) { _, rq := newCoreRequest(t) - wantErr := errors.New("init fail") - g := &initFailGetter{err: wantErr} - elem := rq.NewElement(NewA(g)) - var sb strings.Builder - if err := elem.JawsRender(&sb, nil); !errors.Is(err, wantErr) { + wantErr := errors.New("write fail") + elem := rq.NewElement(NewA(testHTMLGetter("x"))) + if err := elem.JawsRender(&failNthWrite{n: 1, err: wantErr}, nil); !errors.Is(err, wantErr) { t.Fatalf("want %v got %v", wantErr, err) } } -type initFailGetter struct { - err error -} - -func (g *initFailGetter) JawsGetHTML(elem *jaws.Element) template.HTML { return "x" } -func (g *initFailGetter) JawsGetTag(tag.Context) any { return g } -func (g *initFailGetter) JawsInit(elem *jaws.Element) error { return g.err } - func TestImg_RenderAndUpdate(t *testing.T) { _, rq := newCoreRequest(t) src := newTestSetter("image.png") diff --git a/lib/ui/img.go b/lib/ui/img.go index 945aa310..2c085877 100644 --- a/lib/ui/img.go +++ b/lib/ui/img.go @@ -1,7 +1,6 @@ package ui import ( - "html/template" "io" "github.com/linkdata/jaws" @@ -21,13 +20,11 @@ func NewImg(g bind.Getter[string]) *Img { return &Img{Getter: g} } // JawsRender renders ui as an HTML img element. func (u *Img) JawsRender(elem *jaws.Element, w io.Writer, params []any) (err error) { - var getterAttrs []template.HTMLAttr - if _, getterAttrs, err = elem.ApplyGetter(u.Getter); err == nil { - srcAttr := htmlio.Attr("src", u.JawsGet(elem)) - attrs := append(elem.ApplyParams(params), getterAttrs...) - attrs = append(attrs, srcAttr) - err = htmlio.WriteHTMLInner(w, elem.Jid(), "img", "", "", attrs...) - } + _, getterAttrs := elem.ApplyGetter(u.Getter) + srcAttr := htmlio.Attr("src", u.JawsGet(elem)) + attrs := append(elem.ApplyParams(params), getterAttrs...) + attrs = append(attrs, srcAttr) + err = htmlio.WriteHTMLInner(w, elem.Jid(), "img", "", "", attrs...) return } diff --git a/lib/ui/input_widgets.go b/lib/ui/input_widgets.go index 54d00772..1206ec77 100644 --- a/lib/ui/input_widgets.go +++ b/lib/ui/input_widgets.go @@ -30,8 +30,8 @@ type Input struct { Last atomic.Value // last rendered or accepted browser value for the Element } -func (u *Input) applyGetterAttrs(elem *jaws.Element, getter any) (attrs []template.HTMLAttr, err error) { - u.tag, attrs, err = elem.ApplyGetter(getter) +func (u *Input) applyGetterAttrs(elem *jaws.Element, getter any) (attrs []template.HTMLAttr) { + u.tag, attrs = elem.ApplyGetter(getter) return } @@ -51,13 +51,11 @@ type InputText struct { } func (u *InputText) renderStringInput(elem *jaws.Element, w io.Writer, htmlType string, params ...any) (err error) { - var getterAttrs []template.HTMLAttr - if getterAttrs, err = u.applyGetterAttrs(elem, u.Setter); err == nil { - attrs := append(elem.ApplyParams(params), getterAttrs...) - v := u.JawsGet(elem) - u.Last.Store(v) - err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, v, attrs) - } + getterAttrs := u.applyGetterAttrs(elem, u.Setter) + attrs := append(elem.ApplyParams(params), getterAttrs...) + v := u.JawsGet(elem) + u.Last.Store(v) + err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, v, attrs) return } @@ -87,16 +85,14 @@ type InputBool struct { } func (u *InputBool) renderBoolInput(elem *jaws.Element, w io.Writer, htmlType string, params ...any) (err error) { - var getterAttrs []template.HTMLAttr - if getterAttrs, err = u.applyGetterAttrs(elem, u.Setter); err == nil { - attrs := append(elem.ApplyParams(params), getterAttrs...) - v := u.JawsGet(elem) - u.Last.Store(v) - if v { - attrs = append(attrs, "checked") - } - err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, "", attrs) + getterAttrs := u.applyGetterAttrs(elem, u.Setter) + attrs := append(elem.ApplyParams(params), getterAttrs...) + v := u.JawsGet(elem) + u.Last.Store(v) + if v { + attrs = append(attrs, "checked") } + err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, "", attrs) return } @@ -153,17 +149,15 @@ func (u *InputFloat) str(value float64) string { } func (u *InputFloat) renderFloatInput(elem *jaws.Element, w io.Writer, htmlType string, params ...any) (err error) { - var getterAttrs []template.HTMLAttr - if getterAttrs, err = u.applyGetterAttrs(elem, u.Setter); err == nil { - v := u.JawsGet(elem) - if !finite(v) { - elem.Cancel(fmt.Errorf("%w: %g", jaws.ErrValueNotFinite, v)) - return - } - u.Last.Store(v) - attrs := append(elem.ApplyParams(params), getterAttrs...) - err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, u.str(v), attrs) + getterAttrs := u.applyGetterAttrs(elem, u.Setter) + v := u.JawsGet(elem) + if !finite(v) { + elem.Cancel(fmt.Errorf("%w: %g", jaws.ErrValueNotFinite, v)) + return } + u.Last.Store(v) + attrs := append(elem.ApplyParams(params), getterAttrs...) + err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, u.str(v), attrs) return } @@ -241,16 +235,14 @@ func (u *InputDate) str(v time.Time) string { } func (u *InputDate) renderDateInput(elem *jaws.Element, w io.Writer, htmlType string, params ...any) (err error) { - var getterAttrs []template.HTMLAttr - if getterAttrs, err = u.applyGetterAttrs(elem, u.Setter); err == nil { - attrs := append(elem.ApplyParams(params), getterAttrs...) - v := u.JawsGet(elem) - // Dedup on the rendered ISO8601 string, not the raw time.Time: comparing - // time.Time with == also compares the monotonic reading and *Location, so - // equal calendar dates can compare unequal. The string is what we send. - u.Last.Store(u.str(v)) - err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, u.str(v), attrs) - } + getterAttrs := u.applyGetterAttrs(elem, u.Setter) + attrs := append(elem.ApplyParams(params), getterAttrs...) + v := u.JawsGet(elem) + // Dedup on the rendered ISO8601 string, not the raw time.Time: comparing + // time.Time with == also compares the monotonic reading and *Location, so + // equal calendar dates can compare unequal. The string is what we send. + u.Last.Store(u.str(v)) + err = htmlio.WriteHTMLInput(w, elem.Jid(), htmlType, u.str(v), attrs) return } diff --git a/lib/ui/jsvar.go b/lib/ui/jsvar.go index 722c3938..114cfbd8 100644 --- a/lib/ui/jsvar.go +++ b/lib/ui/jsvar.go @@ -14,7 +14,6 @@ import ( "github.com/linkdata/jaws" "github.com/linkdata/jaws/lib/bind" "github.com/linkdata/jaws/lib/htmlio" - "github.com/linkdata/jaws/lib/tag" "github.com/linkdata/jaws/lib/what" "github.com/linkdata/jaws/lib/wire" "github.com/linkdata/jq" @@ -413,10 +412,10 @@ func (jsvar *JsVar[T]) JawsSet(elem *jaws.Element, value T) (err error) { // The serialized value is a render-time snapshot. See [JsVar] for the // synchronization semantics between rendering and the WebSocket subscription. // -// The bound value's [tag.TagGetter.JawsGetTag] and [jaws.InitHandler.JawsInit] -// callbacks run while the JsVar write lock is held, so they must not re-enter this -// JsVar (for example call JawsGet or JawsSet on it), which would self-deadlock the -// non-reentrant lock. +// The bound value's [github.com/linkdata/jaws/lib/tag.TagGetter] JawsGetTag callback +// runs while the JsVar write lock is held, so it must not re-enter this JsVar (for +// example call JawsGet or JawsSet on it), which would self-deadlock the non-reentrant +// lock. func (jsvar *JsVar[T]) JawsRender(elem *jaws.Element, w io.Writer, params []any) (err error) { // The render-time snapshot is taken under the write lock; see renderSnapshot. // Everything below runs without the lock: ApplyParams and, crucially, writing to @@ -445,15 +444,15 @@ func (jsvar *JsVar[T]) JawsRender(elem *jaws.Element, w io.Writer, params []any) // // It resolves the dirty tag and any initial HTML attrs from the bound value, // registers this JsVar as elem's handler, validates the JsVar name, and marshals a -// snapshot of Ptr. The lock spans [Element.ApplyGetter] and the marshal so the +// snapshot of Ptr. The lock spans [jaws.Element.ApplyGetter] and the marshal so the // serialized value stays consistent with the dirty tag even if another request // sharing this JsVar sets it concurrently. The deferred unlock ensures a panic from // a bound-value callback or from marshaling cannot leak the lock. // -// A nil Ptr has no bound value to inspect, so the getter and init callbacks are -// skipped and the initial data is omitted. Passing the typed-nil Ptr to ApplyGetter -// instead would call a value-receiver [tag.TagGetter] or [jaws.InitHandler] through -// the nil pointer and panic. +// A nil Ptr has no bound value to inspect, so the getter callback is skipped and the +// initial data is omitted. Passing the typed-nil Ptr to ApplyGetter instead would call +// a value-receiver [github.com/linkdata/jaws/lib/tag.TagGetter] through the nil +// pointer and panic. func (jsvar *JsVar[T]) renderSnapshot(elem *jaws.Element, params []any) (getterAttrs []template.HTMLAttr, jsvarName string, data []byte, err error) { jsvar.Lock() defer jsvar.Unlock() @@ -461,19 +460,27 @@ func (jsvar *JsVar[T]) renderSnapshot(elem *jaws.Element, params []any) (getterA if jsvar.Ptr != nil { getter = jsvar.Ptr } - if jsvar.dirtyTag, getterAttrs, err = elem.ApplyGetter(getter); err == nil { - elem.AddHandlers(jsvar) - if jsvarName, err = validateJsVarName(params); err == nil && jsvar.Ptr != nil { - data, err = json.Marshal(jsvar.Ptr) - } + jsvar.dirtyTag, getterAttrs = elem.ApplyGetter(getter) + elem.AddHandlers(jsvar) + if jsvarName, err = validateJsVarName(params); err == nil && jsvar.Ptr != nil { + data, err = json.Marshal(jsvar.Ptr) } return } -// JawsGetTag returns the current dirty tag. +// JawsGetTag returns the dirty tag, or nil before the first render initializes it. +// +// That nil is not a dirty target. After initialization JawsGetTag obeys the normal +// idempotent tag identity contract; see +// [github.com/linkdata/jaws/lib/tag.TagGetter]. +// +// A JsVar is therefore only usable as a tag once it has rendered. Passing one as a tag +// to another widget beforehand expands to no keys at all, so that widget registers +// under nothing and a later dirty of this JsVar never reaches it. Render the JsVar +// first, or tag the other widget with a value that does not depend on this one. // -// It is safe for concurrent use. The tag.Context argument is ignored and may be nil. -func (jsvar *JsVar[T]) JawsGetTag(tag.Context) any { +// It is safe for concurrent use. +func (jsvar *JsVar[T]) JawsGetTag() any { jsvar.RLock() defer jsvar.RUnlock() return jsvar.dirtyTag diff --git a/lib/ui/jsvar_lifecycle_test.go b/lib/ui/jsvar_lifecycle_test.go new file mode 100644 index 00000000..344c9ab6 --- /dev/null +++ b/lib/ui/jsvar_lifecycle_test.go @@ -0,0 +1,100 @@ +package ui + +import ( + "strings" + "sync" + "testing" +) + +// TestJsVar_JawsGetTagInitialization covers the one initialization phase the +// tag.TagGetter contract allows: JsVar reports nil until its first render resolves the +// dirty tag from the bound value, and reports that same tag from then on. The nil is +// not a dirty target, which TestJsVar_SetBeforeRenderDoesNotBroadcast covers. +func TestJsVar_JawsGetTagInitialization(t *testing.T) { + _, rq := newCoreRequest(t) + + var mu sync.Mutex + v := jsVarData{Text: "initial"} + jsv := NewJsVar(&mu, &v) + + if got := jsv.JawsGetTag(); got != nil { + t.Fatalf("JawsGetTag() before render = %#v, want nil", got) + } + // Repeated pre-render calls stay nil: the transition happens in JawsRender, not on + // first access. + if got := jsv.JawsGetTag(); got != nil { + t.Fatalf("second JawsGetTag() before render = %#v, want nil", got) + } + + elem := rq.NewElement(jsv) + var sb strings.Builder + if err := jsv.JawsRender(elem, &sb, []any{"lifecycle"}); err != nil { + t.Fatal(err) + } + + // The bound value is not a tag.TagGetter, so the resolved tag is Ptr itself. + first := jsv.JawsGetTag() + if first != any(&v) { + t.Fatalf("JawsGetTag() after render = %#v, want the bound pointer", first) + } + + // Past initialization the reported tag is stable, including across a set that + // broadcasts through it. + if err := jsv.JawsSetPath(elem, "text", "changed"); err != nil { + t.Fatal(err) + } + if got := jsv.JawsGetTag(); got != first { + t.Fatalf("JawsGetTag() after JawsSetPath = %#v, want %#v", got, first) + } + if got := jsv.JawsGetTag(); got != first { + t.Fatalf("repeated JawsGetTag() = %#v, want %#v", got, first) + } +} + +// TestJsVar_NotYetRenderedIsNotUsableAsTag pins the consequence of the nil phase that +// JsVar.JawsGetTag documents: expanding a JsVar before its first render yields no keys +// at all, so a widget tagged with it registers under nothing, and dirtying the JsVar +// once it has rendered still does not reach that widget. Rendering the JsVar first is +// what makes it a working tag. +// +// The test tags through jaws.Element.Tag, which expands unconditionally. A JsVar +// arriving via ParseParams or ApplyGetter is also accepted there — it satisfies their +// usable-as-tag check by being a tag.TagGetter — and then reaches the same expansion. +func TestJsVar_NotYetRenderedIsNotUsableAsTag(t *testing.T) { + _, rq := newCoreRequest(t) + + var mu sync.Mutex + v := jsVarData{Text: "initial"} + jsv := NewJsVar(&mu, &v) + + // Tag a second widget with the not-yet-rendered JsVar. + other := rq.NewElement(NewSpan(testHTMLGetter("x"))) + other.Tag(jsv) + if tags := rq.TagsOf(other); len(tags) != 0 { + t.Fatalf("a JsVar in its nil phase registered %v, want no keys", tags) + } + + // Rendering the JsVar resolves its dirty tag, but that does not retroactively + // register the widget that was tagged during the nil phase. + elem := rq.NewElement(jsv) + var sb strings.Builder + if err := jsv.JawsRender(elem, &sb, []any{"lifecycle"}); err != nil { + t.Fatal(err) + } + if tags := rq.TagsOf(other); len(tags) != 0 { + t.Fatalf("rendering the JsVar retroactively registered %v, want no keys", tags) + } + + // Tagging after the render is what works. + after := rq.NewElement(NewSpan(testHTMLGetter("y"))) + after.Tag(jsv) + if !after.HasTag(any(&v)) { + t.Fatalf("after render, tagging with the JsVar registered %v, want the bound pointer", rq.TagsOf(after)) + } + // HasTag looks up a single registered key and does not expand, so the JsVar that + // produced the registration is itself not a hit. This is the asymmetry + // jaws.Request.HasTag documents. + if after.HasTag(jsv) { + t.Fatal("HasTag matched an unexpanded tag.TagGetter, want only its expanded key") + } +} diff --git a/lib/ui/jsvar_test.go b/lib/ui/jsvar_test.go index e379f35c..67d3eb0e 100644 --- a/lib/ui/jsvar_test.go +++ b/lib/ui/jsvar_test.go @@ -103,7 +103,7 @@ func TestJsVar_RenderSetAndEvent(t *testing.T) { t.Fatalf("data-jawsdata = %#v, want %#v", gotData, v) } - if jsv.JawsGetTag(rq) == nil { + if jsv.JawsGetTag() == nil { t.Fatal("expected non-nil tag after render") } if gotV := jsv.JawsGet(nil); gotV.Text != v.Text || gotV.Num != 1 { @@ -143,7 +143,6 @@ func TestJsVar_RenderSetAndEvent(t *testing.T) { if err := jaws.CallEventHandlers(jsv, elem, what.Click, `1 2 0 x`); !errors.Is(err, jaws.ErrEventUnhandled) { t.Fatalf("expected ErrEventUnhandled, got %v", err) } - } // TestJsVar_SetBroadcastsWirePayload pins the wire payload broadcast when a @@ -521,7 +520,7 @@ type jsVarTagState struct { Value string `json:"value"` } -func (jsVarTagState) JawsGetTag(tag.Context) any { +func (jsVarTagState) JawsGetTag() any { return tag.Tag("state") } diff --git a/lib/ui/object.go b/lib/ui/object.go index 4486ee05..9da1ec27 100644 --- a/lib/ui/object.go +++ b/lib/ui/object.go @@ -122,11 +122,17 @@ func (obj *object) JawsInitialHTMLAttr(elem *jaws.Element) (attr template.HTMLAt return } -func (obj *object) JawsGetTag(ctx tag.Context) any { +// JawsGetTag collects the non-nil tags of every link in the chain. +// +// The returned container is freshly built per call, which [tag.TagGetter] permits: only +// the expanded key set has to stay the same. A chained getter that has its own +// documented nil initialization phase (see [JsVar.JawsGetTag]) propagates that phase +// here, so this object is likewise only usable as a tag once that link has initialized. +func (obj *object) JawsGetTag() any { var tags []any for obj != nil { if h, ok := obj.handler.(tag.TagGetter); ok { - if t := h.JawsGetTag(ctx); t != nil { + if t := h.JawsGetTag(); t != nil { tags = append(tags, t) } } diff --git a/lib/ui/object_test.go b/lib/ui/object_test.go index 9a169a5c..bd541f4c 100644 --- a/lib/ui/object_test.go +++ b/lib/ui/object_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/linkdata/jaws" - "github.com/linkdata/jaws/lib/tag" ) type testObjectStringer struct { @@ -22,7 +21,7 @@ type testObjectTagGetter struct { v any } -func (g testObjectTagGetter) JawsGetTag(tag.Context) any { +func (g testObjectTagGetter) JawsGetTag() any { return g.v } @@ -35,7 +34,7 @@ func TestObject_NewForwardsHTMLAndTag(t *testing.T) { if got, want := string(obj.JawsGetHTML(elem)), "<b>x</b>"; got != want { t.Fatalf("want %q got %q", want, got) } - if got, want := obj.JawsGetTag(rq), any(inner); got != want { + if got, want := obj.JawsGetTag(), any(inner); got != want { t.Fatalf("want tag %#v got %#v", want, got) } } @@ -198,7 +197,7 @@ func TestObject_GetTag_MultipleTagsReturnsSlice(t *testing.T) { }, } - got := obj.JawsGetTag(nil) + got := obj.JawsGetTag() tags, ok := got.([]any) if !ok { t.Fatalf("want []any got %T (%#v)", got, got) diff --git a/lib/ui/requestwriter_test.go b/lib/ui/requestwriter_test.go index c53aa536..f9147ea2 100644 --- a/lib/ui/requestwriter_test.go +++ b/lib/ui/requestwriter_test.go @@ -3,7 +3,6 @@ package ui import ( "bytes" "errors" - "html/template" "io" "log/slog" "strings" @@ -52,14 +51,6 @@ func (u *registerClickUpdater) JawsClick(elem *jaws.Element, click jaws.Click) e return nil } -type requestWriterFailGetter struct { - err error -} - -func (g requestWriterFailGetter) JawsGetHTML(elem *jaws.Element) template.HTML { return "x" } -func (g requestWriterFailGetter) JawsGetTag(tag.Context) any { return g } -func (g requestWriterFailGetter) JawsInit(elem *jaws.Element) error { return g.err } - func TestRequestWriter_MethodsAndWidgetHelpers(t *testing.T) { jw, rq := newCoreSessionBoundRequest(t) var buf bytes.Buffer @@ -169,7 +160,7 @@ func TestRequestWriterUI_RenderErrorDoesNotLeakElement(t *testing.T) { rw := RequestWriter{Request: rq, Writer: &buf} renderErr := errors.New("render failed") - if err := rw.NewUI(NewA(requestWriterFailGetter{err: renderErr})); !errors.Is(err, renderErr) { + if err := rw.NewUI(testRenderErrorUI{err: renderErr}); !errors.Is(err, renderErr) { t.Fatalf("want %v got %v", renderErr, err) } diff --git a/lib/ui/template.go b/lib/ui/template.go index 0e506ce8..a8769282 100644 --- a/lib/ui/template.go +++ b/lib/ui/template.go @@ -207,7 +207,7 @@ func (tmpl Template) render(elem *jaws.Element, w io.Writer, params []any) (err } doWrap := tmpl.OuterHTMLTag != "" var expandedTags []any - if expandedTags, err = tag.TagExpand(elem.Request, tmpl.Dot); err == nil { + if expandedTags, err = tag.TagExpand(tmpl.Dot); err == nil { elem.Request.TagExpanded(elem, expandedTags) tags, handlers, attrs := jaws.ParseParams(params) elem.Tag(tags...) diff --git a/lib/ui/textarea.go b/lib/ui/textarea.go index a79f0d80..47486cae 100644 --- a/lib/ui/textarea.go +++ b/lib/ui/textarea.go @@ -20,14 +20,12 @@ func NewTextarea(g bind.Setter[string]) *Textarea { return &Textarea{InputText{S // JawsRender renders ui as an HTML textarea. func (u *Textarea) JawsRender(elem *jaws.Element, w io.Writer, params []any) (err error) { - var getterAttrs []template.HTMLAttr - if getterAttrs, err = u.applyGetterAttrs(elem, u.Setter); err == nil { - attrs := append(elem.ApplyParams(params), getterAttrs...) - v := u.JawsGet(elem) - u.Last.Store(v) - v = template.HTMLEscapeString(v) - err = htmlio.WriteHTMLInner(w, elem.Jid(), "textarea", "", template.HTML(v), attrs...) // #nosec G203 - } + getterAttrs := u.applyGetterAttrs(elem, u.Setter) + attrs := append(elem.ApplyParams(params), getterAttrs...) + v := u.JawsGet(elem) + u.Last.Store(v) + v = template.HTMLEscapeString(v) + err = htmlio.WriteHTMLInner(w, elem.Jid(), "textarea", "", template.HTML(v), attrs...) // #nosec G203 return } diff --git a/parseparams.go b/parseparams.go index 1e5aa790..2df7d075 100644 --- a/parseparams.go +++ b/parseparams.go @@ -24,9 +24,9 @@ func usableAsTag(t any) (ok bool) { // // Unlike [Element.ApplyGetter], which is given the primary getter, ParseParams // only recognizes [InputFn], [InputHandler], [ClickHandler] and -// [ContextMenuHandler]. A param implementing [InitHandler] or -// [InitialHTMLAttrHandler] is treated only as a tag here; its JawsInit / -// JawsInitialHTMLAttr are intentionally invoked only for the primary getter. +// [ContextMenuHandler]. A param implementing [InitialHTMLAttrHandler] is treated +// only as a tag here; its JawsInitialHTMLAttr is intentionally invoked only for the +// primary getter. // // A param recognized as an event handler is appended to handlers, and if it is // also usable as a tag (comparable, per usableAsTag) it is additionally appended diff --git a/parseparams_fuzz_test.go b/parseparams_fuzz_test.go index 82ec5ecc..7709891c 100644 --- a/parseparams_fuzz_test.go +++ b/parseparams_fuzz_test.go @@ -91,24 +91,19 @@ type fuzzParseParamsTagGetter struct { ID byte } -func (h fuzzParseParamsTagGetter) JawsGetTag(tag.Context) any { +func (h fuzzParseParamsTagGetter) JawsGetTag() any { return tag.Tag(fmt.Sprintf("taggetter:%d", h.ID)) } type fuzzParseParamsNonComparableTagGetter []byte -func (h fuzzParseParamsNonComparableTagGetter) JawsGetTag(tag.Context) any { +func (h fuzzParseParamsNonComparableTagGetter) JawsGetTag() any { return tag.Tag(fmt.Sprintf("non-comparable-tagger:%x", []byte(h))) } -type fuzzParseParamsInitOnly struct { - ID byte -} - -func (h fuzzParseParamsInitOnly) JawsInit(*Element) error { - return nil -} - +// fuzzParseParamsInitialAttrOnly covers a param implementing an interface ParseParams +// does not recognize: JawsInitialHTMLAttr is only invoked for the primary getter, so +// ParseParams must classify this as a tag and nothing else. type fuzzParseParamsInitialAttrOnly struct { ID byte } @@ -120,8 +115,8 @@ func (h fuzzParseParamsInitialAttrOnly) JawsInitialHTMLAttr(*Element) template.H func FuzzParseParams(f *testing.F) { f.Add([]byte{}, "") f.Add([]byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, "a") - f.Add([]byte{11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}, `data-x="1"`) - f.Add([]byte{21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0}, "\x00\n\t") + f.Add([]byte{11, 12, 13, 14, 15, 16, 17, 18, 19, 20}, `data-x="1"`) + f.Add([]byte{20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0}, "\x00\n\t") f.Fuzz(func(t *testing.T, recipe []byte, text string) { if len(recipe) > 128 { @@ -153,7 +148,7 @@ func fuzzParseParamsBuild(recipe []byte, text string) (params []any, want fuzzPa for i, b := range recipe { token := fuzzParseParamsToken(i, b, text) data := fuzzParseParamsData(i, b, text) - switch b % 22 { + switch b % 21 { case 0: params = append(params, nil) want.tags = append(want.tags, "nil") @@ -227,19 +222,15 @@ func fuzzParseParamsBuild(recipe []byte, text string) (params []any, want fuzzPa case 17: params = append(params, []int{int(b), i}) case 18: - v := fuzzParseParamsInitOnly{ID: b} - params = append(params, v) - want.tags = append(want.tags, fuzzParseParamsLabel("initOnly", v.ID)) - case 19: v := fuzzParseParamsInitialAttrOnly{ID: b} params = append(params, v) want.tags = append(want.tags, fuzzParseParamsLabel("initialAttrOnly", v.ID)) - case 20: + case 19: var h *fuzzParseParamsPointerInputHandler params = append(params, h) want.handlers = append(want.handlers, "nilPointerInput") want.tags = append(want.tags, "nilPointerInput") - case 21: + case 20: h := fuzzParseParamsDualHandler{ID: b} params = append(params, h) want.handlers = append(want.handlers, fuzzParseParamsLabel("dual", h.ID)) @@ -305,8 +296,6 @@ func fuzzParseParamsClassifyTags(t *testing.T, tags []any) (labels []string) { labels = append(labels, fuzzParseParamsLabel("tagGetter", v.ID)) case fuzzParseParamsNonComparableTagGetter: labels = append(labels, fuzzParseParamsDataLabel("nonComparableTagGetter", []byte(v))) - case fuzzParseParamsInitOnly: - labels = append(labels, fuzzParseParamsLabel("initOnly", v.ID)) case fuzzParseParamsInitialAttrOnly: labels = append(labels, fuzzParseParamsLabel("initialAttrOnly", v.ID)) case *fuzzParseParamsPointerInputHandler: diff --git a/request.go b/request.go index 37aae4d2..a0e4f8fa 100644 --- a/request.go +++ b/request.go @@ -728,8 +728,13 @@ func (rq *Request) TagsOf(elem *Element) (tags []any) { } // Dirty marks all [Element] values that have one or more of the given tags as dirty. +// +// Dirtying is [Jaws]-wide rather than scoped to rq: matching Elements on every live +// [Request] are marked. The tags are expanded through [Jaws.MustTagExpand], so an +// expansion error is logged and the partial result still applied when a [Jaws.Logger] +// is configured, and panics before anything is marked dirty when one is not. func (rq *Request) Dirty(dirtyTags ...any) { - rq.Jaws.setDirty(tag.MustTagExpand(rq, dirtyTags)) + rq.Jaws.setDirty(rq.Jaws.MustTagExpand(dirtyTags)) } // wantMessage returns true if the Request want the message. @@ -834,6 +839,11 @@ func (rq *Request) hasTagLocked(elem *Element, tagValue any) bool { } // HasTag reports whether elem has tagValue in rq. +// +// tagValue is looked up as a single registered key and is not expanded, so passing a +// [tag.TagGetter] or a tag slice reports false even when its expanded keys are +// registered. Use [Request.GetElements], which expands, or [Request.TagsOf] to see the +// registered keys themselves. func (rq *Request) HasTag(elem *Element, tagValue any) (yes bool) { rq.mu.RLock() yes = rq.hasTagLocked(elem, tagValue) @@ -858,14 +868,24 @@ func (rq *Request) appendDirtyTags(tags []any) { // TagExpanded adds already-expanded tags to the given [Element]. // -// It is a no-op once rq has finished and released its buffers (rq.tagMap is nil), -// so a still-running initial renderer that keeps tagging after a racy teardown -// degrades to untracked elements instead of panicking on a nil-map write. +// TagExpanded is an advanced API that adds keys without expanding or validating them. +// Callers should normally use [Request.Tag]. expandedTags must contain only keys that +// [tag.TagExpand] can emit, either from a successful expansion or from a partial +// result whose error the caller handled. Other values may panic or create +// registrations that the canonical tag APIs cannot retrieve. +// +// It is a no-op for a nil, deleted, or foreign [Element], and after the [Request] has +// released its tag map. func (rq *Request) TagExpanded(elem *Element, expandedTags []any) { - if elem != nil && !elem.deleted.Load() && elem.Request == rq { + if elem != nil && elem.Request == rq { rq.mu.Lock() defer rq.mu.Unlock() - if rq.tagMap != nil { + // Deletion marks the Element and removes its tags while holding rq.mu. + // Check here so it cannot finish between the check and registration. + // A nil tagMap means rq finished and released its buffers, so a still-running + // initial renderer that keeps tagging after a racy teardown degrades to + // untracked elements instead of panicking on a nil-map write. + if !elem.deleted.Load() && rq.tagMap != nil { for _, tagValue := range expandedTags { if !rq.hasTagLocked(elem, tagValue) { rq.tagMap[tagValue] = append(rq.tagMap[tagValue], elem) @@ -876,15 +896,24 @@ func (rq *Request) TagExpanded(elem *Element, expandedTags []any) { } // Tag adds the given tags to the given [Element]. +// +// tagItems are expanded through [Jaws.MustTagExpand], so an expansion error is logged +// and the partial result still registered when a [Jaws.Logger] is configured, and +// panics before anything is registered when one is not. Use [Request.TagExpanded] to +// register keys you expanded yourself. func (rq *Request) Tag(elem *Element, tagItems ...any) { if elem != nil && len(tagItems) > 0 && elem.Request == rq { - rq.TagExpanded(elem, tag.MustTagExpand(elem.Request, tagItems)) + rq.TagExpanded(elem, rq.Jaws.MustTagExpand(tagItems)) } } // GetElements returns a list of the UI elements in the [Request] that have the given tags. +// +// tagValue is expanded through [Jaws.MustTagExpand], so an expansion error is logged +// and the partial result still used for the lookup when a [Jaws.Logger] is configured, +// and panics before the lookup when one is not. func (rq *Request) GetElements(tagValue any) (elems []*Element) { - expanded := tag.MustTagExpand(rq, tagValue) + expanded := rq.Jaws.MustTagExpand(tagValue) rq.mu.RLock() defer rq.mu.RUnlock() if len(expanded) == 1 { diff --git a/request_test.go b/request_test.go index 6477ecb9..6836e418 100644 --- a/request_test.go +++ b/request_test.go @@ -14,6 +14,7 @@ import ( "net/http/httptest" "net/url" "reflect" + "runtime" "strconv" "strings" "sync" @@ -106,6 +107,38 @@ func TestRequest_DeleteElementNil(t *testing.T) { rq.DeleteElement(nil) } +func TestRequest_TagExpandedDoesNotRetagConcurrentDeletion(t *testing.T) { + rq := &Request{tagMap: make(map[any][]*Element)} + elem := rq.NewElement(&testUi{}) + tagValue := tag.Tag("deleted") + done := make(chan struct{}) + + // A mutex wait is not durably blocked for testing/synctest, so use the + // RWMutex's writer preference to observe that TagExpanded is waiting for its + // write lock. Since this bare Request has no other goroutines, TryRLock + // failing proves TagExpanded passed its pre-lock checks and queued as a writer. + rq.mu.RLock() + go func() { + rq.TagExpanded(elem, []any{tagValue}) + close(done) + }() + for rq.mu.TryRLock() { + rq.mu.RUnlock() + runtime.Gosched() + } + + // Deletion stores this flag while holding rq.mu. Store it directly here to + // model that state transition between TagExpanded's check and registration; + // the empty tag map means deletion's other cleanup has no bearing on the test. + elem.deleted.Store(true) + rq.mu.RUnlock() + <-done + + if tags := rq.TagsOf(elem); len(tags) != 0 { + t.Fatalf("deleted Element retained tags: %v", tags) + } +} + func TestRequest_DeleteElements(t *testing.T) { rq := newTestRequest(t) defer rq.Close() @@ -1753,6 +1786,37 @@ func TestRequest_Log(t *testing.T) { } } +// TestRequest_MustLog covers the nil-receiver forwarding that exists purely for +// diagnostics, plus the two Logger states. Tag expansion logs through +// Jaws.MustTagExpand, so Request.MustLog needs direct coverage here. +func TestRequest_MustLog(t *testing.T) { + wantErr := errors.New("request mustlog test") + + var log bytes.Buffer + rq := &Request{ + Jaws: &Jaws{ + Logger: slog.New(slog.NewTextHandler(&log, nil)), + }, + } + rq.MustLog(wantErr) + if s := log.String(); !strings.Contains(s, wantErr.Error()) { + t.Fatalf("Request.MustLog() did not write error to logger: %q", s) + } + + // A nil error is a no-op even without a Logger. + (*Request)(nil).MustLog(nil) + + // Without a Logger it panics, including through a nil *Request. + func() { + defer func() { + if recover() == nil { + t.Error("Request.MustLog with no Logger must panic") + } + }() + (&Request{Jaws: &Jaws{}}).MustLog(wantErr) + }() +} + func TestRequest_Dirty(t *testing.T) { synctest.Test(t, func(t *testing.T) { th := newTestHelper(t) diff --git a/tag_render_production_test.go b/tag_render_production_test.go index 96947cae..2eb58590 100644 --- a/tag_render_production_test.go +++ b/tag_render_production_test.go @@ -15,27 +15,29 @@ type productionTagRenderUI struct { elem *Element } -func (ui *productionTagRenderUI) JawsRender(elem *Element, _ io.Writer, _ []any) (err error) { +func (ui *productionTagRenderUI) JawsRender(elem *Element, _ io.Writer, _ []any) error { ui.elem = elem - _, _, err = elem.ApplyGetter(ui.getter) - return + // An unusable tag reaches Jaws.Logger through Jaws.MustTagExpand, which is what + // these tests assert. + elem.ApplyGetter(ui.getter) + return nil } func (*productionTagRenderUI) JawsUpdate(*Element) {} type productionNamedFloatTag float64 -type productionFunctionTagGetter func(tag.Context) any +type productionFunctionTagGetter func() any -func (fn productionFunctionTagGetter) JawsGetTag(ctx tag.Context) any { - return fn(ctx) +func (fn productionFunctionTagGetter) JawsGetTag() any { + return fn() } type productionTagGetterWrapper struct { value any } -func (wrapper productionTagGetterWrapper) JawsGetTag(tag.Context) any { +func (wrapper productionTagGetterWrapper) JawsGetTag() any { return wrapper.value } @@ -63,7 +65,7 @@ func TestUIRenderResolvesDistinctFunctionTagGetters(t *testing.T) { getters := make([]productionFunctionTagGetter, len(next)) for i := range next { i := i - getters[i] = func(tag.Context) any { return next[i] } + getters[i] = func() any { return next[i] } } leafGetter := getters[0] rootGetter := getters[1] diff --git a/tagexpand.go b/tagexpand.go new file mode 100644 index 00000000..aefb54a0 --- /dev/null +++ b/tagexpand.go @@ -0,0 +1,14 @@ +package jaws + +import "github.com/linkdata/jaws/lib/tag" + +// MustTagExpand expands tagValue and reports expansion errors through [Jaws.MustLog]. +// +// With a [Jaws.Logger] configured, the error is logged and MustTagExpand returns +// [github.com/linkdata/jaws/lib/tag.TagExpand]'s partial result. Without one, +// [Jaws.MustLog] panics, so the partial result never reaches the caller. +func (jw *Jaws) MustTagExpand(tagValue any) (result []any) { + result, err := tag.TagExpand(tagValue) + jw.MustLog(err) + return +} diff --git a/tagexpand_test.go b/tagexpand_test.go new file mode 100644 index 00000000..27fb42bd --- /dev/null +++ b/tagexpand_test.go @@ -0,0 +1,71 @@ +package jaws + +import ( + "errors" + "reflect" + "testing" + + "github.com/linkdata/jaws/lib/tag" +) + +func TestJaws_MustTagExpand(t *testing.T) { + jw, err := New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + logger := &captureErrorLogger{} + jw.Logger = logger + + if got := jw.MustTagExpand([]any{tag.Tag("a"), tag.Tag("b")}); !reflect.DeepEqual(got, []any{tag.Tag("a"), tag.Tag("b")}) { + t.Fatalf("MustTagExpand = %#v, want both tags", got) + } + if logger.err != nil { + t.Fatalf("successful expansion logged %v, want nothing", logger.err) + } +} + +// TestJaws_MustTagExpandLogsAndReturnsPartial locks in the documented behavior with a +// Logger configured: the error is reported and the tags expanded before the failure are +// still returned, so the caller applies a partial result rather than nothing. +func TestJaws_MustTagExpandLogsAndReturnsPartial(t *testing.T) { + jw, err := New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + logger := &captureErrorLogger{} + jw.Logger = logger + + // "bad" is a plain string, an illegal tag type, so expansion fails after + // tag.Tag("ok") has already been collected. + got := jw.MustTagExpand([]any{tag.Tag("ok"), "bad"}) + if !errors.Is(logger.err, tag.ErrIllegalTagType) { + t.Fatalf("logged error = %v, want %v", logger.err, tag.ErrIllegalTagType) + } + if !reflect.DeepEqual(got, []any{tag.Tag("ok")}) { + t.Fatalf("MustTagExpand = %#v, want the partial result [tag.Tag(\"ok\")]", got) + } +} + +// TestJaws_MustTagExpandPanicsWithoutLogger is the other half of the contract: with no +// Logger, Jaws.MustLog panics, so the partial result never reaches the caller. +func TestJaws_MustTagExpandPanicsWithoutLogger(t *testing.T) { + jw, err := New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + if jw.Logger != nil { + t.Fatal("expected nil Logger by default") + } + + defer func() { + x := recover() + if e, ok := x.(error); !ok || !errors.Is(e, tag.ErrIllegalTagType) { + t.Fatalf("recovered %#v, want an error matching %v", x, tag.ErrIllegalTagType) + } + }() + jw.MustTagExpand([]any{tag.Tag("ok"), "bad"}) + t.Fatal("MustTagExpand returned instead of panicking") +} diff --git a/testhelpers_test.go b/testhelpers_test.go index 0679f750..08f56052 100644 --- a/testhelpers_test.go +++ b/testhelpers_test.go @@ -221,7 +221,7 @@ func writeTestTemplateWrapperStart(elem *Element, w io.Writer, outerHTMLTag stri func (t testTemplateUI) JawsRender(elem *Element, w io.Writer, params []any) (err error) { doWrap := t.OuterHTMLTag != "" var expandedTags []any - if expandedTags, err = tag.TagExpand(elem.Request, t.Dot); err == nil { + if expandedTags, err = tag.TagExpand(t.Dot); err == nil { elem.Request.TagExpanded(elem, expandedTags) tags, handlers, attrs := ParseParams(params) elem.Tag(tags...) @@ -315,12 +315,11 @@ func newTestTextInputWidget(s testStringSetter) *testTextInputWidget { func (u *testTextInputWidget) JawsRender(elem *Element, w io.Writer, params []any) (err error) { var getterAttrs []template.HTMLAttr - if u.tagValue, getterAttrs, err = elem.ApplyGetter(u.setter); err == nil { - attrs := append(elem.ApplyParams(params), getterAttrs...) - v := u.setter.JawsGet(elem) - u.last = v - err = htmlio.WriteHTMLInput(w, elem.Jid(), "text", v, attrs) - } + u.tagValue, getterAttrs = elem.ApplyGetter(u.setter) + attrs := append(elem.ApplyParams(params), getterAttrs...) + v := u.setter.JawsGet(elem) + u.last = v + err = htmlio.WriteHTMLInput(w, elem.Jid(), "text", v, attrs) return }