فهرست منبع

Neo: harden the shared object state (#5839)

From the PR review:
- A late response older than the state (after a newer socket event) still
  applies the viewer's own fields but keeps the newer counter and ts
  (mergeState, reports).
- Props with ts seed the store when it has no entry, so a socket update on a
  cached copy merges onto the newest copy's fields, not its own old ones.
  Seeding re-runs when the entry changes (clear, eviction).
- A clear (logout, profile switch) bumps a generation; object action
  responses and post reloads started before it don't write.
- The LRU limit evicts only entries no mounted widget reads.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
RomanL 1 هفته پیش
والد
کامیت
ca34041065

+ 5 - 3
neo/packages/app/components/elements/feed-item-parts.js

@@ -31,7 +31,7 @@ import AnimatedBlock from 'app/ui/atoms/animated-block'
 import { CommentsBrowseShort } from 'app/components/elements/comments-browse';
 import { Pressable } from 'app/design/view'
 import { subscribeTimelineEdit } from 'app/lib/timeline-edits'
-import { commentsObjectKey, storeObjectsFrom, useObjectState } from 'app/lib/object-state'
+import { commentsObjectKey, objectStateGeneration, storeObjectsFrom, useObjectState } from 'app/lib/object-state'
 import { getDataForMenu } from 'app/lib/util'
 import Form from 'app/components/elements/form'
 import useFetchForm from 'app/lib/hooks/use-fetch-form'
@@ -702,6 +702,7 @@ export const UnitFeed = ({ data, mode, DefaultUnit, SmallUnit, feed_type }) => {
     useEffect(() => {
         if (itemId == null) return undefined
         return subscribeTimelineEdit(itemId, async () => {
+            const generation = objectStateGeneration()
             const result = await fetcher(
                 '/api.php?r=' +
                 appSetting('urls', 'feed_item') +
@@ -710,8 +711,9 @@ export const UnitFeed = ({ data, mode, DefaultUnit, SmallUnit, feed_type }) => {
                 '}}'
             )
             if (result.data) {
-                // Counters, reactions and comments of the post for every widget.
-                storeObjectsFrom(result.data)
+                // Counters, reactions and comments of the post for every widget,
+                // unless the viewer logged out or switched profiles meanwhile.
+                if (generation === objectStateGeneration()) storeObjectsFrom(result.data)
                 setDatas(enrichFeedItem(result.data))
             }
         })

+ 5 - 3
neo/packages/app/components/elements/feed-item.js

@@ -9,7 +9,7 @@ import { UnitImages } from 'app/lib/feed-items'
 import { PollItem } from 'app/components/elements/entity-poll';
 import { BlockWrapper } from 'app/components/block-wrapper'
 import { fetcher } from 'app/lib/fetcher'
-import { storeObjectsFrom } from 'app/lib/object-state'
+import { objectStateGeneration, storeObjectsFrom } from 'app/lib/object-state'
 
 export default function ElementFeedItem({ data, isModal, blockWrapperProps }) {
     const eventId = data?.event?.id
@@ -20,6 +20,7 @@ export default function ElementFeedItem({ data, isModal, blockWrapperProps }) {
         if (eventId == null) return undefined
 
         return subscribeTimelineEdit(eventId, async () => {
+            const generation = objectStateGeneration()
             const result = await fetcher(
                 '/api.php?r=' +
                 appSetting('urls', 'feed_item') +
@@ -27,8 +28,9 @@ export default function ElementFeedItem({ data, isModal, blockWrapperProps }) {
                 eventId +
                 '}}'
             )
-            // Counters and reactions of the post for every widget.
-            if (result.data) storeObjectsFrom(result.data)
+            // Counters and reactions of the post for every widget, unless the
+            // viewer logged out or switched profiles meanwhile.
+            if (result.data && generation === objectStateGeneration()) storeObjectsFrom(result.data)
             if (result.data?.content) setContent(result.data.content)
         })
     }, [eventId])

+ 30 - 8
neo/packages/app/lib/object-state.ts

@@ -58,10 +58,14 @@ function put(key: string, entry: Entry) {
     // Re-insert so Map order is least recently written first.
     entries.delete(key);
     entries.set(key, entry);
-    while (entries.size > MAX_ENTRIES) {
-        const oldest = entries.keys().next().value as string;
-        entries.delete(oldest);
-        notify(oldest);
+    // Evict the oldest entries no mounted widget reads: an evicted entry would
+    // send a mounted widget back to its (older) props. The limit gives way
+    // when every entry is on screen.
+    if (entries.size > MAX_ENTRIES) {
+        for (const oldest of [...entries.keys()]) {
+            if (entries.size <= MAX_ENTRIES) break;
+            if (oldest !== key && !listeners.has(oldest)) entries.delete(oldest);
+        }
     }
     notify(key);
 }
@@ -128,8 +132,20 @@ export function storeObjectsFrom(data: unknown) {
     }
 }
 
+let generation = 0;
+
+/**
+ * Changes on every clear. A request started before a logout or a profile
+ * switch compares it before writing, so the previous viewer's state can't
+ * come back.
+ */
+export function objectStateGeneration() {
+    return generation;
+}
+
 /** Drop everything (logout, profile switch). */
 export function clearObjectState() {
+    generation++;
     const keys = [...entries.keys()];
     entries.clear();
     keys.forEach(notify);
@@ -164,11 +180,17 @@ export function useObjectState<T>(key: string | null | undefined, props: any, in
     else if (current?.state !== undefined) value = current.state as T;
     else value = init(current?.props ?? props);
 
-    // Props newer than the shared state (a reloaded list): share them, so older
-    // copies of the object catch up too.
+    // Share props that are the newest data for the object: the first copy on
+    // screen seeds the entry, a reloaded list replaces an older one. Older copies
+    // then show them too, and a socket update merges onto them rather than onto
+    // an older copy's fields. Props without `ts` (an older UNA) never seed:
+    // nothing would let a fresher copy replace them. Runs again when the entry
+    // changes, so a cleared or evicted entry is seeded by the copies on screen.
     useEffect(() => {
-        if (shared && propsNewer) put(key as string, { props, ts: propsTs });
-    }, [shared, key, propsNewer, props, propsTs]);
+        if (!shared || propsTs === undefined) return;
+        const stored = entries.get(key as string);
+        if (!stored || isNewer(propsTs, stored)) put(key as string, { props, ts: propsTs });
+    }, [shared, key, props, propsTs, entry]);
 
     const setValue = useCallback<SetState<T>>(
         (next) => {

+ 19 - 3
neo/packages/app/ui/molecules/objects/helpers.js

@@ -7,7 +7,7 @@ import { useEffect } from 'react';
 import { fetcher } from 'app/lib/fetcher';
 import { FeedbackHaptics } from 'app/lib/util';
 import { subscribe } from 'app/ui/atoms/socket';
-import { socialObjectKey } from 'app/lib/object-state';
+import { objectStateGeneration, objectTs, socialObjectKey } from 'app/lib/object-state';
 import Profile from 'app/ui/molecules/profile/profile';
 import { View, Row } from 'app/design/view';
 import {
@@ -35,8 +35,21 @@ export function objectStateFromProps(props) {
     return { ...props.action, counter: props.counter, ts: props.ts };
 }
 
+/**
+ * Merge a response or a socket update into the state. An update older than the
+ * state (a late response after a newer socket event) still brings the viewer's
+ * own fields, but keeps the newer `counter` and `ts`.
+ */
 export function mergeState(prev, next) {
-    return !prev ? next : { ...prev, ...next };
+    if (!prev) return next;
+    if (!next) return prev;
+    const prevTs = objectTs(prev);
+    const nextTs = objectTs(next);
+    if (prevTs !== undefined && nextTs !== undefined && nextTs < prevTs) {
+        const { counter, ts, ...own } = next;
+        return { ...prev, ...own };
+    }
+    return { ...prev, ...next };
 }
 
 // ---------------------------------------------------------------------------
@@ -46,7 +59,8 @@ export function mergeState(prev, next) {
 /**
  * UNA object request: /api.php?r=system/{action}/{service}&params[]={s,o,...}
  * @param {{ service: string, system: string, objectId: string|number }} object_params
- * @returns full fetcher response; onLoad gets response.data when provided
+ * @returns full fetcher response; onLoad gets response.data when provided.
+ * A response that arrives after a logout or a profile switch is dropped (null).
  */
 export async function objectRequest(
     { service, system, objectId },
@@ -62,7 +76,9 @@ export async function objectRequest(
         service +
         '&params[]=' +
         JSON.stringify(body);
+    const generation = objectStateGeneration();
     const response = await fetcher(url);
+    if (generation !== objectStateGeneration()) return null;
     if (typeof onLoad === 'function') onLoad(response?.data);
     return response;
 }

+ 2 - 1
neo/packages/app/ui/molecules/objects/reactions.tsx

@@ -149,7 +149,8 @@ type ReactionsObjectState = {
 const reactionsStateFromProps = (props: ReactionsProps): ReactionsObjectState => ({
     action: props.action,
     counter: props.counter,
-    live: {},
+    // `ts` lets mergeState keep a newer counter from a late response.
+    live: props.ts !== undefined ? { ts: props.ts } : {},
     ts: props.ts,
 });
 

+ 17 - 13
neo/packages/app/ui/molecules/objects/reports.js

@@ -216,19 +216,23 @@ export default function ElementReports(props) {
         (payload) => {
             if (!payload) return;
 
-            setReportState((prev) => ({
-                action: {
-                    ...prev.action,
-                    is_reported: payload?.is_reported ?? prev.action.is_reported,
-                    is_disabled: payload?.is_disabled ?? prev.action.is_disabled,
-                    title: payload?.title ?? prev.action.title,
-                },
-                counter:
-                    payload?.counter?.count !== undefined
-                        ? { count: Number(payload.counter.count || 0) }
-                        : prev.counter,
-                ts: payload?.ts ?? prev.ts,
-            }));
+            setReportState((prev) => {
+                // A late response keeps the newer counter (see mergeState).
+                const older = payload?.ts !== undefined && prev.ts !== undefined && payload.ts < prev.ts;
+                return {
+                    action: {
+                        ...prev.action,
+                        is_reported: payload?.is_reported ?? prev.action.is_reported,
+                        is_disabled: payload?.is_disabled ?? prev.action.is_disabled,
+                        title: payload?.title ?? prev.action.title,
+                    },
+                    counter:
+                        payload?.counter?.count !== undefined && !older
+                            ? { count: Number(payload.counter.count || 0) }
+                            : prev.counter,
+                    ts: older ? prev.ts : (payload?.ts ?? prev.ts),
+                };
+            });
 
             if (typeof props?.onChangeTitle === 'function' && payload?.title) {
                 props.onChangeTitle(payload.title);