diff --git a/docs/SpaceControllerRefactor.md b/docs/SpaceControllerRefactor.md new file mode 100644 index 0000000000..416bd9c4aa --- /dev/null +++ b/docs/SpaceControllerRefactor.md @@ -0,0 +1,299 @@ +# SpaceController machinery: how it works today and how to refactor it + +Research notes, June 2026. Scope: `space/` service layer — `space.Service`, the four +`SpaceController` implementations, the `mode.StateMachine`, the space processes +(loader / joiner / offloader), and the lazy-loading bolt-on. + +--- + +## 1. How it works today + +### 1.1 The layers + +``` +space.Service (space/service.go) + ├── techspace (space views = persistent per-space state, synced as objects) + ├── spaceWatcher (space/spacewatcher.go) + │ objectstore subscription on spaceView objects in tech space + │ → dedupqueue → onSpaceStatusUpdated (goroutine per event) + ├── spaceControllers map[string]SpaceController (registry) + ├── waiting map[string]controllerWaiter (hand-rolled singleflight + error cache) + └── lazy-loading bolt-on: deferredStatuses, lazyMode, releasing, + preloadOnce/preloadCh, ensureSpaceStarted, resolveDerivedInfo + +SpaceController (4 implementations) + ├── personalspace.spaceController Initial / Loading(+migration) / Offloading + ├── shareablespace.spaceController Initial / Loading / Joining / Offloading + ├── streamablespace.spaceController Initial / Loading(guestKey) / Offloading + └── marketplacespace.spaceController fake: no state machine, Mode()==ModeLoading always + each real controller owns: + ├── child app.App with spacestatus (facade over its spaceView) + └── mode.StateMachine (one goroutine per space) + └── current mode.Process = ANOTHER child app.App per mode: + loader → builder, spaceloader, aclnotifications, + aclobjectmanager, participantwatcher, migration + joiner → statuschanger, aclnotifications, aclwaiter + offloader→ spaceoffloader +``` + +### 1.2 The data flow (a feedback loop through the object store) + +1. Persistent intent lives in the **spaceView** object in tech space + (`AccountStatus`: Unknown/Active/Joining/Removing/Deleted, plus aclHeadId, guestKey). +2. `spaceWatcher` subscribes to all spaceViews (`space/spacesub.go`) and feeds every + change through a dedup queue into `service.onSpaceStatusUpdated`. +3. The service creates the controller if needed (`startStatus`, dispatch by + id/EncodedKey → factory) and calls `ctrl.Update()`. +4. `Update()` maps AccountStatus → Mode (`Deleted/Removing→Offloading`, + `Joining→Joining`, else `Loading`) and calls `sm.ChangeMode(mode)`. +5. The state machine tears down the current mode's child app and starts the next one. +6. The processes write status *back* into the spaceView (loader sets + LocalStatus Loading→Ok/Missing; joiner's aclwaiter flips AccountStatus + Joining→Active/Deleted on ACL events) — which re-triggers the subscription (2). + +RPC intents (`Join`, `InviteJoin`, `Delete`, `CancelLeave`) bypass part of the loop: +they create controllers directly *and* write the spaceView, guarded by the `waiting` +map. `join.go` carries the TODO: *"refactor using unidirectional model where we +change/create space view and it asynchronously starts controller"*. + +### 1.3 The state machine (`space/internal/spaceprocess/mode/statemachine.go`) + +- One goroutine (`loop`) per space; `ChangeMode(next)` registers an unbuffered waiter + channel, pokes `notify`, and **blocks** until the loop has closed the old process and + started the new one. +- Only one pending transition is allowed: a second, different `ChangeMode` while one is + in flight returns `ErrTransitionInProcess`. +- On process-start failure, the machine falls back to `ModeInitial` and sends `nil` to + all waiters, which surfaces as a generic `ErrFailedToStart` (the real error is lost — + see `// TODO: [MR] send error to waiter`). + +--- + +## 2. Why it is error-prone (concrete defects, with locations) + +These are not stylistic complaints; each is a latent or actual bug. + +1. **Dropped transitions.** Every controller's `Update()` sets `lastUpdatedStatus = status` + *before* calling `ChangeMode`. If `ChangeMode` returns `ErrTransitionInProcess` + (status flipped while a previous transition runs, e.g. RPC `SetPersistentInfo` + racing the watcher), the error is logged and dropped (`applySpaceStatus`, + service.go), and the *next identical event is a no-op* because `lastUpdatedStatus` + already matches. The space silently stays in the wrong mode until the status changes + again. (shareable.go:122-145, personal.go:155-174, streamablespace.go:122-141) + +2. **`ChangeMode` cannot be cancelled and waiters are never drained on Close.** + `proc = <-wait` has no ctx/timeout (statemachine.go:126). If `Close()` wins the + select race in `loop`, pending waiters never receive — those goroutines block + forever. A hung `Process.Start` (network) blocks every caller indefinitely. + +3. **Swallowed errors.** Waiters get `nil` → `ErrFailedToStart`; the actual start error + only goes to the log. Callers (Join RPC, Get) cannot distinguish "storage missing" + from "invalid ACL". + +4. **Session-permanent error poisoning.** `s.waiting` entries are never deleted; a + failed factory call caches its error forever (load.go:31-47, join.go, streamable.go). + A transient failure during `Join` makes that space id unusable until app restart. + The lazy-loading code knows this ("would poison s.waiting" — service.go:580). + +5. **`Wait()` is a 500 ms polling loop** (waiter.go:58-67) because there is no event for + "controller appeared in the registry". + +6. **`Current() any` + scattered type assertions.** `load.go:103`, + `create.go:86,129` assert `ctrl.Current().(loader.LoadWaiter)`; marketplace must fake + the whole interface; `join.go` branches on raw `Mode()` values. Mode is overloaded: + `ModeLoading` means both "loading" and "loaded" (`AllLoadedSpaceIds`, service.go:786). + +7. **Massive copy-paste.** personal/shareable/streamable controllers are ~80% identical: + `makeStatusApp`, `Update`, `SetPersistentInfo`, `SetLocalInfo`, `Close`, + `GetStatus`, `GetLocalStatus`, `Delete`, and the AccountStatus→Mode switch is + duplicated **six times** (Start + Update × three controllers). Divergence has already + crept in: shareable handles `AccountStatusJoining`, the others don't; only personal + wires migrations. + +8. **Lazy loading is a bolt-on, not a property of the design.** Because *creating* a + controller immediately *loads* the space (controller Start → ChangeMode(Loading) → + loader app starts building), laziness had to be implemented as "don't create the + controller": `deferredStatuses` backlog + `releasing` + `preloadOnce` + dynamic + fallback + `ensureSpaceStarted` + `resolveDerivedInfo` + two test-seam hooks, with + invariants (B1/B2/B3/E2) enforced by comments and careful lock choreography in + service.go (~150 lines). + +9. **Obscure concurrency idioms.** `spaceViewStatus` carries `mx *sync.Mutex` shared + across value copies to serialize handlers per view (spacesub.go:29); + `onSpaceStatusUpdated` spawns a goroutine per event; loader and offloader each + hand-roll the same retry loop (`loadingSpace.loadRetry` / `offloadingSpace`). + +10. **Construction does I/O.** `personalspace.NewSpaceController` checks/creates the + space view; factory `Create*` methods mix storage mutation, view creation and + controller construction — so "register a controller" can block on the network. + +History confirms the cost: GO-5948 ("Refactor space service to use subscriptions"), +GO-6108 ("Fix WaitSpace for tech space"), GO-5935 ("Offload space when user is kicked"), +GO-7292 (lazy loading, 3 commits of lock-choreography fixes), plus the standing TODOs. + +--- + +## 3. Proposed refactoring: one controller, a reconciler instead of a state machine + +### 3.1 Key insight + +This is a **reconciliation** problem, not an RPC-style state machine problem. +Desired state is already fully described by `(AccountStatus, demand)`; actual state is +"which process app is running". The current design pushes transitions imperatively and +blocks callers on them; everything painful above follows from that. Invert it: + +> A per-space goroutine owns the actual state and continuously converges it to the +> latest desired state. Callers never drive transitions; they update inputs and/or wait +> on outcomes. + +This is the same TODO already written in `join.go` ("unidirectional model"), applied +consistently. + +### 3.2 The pieces + +**One controller for all space kinds.** Replace personal/shareable/streamable with a +single implementation parameterized by a descriptor; kind differences become data: + +```go +type Descriptor struct { + SpaceId string + IsPersonal bool // loader flag + migration component + GuestKey crypto.PrivKey // streamable + Metadata []byte + ExtraLoaderComponents func() []app.Component // personalmigration + JoinSupported bool // shareable only +} +``` + +Marketplace leaves the registry entirely (it is deprecated, GO-6259); `service.Get` +special-cases it exactly like it already special-cases tech space. + +**Explicit states, not overloaded modes:** + +```go +type State int +const ( + StateDormant State = iota // registered, nothing running ← lazy by design + StateLoading + StateLoaded + StateJoining + StateOffloading + StateOffloaded // terminal + StateFailed // holds the error; retryable +) +``` + +**The reconciler loop** (replaces `mode.StateMachine`, still one goroutine per space): + +```go +// inputs, written atomically by anyone, latest-wins: +// accountStatus (from watcher / SetPersistentInfo) +// demand (false = dormant; true = should be loaded) +func (c *controller) run() { + for { + select { + case <-c.wake: // buffered(1); tick on any input change + case <-c.ctx.Done(): + c.teardown(); return + } + target := computeTarget(c.inputs()) // pure function, ONE copy: + // Deleted|Removing → Offloaded ; Joining → Joining(then auto-Active) + // else: demand ? Loaded : Dormant + if target != c.state { + c.transitionTo(target) // close old process app, start new one + } + } +} +``` + +Properties, each fixing a defect from §2: + +- *Latest-wins coalescing*: no transition queue, no `ErrTransitionInProcess`, no + dropped updates (§2.1). A status flip mid-transition just re-ticks `wake`; the loop + re-reads inputs after every transition. +- *Nobody blocks on transitions*: `Update`/`SetPersistentInfo` only store inputs + + tick. Waiting moves to outcome futures: `WaitLoad(ctx)` sets `demand=true` and waits + on a promise the reconciler resolves on entering `StateLoaded` (with the real error + on `StateFailed`) — cancellable by caller ctx (§2.2, §2.3, §2.5). +- *Failures are states, not poison*: a failed load → `StateFailed{err}` with + backoff-retry inside the reconciler; the next demand or status change retries. + The `waiting` error cache dies (§2.4). +- *Lazy by design*: the registry creates a (cheap, no-I/O) Dormant controller for + **every** space view at startup. Loading starts only when something sets demand: + `Get/Wait` (user opened the space), preload release, eager mode (demand=true at + registration), preferred-space (demand only that one). The entire bolt-on — + `deferredStatuses`, `releasing`, `preloadOnce`, `ensureSpaceStarted`, + `resolveDerivedInfo`, `lazyMode` branches — reduces to a per-controller boolean + (§2.8). Offload/join ignore demand because `computeTarget` ranks status first. + +**Processes stay.** The child-app-per-mode pattern (loader/joiner/offloader bundles) +is the *good* part of the current design — keep it, but give processes one uniform +shape and pull the duplicated retry loops up into a shared helper in the reconciler. + +**Unidirectional intents.** `Join`/`InviteJoin`/`Delete`/`CancelLeave` only write the +spaceView (create it if needed) and then wait on the controller future. Controller +creation happens in exactly two places: the watcher (view appeared) and +`registry.GetOrCreate` (demand for an existing view). `Wait()`'s polling loop becomes +"wait for view to exist (subscription event), then `GetOrCreate(id).WaitLoad(ctx)`". + +**Typed interface.** `Current() any` disappears: + +```go +type SpaceController interface { + SpaceId() string + State() State + SetStatusInfo(spaceinfo.SpacePersistentInfo) // input write + wake + Demand() // input write + wake + WaitLoad(ctx) (clientspace.Space, error) // demand + future + WaitOffload(ctx) error + Close(ctx) error +} +``` + +### 3.3 What gets deleted + +| Today | After | +|---|---| +| 3 near-identical controller packages (~600 LOC) | 1 controller (~250 LOC) | +| `mode.StateMachine` + waiters + notify dance | ~60-line reconciler loop | +| `waiting` map + error caching + close/wait choreography in 5 files | controller futures | +| 6 copies of the AccountStatus→Mode switch | 1 pure `computeTarget` | +| lazy bolt-on (~150 LOC + 2 test hooks + B1/B2/B3/E2 comments) | `demand` flag | +| `Wait` 500 ms polling | event-driven wait | +| `Current().(loader.LoadWaiter)` assertions | `WaitLoad` on the interface | +| marketplace fake controller | service-level special case (then delete with GO-6259) | + +### 3.4 Incremental migration (each phase ships independently) + +1. **Dedup controllers** (mechanical, low risk): merge personal/shareable/streamable + into one implementation + `Descriptor`, keeping `mode.StateMachine` and the existing + `SpaceController` interface byte-for-byte. Kills §2.7 and halves the surface for the + next phases. Existing tests keep passing. +2. **Swap the state machine internals for the reconciler**, preserving the public + interface (`Mode()` derived from `State` for `join.go`/`AllLoadedSpaceIds` compat). + Fixes dropped transitions, blocked waiters, swallowed errors. +3. **Introduce `StateDormant` + `Demand()`** and move lazy-mode logic out of + service.go; registry creates Dormant controllers for all views; preferred/preload + becomes demand wiring. +4. **Unidirectional intents**: rewrite `Join`/`InviteJoin`/`Delete`/`AddStreamable` to + write-view-then-wait; delete the `waiting` map and the polling waiter. +5. **Cleanups**: typed interface (`Current()` removal), retire marketplace controller + (GO-6259), unify loader/offloader retry helpers. + +### 3.5 Risks / open questions + +- `clientspace.Space` consumers cache the pointer returned by `Get`; with + Dormant→Loaded→Dormant cycles (future unload support) those references go stale. + V1: no automatic unloading — Dormant is only an *initial* state, same semantics as + today's deferral. Unloading-on-idle becomes possible later precisely because the + reconciler makes Loaded→Dormant a legal transition. +- Join flow timing: today `Join` returns once the controller exists; in the + unidirectional model it should wait for `StateJoining` to be reached (subscribe to + the controller future), not just for the view write — needs an explicit + `WaitState(ctx, StateJoining)` or a joining future. +- The optimistic `LocalStatusOk` fast-path in `spaceloader.startLoad` (cold-start UX) + must survive: it is process-internal and unaffected by the reconciler, but tests + around it should be carried over. +- Personal-space migrations (`WaitMigrations`) are reachable via the `Personal` + interface; with a unified controller this becomes a descriptor-provided component + handle — verify `core/` callers. diff --git a/docs/SpaceControllerSpec.md b/docs/SpaceControllerSpec.md new file mode 100644 index 0000000000..ed830bb56b --- /dev/null +++ b/docs/SpaceControllerSpec.md @@ -0,0 +1,180 @@ +# Spec: space lifecycle architecture (v2) + +Target architecture for the `space/` service layer. Background and evidence for each +decision: [SpaceControllerRefactor.md](SpaceControllerRefactor.md). + +## 1. Goals + +- **G1** One controller implementation for all account spaces (personal, shareable, + streamable, one-to-one). Kind differences are data, not code. +- **G2** Lazy loading is a first-class state: registering a space is cheap and does not + load it. Loading happens only on demand. +- **G3** Unidirectional flow: intents mutate the spaceView; a reconciler converges the + running state to it. Nothing drives transitions imperatively. +- **G4** Errors are values: callers receive the real error; failures are retryable + states, never session-permanent. +- **G5** Every wait is cancellable; no caller ever blocks on a transition. +- **G6** The status→behavior mapping exists in exactly one place. + +## 2. Non-goals (v1) + +- Unload-on-idle (Loaded→Dormant). The design must allow it; v1 does not implement it. +- Changing the spaceView data model, techspace, or tech-space bootstrap. +- Changing process internals (loader / joiner / offloader component bundles). +- Marketplace redesign: it leaves the controller registry and becomes a `service.Get` + special case (like tech space) until deleted with GO-6259. + +## 3. Core model + +### 3.1 States + +``` +Dormant registered, nothing running ← the lazy state +Loading loader process running (incl. its internal retries) +Loaded loader finished; clientspace.Space available +Joining joiner process running (aclwaiter) +Offloading offloader process running +Offloaded local data removed; re-entrant (see 3.3) +Failed last transition failed non-retryably; holds the error +``` + +### 3.2 Inputs and target function + +Each controller has exactly two inputs, written latest-wins: + +- `status` — `spaceinfo.AccountStatus` from the spaceView (watcher or direct set) +- `demand` — local bool: someone wants this space loaded + +One pure function defines all behavior (the only copy in the codebase): + +```go +func computeTarget(status AccountStatus, demand bool) State { + switch status { + case Deleted, Removing: return Offloaded + case Joining: return Joining + default: /* Active, Unknown */ if demand { return Loaded } else { return Dormant } + } +} +``` + +### 3.3 Reconciler + +One goroutine per controller owns the actual state. It is the **single writer**: only +it starts/stops process apps. + +``` +loop: + wait for wake (buffered-1 chan, ticked by any input write) or ctx.Done + target := computeTarget(inputs()) + if target != state: transition (close current process app, start next) + resolve/refresh waiters' futures +``` + +- Latest-wins: a status flip mid-transition re-ticks `wake`; the loop re-reads inputs + after every transition. There is no transition queue and no "transition in process" + error. +- Retry policy: errors the process classifies as retryable stay inside the process + (loader keeps its internal backoff loop, as today). A non-retryable transition + failure → `Failed{err}`; the reconciler retries on the next input change. +- `Offloaded` is re-entrant: if status returns to Active (CancelLeave) and demand + exists, the reconciler loads again (re-fetch from network). No terminal-state special + case. + +### 3.4 Controller interface + +```go +type SpaceController interface { + SpaceId() string + State() State // also exposes Failed error + SetStatusInfo(spaceinfo.SpacePersistentInfo) error // persist to view + input write + SetLocalInfo(spaceinfo.SpaceLocalInfo) error // passthrough to view + Demand() // input write + WaitLoad(ctx) (clientspace.Space, error) // Demand() + future + WaitState(ctx, State) error // join/offload waits + Close(ctx) error +} +``` + +`WaitLoad` blocks until `Loaded` (returns the space), `Failed` (returns the real +error), controller close (`ErrSpaceIsClosing`), or ctx cancellation. No `Current() any`, +no type assertions, no `Mode()`. + +### 3.5 Descriptor + +Controller construction takes a descriptor and performs **no I/O**: + +```go +type Descriptor struct { + SpaceId string + IsPersonal bool // loader flag + GuestKey crypto.PrivKey // streamable; nil otherwise + OwnerMetadata []byte + ExtraLoaderComponents func() []app.Component // e.g. personalmigration +} +``` + +The descriptor is derived from the spaceView (`EncodedKey` → GuestKey, id == +personalSpaceId → IsPersonal), in one place in the registry. + +## 4. Service layer + +### 4.1 Registry + +`registry.GetOrCreate(id) SpaceController` — creates a Dormant controller from the +spaceView-derived descriptor. Replaces both `spaceControllers` and the `waiting` +singleflight/error-cache maps. Plain map + mutex; creation is cheap and synchronous. + +### 4.2 Flows + +- **Watcher** (unchanged subscription): view added → `GetOrCreate(id)`; view changed → + `ctrl.SetStatusInfo(...)`. Delivery is a non-blocking input write — no + goroutine-per-event, no shared-mutex-in-copied-struct idiom. +- **Get/Wait**: tech space and marketplace special-cased; else + `GetOrCreate(id).WaitLoad(ctx)`. `Wait` differs from `Get` only by first waiting + (event-driven, on the watcher) for the spaceView to exist. No polling. +- **Create*** (new space / one-to-one / streamable): create storage + spaceView as + today, then `GetOrCreate(id)` + `WaitLoad(ctx)`. Factory builds data, not + controllers. +- **Join / InviteJoin / Delete / CancelLeave**: write the spaceView (create if absent), + then `GetOrCreate(id).WaitState(ctx, Joining / Loaded / …)` as the RPC requires. + This is the unidirectional model from the `join.go` TODO. + +### 4.3 Demand wiring (lazy loading) + +| Trigger | Effect | +|---|---| +| eager mode (no preferredSpaceId) | `Demand()` on every controller at registration | +| lazy mode | `Demand()` only on the preferred space | +| `Get`/`Wait` on a space | `WaitLoad` ⇒ implicit `Demand()` | +| preload release (RPC / timer / preferred-broken fallback) | `Demand()` on all registered controllers | + +This table replaces `deferredStatuses`, `releasing`, `lazyMode` branches, +`ensureSpaceStarted`, `resolveDerivedInfo`, and both test-seam hooks. + +## 5. Invariants + +1. Only the reconciler goroutine starts or stops process apps (single writer). +2. Target is always computed from current inputs by `computeTarget`; transitions are + never queued or requested by name. +3. Input writes never block; callers block only in `Wait*`, always with ctx. +4. Every outstanding `Wait*` resolves on controller close. +5. Deletion/offload outranks demand (encoded in `computeTarget`'s ordering). +6. Controller construction and registry registration perform no I/O. +7. An error can only be observed through a `Wait*` return or `State()`; no error is + cached past the next input change. + +## 6. Shutdown + +`service.Close` cancels all controller contexts in parallel and waits. Each reconciler +tears down its current process app; all futures resolve with `ErrSpaceIsClosing`. + +## 7. Open questions + +- **Deletion controller interplay**: offloader currently calls + `delController.AddSpaceToDelete`; with re-entrant Offloaded, confirm a + CancelLeave→reload also removes the space from the deletion queue. +- **`WaitMigrations`** (personal space): exposed today via the `Personal` interface; + becomes a handle returned by `ExtraLoaderComponents` wiring — verify `core/` callers. +- **Status enum for clients**: `Loaded` vs `Loading` is now observable; decide whether + to surface it in `spaceinfo.LocalStatus` or keep mapping both to `Ok`/`Loading` as + today (optimistic-Ok fast path in `spaceloader` is unaffected either way). diff --git a/space/create.go b/space/create.go index 930269761f..a3bf05a66a 100644 --- a/space/create.go +++ b/space/create.go @@ -10,7 +10,6 @@ import ( "github.com/anyproto/anytype-heart/pkg/lib/pb/model" "github.com/anyproto/anytype-heart/space/clientspace" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/loader" "github.com/anyproto/anytype-heart/space/spacedomain" "github.com/anyproto/anytype-heart/space/spaceinfo" ) @@ -59,86 +58,48 @@ func (s *service) CreateOneToOne(ctx context.Context, description *spaceinfo.Spa err = fmt.Errorf("spacecore: create onetoone: %w", err) return } - s.mu.Lock() - wait := make(chan struct{}) - s.waiting[coreSpace.Id()] = controllerWaiter{ - wait: wait, - } - s.mu.Unlock() participantData := spaceinfo.OneToOneParticipantData{ Identity: bobProfile.IdentityProfile.Identity, RequestMetadataKey: bobProfile.RequestMetadata, } - ctrl, err := s.factory.CreateOneToOneSpace(ctx, coreSpace.Id(), description, participantData) - if err != nil { - s.mu.Lock() - close(wait) - s.waiting[coreSpace.Id()] = controllerWaiter{ - wait: wait, - err: err, - } - s.mu.Unlock() - err = fmt.Errorf("factory: create onetoone: %w", err) - return nil, err + if err = s.factory.CreateOneToOneSpace(ctx, coreSpace.Id(), description, participantData); err != nil { + return nil, fmt.Errorf("factory: create onetoone: %w", err) } - sp, err = ctrl.Current().(loader.LoadWaiter).WaitLoad(ctx) - s.mu.Lock() - close(wait) + ctrl, err := s.waitCtrl(ctx, coreSpace.Id()) if err != nil { - s.waiting[coreSpace.Id()] = controllerWaiter{ - wait: wait, - err: err, - } - s.mu.Unlock() - err = fmt.Errorf("loader: create onetoone: %w", err) - return nil, err + return nil, fmt.Errorf("wait controller: create onetoone: %w", err) + } + sp, err = s.waitLoad(ctx, ctrl) + if err != nil { + return nil, fmt.Errorf("loader: create onetoone: %w", err) } - s.spaceControllers[ctrl.SpaceId()] = ctrl - s.mu.Unlock() s.updater.UpdateCoordinatorStatus() return } +// create makes a new shareable space: storage and space view first, then the +// watcher registers the controller and we wait for the space to load. func (s *service) create(ctx context.Context, description *spaceinfo.SpaceDescription) (sp clientspace.Space, err error) { var spaceType = spacedomain.SpaceTypeRegular coreSpace, err := s.spaceCore.Create(ctx, spaceType, s.repKey, s.AccountMetadataPayload()) if err != nil { return nil, err } - s.mu.Lock() - wait := make(chan struct{}) - s.waiting[coreSpace.Id()] = controllerWaiter{ - wait: wait, + if err = s.factory.CreateShareableSpace(ctx, coreSpace.Id(), description); err != nil { + return nil, err } - s.mu.Unlock() - ctrl, err := s.factory.CreateShareableSpace(ctx, coreSpace.Id(), description) + ctrl, err := s.waitCtrl(ctx, coreSpace.Id()) if err != nil { - s.mu.Lock() - close(wait) - s.waiting[coreSpace.Id()] = controllerWaiter{ - wait: wait, - err: err, - } - s.mu.Unlock() - return nil, err + return nil, fmt.Errorf("wait controller: %w", err) } - sp, err = ctrl.Current().(loader.LoadWaiter).WaitLoad(ctx) - s.mu.Lock() - close(wait) + sp, err = s.waitLoad(ctx, ctrl) if err != nil { - s.waiting[coreSpace.Id()] = controllerWaiter{ - wait: wait, - err: err, - } - s.mu.Unlock() return nil, err } - s.spaceControllers[ctrl.SpaceId()] = ctrl - s.mu.Unlock() s.updater.UpdateCoordinatorStatus() return } diff --git a/space/deletioncontroller/deletioncontroller.go b/space/deletioncontroller/deletioncontroller.go index 677e0772ad..1386a025ff 100644 --- a/space/deletioncontroller/deletioncontroller.go +++ b/space/deletioncontroller/deletioncontroller.go @@ -42,6 +42,9 @@ const ( type DeletionController interface { app.ComponentRunnable AddSpaceToDelete(spaceId string) + // RemoveSpaceToDelete cancels a pending coordinator deletion for the + // space, e.g. when the user restores it (CancelLeave) and it reloads. + RemoveSpaceToDelete(spaceId string) UpdateCoordinatorStatus() } @@ -86,6 +89,12 @@ func (d *deletionController) AddSpaceToDelete(spaceId string) { d.updater.notify() } +func (d *deletionController) RemoveSpaceToDelete(spaceId string) { + d.mx.Lock() + defer d.mx.Unlock() + delete(d.toDelete, spaceId) +} + func (d *deletionController) UpdateCoordinatorStatus() { d.updater.notify() } diff --git a/space/deletioncontroller/deletioncontroller_test.go b/space/deletioncontroller/deletioncontroller_test.go index a26ed950ce..785a5e1f82 100644 --- a/space/deletioncontroller/deletioncontroller_test.go +++ b/space/deletioncontroller/deletioncontroller_test.go @@ -71,6 +71,33 @@ func TestDeletionController_Loop(t *testing.T) { require.NoError(t, err) require.NotContains(t, fx.toDelete, "spaceId1") }) + t.Run("removed space is not deleted", func(t *testing.T) { + fx := newFixture(t) + defer fx.finish(t) + fx.AddSpaceToDelete("spaceId1") + // the space was restored (e.g. CancelLeave) before the loop ran + fx.RemoveSpaceToDelete("spaceId1") + payloads := []*coordinatorproto.SpaceStatusPayload{ + { + Status: coordinatorproto.SpaceStatus_SpaceStatusCreated, + Permissions: coordinatorproto.SpacePermissions_SpacePermissionsOwner, + IsShared: false, + }, + } + fx.mockSpaceManager.EXPECT().AllSpaceIds().Return([]string{"spaceId1"}) + fx.mockClient.EXPECT().StatusCheckMany(ctx, []string{"spaceId1"}).Return(payloads, nil, nil) + status := spaceinfo.NewSpaceLocalInfo("spaceId1") + status. + SetRemoteStatus(spaceinfo.RemoteStatusOk). + SetShareableStatus(spaceinfo.ShareableStatusNotShareable) + fx.mockSpaceManager.EXPECT().UpdateRemoteStatus(ctx, spaceinfo.SpaceRemoteStatusInfo{ + IsOwned: true, + LocalInfo: status, + }).Return(nil) + // no Delete expectation: deleting the restored space fails the test + err := fx.loopIterate(ctx) + require.NoError(t, err) + }) t.Run("nil limits", func(t *testing.T) { fx := newFixture(t) defer fx.finish(t) diff --git a/space/deletioncontroller/mock_deletioncontroller/mock_DeletionController.go b/space/deletioncontroller/mock_deletioncontroller/mock_DeletionController.go index 836ee8a8ae..a05625468e 100644 --- a/space/deletioncontroller/mock_deletioncontroller/mock_DeletionController.go +++ b/space/deletioncontroller/mock_deletioncontroller/mock_DeletionController.go @@ -193,6 +193,39 @@ func (_c *MockDeletionController_Name_Call) RunAndReturn(run func() string) *Moc return _c } +// RemoveSpaceToDelete provides a mock function with given fields: spaceId +func (_m *MockDeletionController) RemoveSpaceToDelete(spaceId string) { + _m.Called(spaceId) +} + +// MockDeletionController_RemoveSpaceToDelete_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveSpaceToDelete' +type MockDeletionController_RemoveSpaceToDelete_Call struct { + *mock.Call +} + +// RemoveSpaceToDelete is a helper method to define mock.On call +// - spaceId string +func (_e *MockDeletionController_Expecter) RemoveSpaceToDelete(spaceId interface{}) *MockDeletionController_RemoveSpaceToDelete_Call { + return &MockDeletionController_RemoveSpaceToDelete_Call{Call: _e.mock.On("RemoveSpaceToDelete", spaceId)} +} + +func (_c *MockDeletionController_RemoveSpaceToDelete_Call) Run(run func(spaceId string)) *MockDeletionController_RemoveSpaceToDelete_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(string)) + }) + return _c +} + +func (_c *MockDeletionController_RemoveSpaceToDelete_Call) Return() *MockDeletionController_RemoveSpaceToDelete_Call { + _c.Call.Return() + return _c +} + +func (_c *MockDeletionController_RemoveSpaceToDelete_Call) RunAndReturn(run func(string)) *MockDeletionController_RemoveSpaceToDelete_Call { + _c.Run(run) + return _c +} + // Run provides a mock function with given fields: ctx func (_m *MockDeletionController) Run(ctx context.Context) error { ret := _m.Called(ctx) diff --git a/space/init.go b/space/init.go index 4e4db049ec..73230e8a50 100644 --- a/space/init.go +++ b/space/init.go @@ -2,10 +2,10 @@ package space import ( "context" - - "github.com/anyproto/anytype-heart/pkg/lib/localstore/addr" ) +// initMarketplaceSpace sets up the virtual marketplace space. It lives +// outside the controller registry (like tech space): Get special-cases it. func (s *service) initMarketplaceSpace(ctx context.Context) error { ctrl, err := s.factory.CreateMarketplaceSpace(ctx) if err != nil { @@ -15,14 +15,7 @@ func (s *service) initMarketplaceSpace(ctx context.Context) error { if err != nil { return err } - s.mu.Lock() - defer s.mu.Unlock() - wait := make(chan struct{}) - close(wait) - s.waiting[addr.AnytypeMarketplaceWorkspace] = controllerWaiter{ - wait: wait, - } - s.spaceControllers[addr.AnytypeMarketplaceWorkspace] = ctrl + s.marketplaceCtrl = ctrl return nil } diff --git a/space/internal/accountspace/accountspace.go b/space/internal/accountspace/accountspace.go new file mode 100644 index 0000000000..0754828d89 --- /dev/null +++ b/space/internal/accountspace/accountspace.go @@ -0,0 +1,200 @@ +// Package accountspace provides the single SpaceController implementation for +// all account spaces (personal, shareable, streamable, one-to-one). Kind +// differences are expressed as Descriptor data, not separate controller types. +package accountspace + +import ( + "context" + "fmt" + + "github.com/anyproto/any-sync/app" + "github.com/anyproto/any-sync/app/logger" + "github.com/anyproto/any-sync/util/crypto" + "go.uber.org/zap" + + "github.com/anyproto/anytype-heart/space/clientspace" + "github.com/anyproto/anytype-heart/space/deletioncontroller" + "github.com/anyproto/anytype-heart/space/internal/components/spacestatus" + "github.com/anyproto/anytype-heart/space/internal/spacecontroller" + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/initial" + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/joiner" + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/loader" + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/offloader" + "github.com/anyproto/anytype-heart/space/spaceinfo" +) + +var log = logger.NewNamed("common.space.accountspace") + +// Descriptor carries everything kind-specific about a space controller. +type Descriptor struct { + SpaceId string + // IsPersonal makes the loader stop on mandatory-objects failure and is + // reported to spaceloader (stopIfMandatoryFail). + IsPersonal bool + // GuestKey, when set, makes the space load with a guest signing key + // (streamable spaces). + GuestKey crypto.PrivKey + OwnerMetadata []byte + // ExtraLoaderComponents returns components registered into each loading + // app in addition to the standard set. Called once per loading transition + // so components are always fresh (e.g. personalmigration). + ExtraLoaderComponents func() []app.Component +} + +type statusUpdater interface { + UpdateCoordinatorStatus() +} + +type spaceController struct { + spaceId string + desc Descriptor + app *app.App + status spacestatus.SpaceStatus + updater statusUpdater + + rec *reconciler +} + +func makeStatusApp(a *app.App, spaceId string) (*app.App, error) { + newApp := a.ChildApp() + newApp.Register(spacestatus.New(spaceId)) + err := newApp.Start(context.Background()) + if err != nil { + return nil, err + } + return newApp, nil +} + +func NewSpaceController(desc Descriptor, a *app.App) (spacecontroller.SpaceController, error) { + newApp, err := makeStatusApp(a, desc.SpaceId) + if err != nil { + return nil, err + } + s := &spaceController{ + spaceId: desc.SpaceId, + desc: desc, + status: newApp.MustComponent(spacestatus.CName).(spacestatus.SpaceStatus), + app: newApp, + } + + // this is done for tests to not complicate them :-) + if updater, ok := a.Component(deletioncontroller.CName).(statusUpdater); ok { + s.updater = updater + } + s.rec = newReconciler(s, log.With(zap.String("spaceId", desc.SpaceId))) + // seed the desired status so deletion/joining converge in the background + // even when the space is never demanded (deletion outranks demand) + s.rec.setStatus(s.status.GetPersistentStatus()) + return s, nil +} + +func (s *spaceController) SpaceId() string { + return s.spaceId +} + +// Start demands the space and blocks until the reconciler converges on the +// first target (loading/joining/offloading started), returning the real +// transition error on failure. +func (s *spaceController) Start(ctx context.Context) error { + defer func() { + if s.updater != nil { + s.updater.UpdateCoordinatorStatus() + } + }() + s.rec.setInputs(s.status.GetPersistentStatus(), true) + _, err := s.rec.waitConverged(ctx) + return err +} + +// Demand marks the space as wanted-loaded; loading happens in the background. +func (s *spaceController) Demand() { + s.rec.setDemand() +} + +// WaitLoad demands the space and blocks until it is fully loaded. It fails +// with the real load error, with ErrModeUnreachable when the space status +// dictates another mode (offloading/joining), or on ctx/close. +func (s *spaceController) WaitLoad(ctx context.Context) (clientspace.Space, error) { + s.rec.setDemand() + proc, err := s.rec.waitMode(ctx, mode.ModeLoading) + if err != nil { + return nil, err + } + ld, ok := proc.(loader.LoadWaiter) + if !ok { + return nil, fmt.Errorf("loading process does not support WaitLoad") + } + return ld.WaitLoad(ctx) +} + +// WaitMode blocks until the process for mode m is running, failing with the +// real transition error or ErrModeUnreachable when the target differs. +func (s *spaceController) WaitMode(ctx context.Context, m mode.Mode) error { + _, err := s.rec.waitMode(ctx, m) + return err +} + +func (s *spaceController) Mode() mode.Mode { + return s.rec.getMode() +} + +func (s *spaceController) SetPersistentInfo(ctx context.Context, info spaceinfo.SpacePersistentInfo) error { + err := s.status.SetPersistentInfo(info) + if err != nil { + return err + } + return s.Update() +} + +func (s *spaceController) SetLocalInfo(ctx context.Context, info spaceinfo.SpaceLocalInfo) error { + return s.status.SetLocalInfo(info) +} + +// Update pushes the latest persistent status into the reconciler. It never +// blocks on the resulting transition; convergence is observed via Wait*. +func (s *spaceController) Update() error { + s.rec.setStatus(s.status.GetPersistentStatus()) + return nil +} + +func (s *spaceController) Process(md mode.Mode) mode.Process { + switch md { + case mode.ModeLoading: + var extraComps []app.Component + if s.desc.ExtraLoaderComponents != nil { + extraComps = s.desc.ExtraLoaderComponents() + } + return loader.New(s.app, loader.Params{ + SpaceId: s.spaceId, + IsPersonal: s.desc.IsPersonal, + OwnerMetadata: s.desc.OwnerMetadata, + GuestKey: s.desc.GuestKey, + AdditionalComps: extraComps, + }) + case mode.ModeOffloading: + return offloader.New(s.app) + case mode.ModeJoining: + return joiner.New(s.app, joiner.Params{ + SpaceId: s.spaceId, + Status: s.status, + Log: log, + }) + default: + return initial.New() + } +} + +func (s *spaceController) Close(ctx context.Context) error { + s.rec.close(ctx) + // this closes status + return s.app.Close(ctx) +} + +func (s *spaceController) GetStatus() spaceinfo.AccountStatus { + return s.status.GetPersistentStatus() +} + +func (s *spaceController) GetLocalStatus() spaceinfo.LocalStatus { + return s.status.GetLocalStatus() +} diff --git a/space/internal/accountspace/accountspace_test.go b/space/internal/accountspace/accountspace_test.go new file mode 100644 index 0000000000..2ab04cec75 --- /dev/null +++ b/space/internal/accountspace/accountspace_test.go @@ -0,0 +1,496 @@ +package accountspace + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/anyproto/any-sync/app" + "github.com/anyproto/any-sync/util/crypto" + "github.com/stretchr/testify/require" + + "github.com/anyproto/anytype-heart/pkg/lib/pb/model" + "github.com/anyproto/anytype-heart/space/clientspace" + "github.com/anyproto/anytype-heart/space/internal/components/spacestatus" + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/initial" + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" + "github.com/anyproto/anytype-heart/space/spaceinfo" + "github.com/anyproto/anytype-heart/space/techspace" +) + +func TestSpaceController_InvitingLoading(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusJoining) + defer fx.stop() + err := fx.ctrl.Start(context.Background()) + require.NoError(t, err) + require.Equal(t, mode.ModeJoining, fx.ctrl.Mode()) + // the joining stub flips the status to Active, which must converge to loading + fx.waitModes(t, mode.ModeJoining, mode.ModeLoading) +} + +func TestSpaceController_LoadingDeleting(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + defer fx.stop() + err := fx.ctrl.Start(context.Background()) + require.NoError(t, err) + require.Equal(t, mode.ModeLoading, fx.ctrl.Mode()) + err = fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusDeleted)) + require.NoError(t, err) + fx.waitModes(t, mode.ModeLoading, mode.ModeOffloading) +} + +func TestSpaceController_LoadingDeletingMultipleUpdates(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + defer fx.stop() + err := fx.ctrl.Start(context.Background()) + require.NoError(t, err) + require.Equal(t, mode.ModeLoading, fx.ctrl.Mode()) + wg := sync.WaitGroup{} + for i := 0; i < 10; i++ { + wg.Add(1) + go func() { + err := fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusDeleted)) + require.NoError(t, err) + wg.Done() + }() + } + wg.Wait() + fx.waitModes(t, mode.ModeLoading, mode.ModeOffloading) +} + +func TestSpaceController_Deleting(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusDeleted) + defer fx.stop() + err := fx.ctrl.Start(context.Background()) + require.NoError(t, err) + require.Equal(t, mode.ModeOffloading, fx.ctrl.Mode()) + fx.waitModes(t, mode.ModeOffloading) +} + +func TestSpaceController_DeletedThenActiveReloads(t *testing.T) { + // offloading is not terminal: when the status returns to Active + // (e.g. CancelLeave), the reconciler loads the space again + fx := newFixture(t, spaceinfo.AccountStatusDeleted) + defer fx.stop() + err := fx.ctrl.Start(context.Background()) + require.NoError(t, err) + require.Equal(t, mode.ModeOffloading, fx.ctrl.Mode()) + err = fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusActive)) + require.NoError(t, err) + fx.waitModes(t, mode.ModeOffloading, mode.ModeLoading) +} + +func TestSpaceController_LatestWinsCoalescing(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + defer fx.stop() + err := fx.ctrl.Start(context.Background()) + require.NoError(t, err) + // rapid flips must converge on the last written status without losing it + for i := 0; i < 5; i++ { + require.NoError(t, fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusDeleted))) + require.NoError(t, fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusActive))) + } + require.NoError(t, fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusDeleted))) + require.Eventually(t, func() bool { + return fx.ctrl.Mode() == mode.ModeOffloading + }, time.Second, 5*time.Millisecond) +} + +func TestSpaceController_StartFailureSurfacesErrorAndRetries(t *testing.T) { + startErr := errors.New("process start failed") + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + defer fx.stop() + fx.f.failLoading.Store(&startErr) + + err := fx.ctrl.Start(context.Background()) + require.ErrorIs(t, err, startErr) + require.Equal(t, mode.ModeInitial, fx.ctrl.Mode()) + + // next input change clears the failure and retries + fx.f.failLoading.Store(nil) + err = fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusActive)) + require.NoError(t, err) + require.Eventually(t, func() bool { + return fx.ctrl.Mode() == mode.ModeLoading + }, time.Second, 5*time.Millisecond) +} + +// Regression: an input change racing a failing transition must not be lost to +// the recorded failure — the reconciler retries against the fresh inputs. +func TestSpaceController_InputDuringFailingTransitionIsNotLost(t *testing.T) { + startErr := errors.New("loading start failed") + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + defer fx.stop() + fx.f.failLoading.Store(&startErr) + fx.f.blockLoading = make(chan struct{}) + + go func() { + _ = fx.ctrl.Start(context.Background()) + }() + // loading.Start is now blocked and will fail when released + require.Eventually(t, func() bool { + return fx.f.loadingStarted.Load() + }, time.Second, time.Millisecond) + + // the status flips to Deleted while the failing transition is in flight + require.NoError(t, fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusDeleted))) + close(fx.f.blockLoading) + + // the deletion must win: the controller converges to offloading instead + // of parking in the failed state + require.Eventually(t, func() bool { + return fx.ctrl.Mode() == mode.ModeOffloading + }, time.Second, 5*time.Millisecond) +} + +// Regression: a repeated demand (user retry via Get/WaitLoad) clears a parked +// failure even though demand was already set. +func TestSpaceController_RepeatedDemandClearsParkedFailure(t *testing.T) { + startErr := errors.New("process start failed") + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + defer fx.stop() + fx.f.failLoading.Store(&startErr) + + err := fx.ctrl.Start(context.Background()) + require.ErrorIs(t, err, startErr) + require.Equal(t, mode.ModeInitial, fx.ctrl.Mode()) + + // the failure cause is gone; a retry through WaitLoad must succeed even + // though neither status nor the demand flag changes value + fx.f.failLoading.Store(nil) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + _, err = fx.ctrl.WaitLoad(ctx) + require.NoError(t, err) + require.Equal(t, mode.ModeLoading, fx.ctrl.Mode()) +} + +func TestSpaceController_DormantStaysIdleUntilDemand(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + defer fx.stop() + // registration alone (no Start) must not load anything + time.Sleep(50 * time.Millisecond) + require.Equal(t, mode.ModeInitial, fx.ctrl.Mode()) + require.Empty(t, fx.reg.snapshot()) + + fx.ctrl.Demand() + fx.waitModes(t, mode.ModeLoading) +} + +func TestSpaceController_DormantOffloadsWithoutDemand(t *testing.T) { + // deletion outranks demand: a never-demanded space still offloads + fx := newFixture(t, spaceinfo.AccountStatusDeleted) + defer fx.stop() + fx.waitModes(t, mode.ModeOffloading) +} + +func TestSpaceController_WaitLoadFailsWhenOffloading(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusDeleted) + defer fx.stop() + fx.waitModes(t, mode.ModeOffloading) + _, err := fx.ctrl.WaitLoad(context.Background()) + require.ErrorIs(t, err, ErrModeUnreachable) +} + +func TestSpaceController_CloseUnblocksWaiters(t *testing.T) { + fx := newFixture(t, spaceinfo.AccountStatusUnknown) + fx.f.blockLoading = make(chan struct{}) + + startDone := make(chan error, 1) + go func() { + startDone <- fx.ctrl.Start(context.Background()) + }() + // wait until the loading process is blocked in Start + require.Eventually(t, func() bool { + return fx.f.loadingStarted.Load() + }, time.Second, time.Millisecond) + + closeDone := make(chan error, 1) + go func() { + closeDone <- fx.ctrl.Close(context.Background()) + }() + close(fx.f.blockLoading) + + select { + case err := <-startDone: + // either outcome is fine (converged just before close, or unblocked by + // close), but the waiter must not hang + _ = err + case <-time.After(time.Second): + t.Fatal("Start blocked after Close") + } + select { + case err := <-closeDone: + require.NoError(t, err) + case <-time.After(time.Second): + t.Fatal("Close hung") + } +} + +func makePersistentInfo(spaceId string, status spaceinfo.AccountStatus) spaceinfo.SpacePersistentInfo { + info := spaceinfo.NewSpacePersistentInfo(spaceId) + info.SetAccountStatus(status) + return info +} + +type modeRegister struct { + modes []mode.Mode + sync.Mutex +} + +func (m *modeRegister) register(mode mode.Mode) { + m.Lock() + m.modes = append(m.modes, mode) + m.Unlock() +} + +func (m *modeRegister) snapshot() []mode.Mode { + m.Lock() + defer m.Unlock() + return append([]mode.Mode{}, m.modes...) +} + +type spaceStatusStub struct { + spaceId string + localStatus spaceinfo.LocalStatus + remoteStatus spaceinfo.RemoteStatus + accountStatus spaceinfo.AccountStatus + persistentUpdater func(status spaceinfo.AccountStatus) + sync.Mutex +} + +func (s *spaceStatusStub) Init(a *app.App) (err error) { + return nil +} + +func (s *spaceStatusStub) Name() (name string) { + return spacestatus.CName +} + +func (s *spaceStatusStub) SpaceId() string { + return s.spaceId +} + +func (s *spaceStatusStub) GetLocalStatus() spaceinfo.LocalStatus { + s.Lock() + defer s.Unlock() + return s.localStatus +} + +func (s *spaceStatusStub) SetOwner(ownerIdentity string, createdDate int64) (err error) { + return +} + +func (s *spaceStatusStub) GetRemoteStatus() spaceinfo.RemoteStatus { + s.Lock() + defer s.Unlock() + return s.remoteStatus +} + +func (s *spaceStatusStub) GetPersistentStatus() spaceinfo.AccountStatus { + s.Lock() + defer s.Unlock() + return s.accountStatus +} + +func (s *spaceStatusStub) Run(ctx context.Context) (err error) { + return nil +} + +func (s *spaceStatusStub) Close(ctx context.Context) (err error) { + return nil +} + +func (s *spaceStatusStub) SetPersistentStatus(status spaceinfo.AccountStatus) (err error) { + s.Lock() + defer s.Unlock() + s.accountStatus = status + if s.persistentUpdater != nil { + s.persistentUpdater(status) + } + return nil +} + +func (s *spaceStatusStub) SetPersistentInfo(info spaceinfo.SpacePersistentInfo) (err error) { + s.Lock() + defer s.Unlock() + s.accountStatus = info.GetAccountStatus() + return +} + +func (s *spaceStatusStub) SetLocalStatus(status spaceinfo.LocalStatus) error { + s.Lock() + defer s.Unlock() + s.localStatus = status + return nil +} + +func (s *spaceStatusStub) SetLocalInfo(info spaceinfo.SpaceLocalInfo) (err error) { + s.Lock() + defer s.Unlock() + s.localStatus = info.GetLocalStatus() + return +} + +func (s *spaceStatusStub) SetAccessType(status spaceinfo.AccessType) (err error) { + return +} + +func (s *spaceStatusStub) SetAclInfo(isAclEmpty bool, pushKey crypto.PrivKey, pushEncryptionKey crypto.SymKey, spaceJoinedDate int64) (err error) { + return +} + +func (s *spaceStatusStub) GetLatestAclHeadId() string { + return "" +} + +func (s *spaceStatusStub) SetMyParticipantStatus(st model.ParticipantStatus) (err error) { + return nil +} + +func (s *spaceStatusStub) GetSpaceView() techspace.SpaceView { + return nil +} + +var _ spacestatus.SpaceStatus = (*spaceStatusStub)(nil) + +type joining struct { + status spacestatus.SpaceStatus + reg *modeRegister +} + +func (i *joining) Start(ctx context.Context) error { + go func() { + _ = i.status.SetPersistentStatus(spaceinfo.AccountStatusActive) + }() + i.reg.register(mode.ModeJoining) + return nil +} + +func (i *joining) Close(ctx context.Context) error { + return nil +} + +type loading struct { + f *factory + reg *modeRegister +} + +func (l *loading) Start(ctx context.Context) error { + l.f.loadingStarted.Store(true) + if l.f.blockLoading != nil { + <-l.f.blockLoading + } + if errp := l.f.failLoading.Load(); errp != nil && *errp != nil { + return *errp + } + l.reg.register(mode.ModeLoading) + return nil +} + +func (l *loading) Close(ctx context.Context) error { + return nil +} + +func (l *loading) WaitLoad(ctx context.Context) (clientspace.Space, error) { + return nil, nil +} + +type offloading struct { + reg *modeRegister +} + +func (l *offloading) Start(ctx context.Context) error { + l.reg.register(mode.ModeOffloading) + return nil +} + +func (l *offloading) Close(ctx context.Context) error { + return nil +} + +type factory struct { + status spacestatus.SpaceStatus + reg *modeRegister + + failLoading atomic.Pointer[error] + blockLoading chan struct{} + loadingStarted atomic.Bool +} + +func (f *factory) Process(md mode.Mode) mode.Process { + switch md { + case mode.ModeInitial: + return initial.New() + case mode.ModeJoining: + return &joining{status: f.status, reg: f.reg} + case mode.ModeLoading: + return &loading{f: f, reg: f.reg} + case mode.ModeOffloading: + return &offloading{reg: f.reg} + default: + panic("unhandled default case") + } +} + +type fixture struct { + f *factory + s *spaceStatusStub + ctrl *spaceController + reg *modeRegister +} + +func newFixture(t *testing.T, startStatus spaceinfo.AccountStatus) *fixture { + reg := &modeRegister{} + s := &spaceStatusStub{ + spaceId: "spaceId", + accountStatus: startStatus, + } + f := &factory{ + status: s, + reg: reg, + } + controller := &spaceController{ + spaceId: "spaceId", + status: s, + app: &app.App{}, + } + controller.rec = newReconciler(f, log) + // mirror NewSpaceController: seed the desired status at registration + controller.rec.setStatus(s.GetPersistentStatus()) + s.persistentUpdater = func(status spaceinfo.AccountStatus) { + go func() { + err := controller.Update() + require.NoError(t, err) + }() + } + return &fixture{ + f: f, + s: s, + ctrl: controller, + reg: reg, + } +} + +// waitModes asserts the registered mode sequence converges to want. +func (fx *fixture) waitModes(t *testing.T, want ...mode.Mode) { + require.Eventually(t, func() bool { + got := fx.reg.snapshot() + if len(got) != len(want) { + return false + } + for i := range want { + if got[i] != want[i] { + return false + } + } + return true + }, time.Second, 5*time.Millisecond, "modes: %v", fx.reg.snapshot()) +} + +func (fx *fixture) stop() { + fx.ctrl.rec.close(context.Background()) +} diff --git a/space/internal/accountspace/reconciler.go b/space/internal/accountspace/reconciler.go new file mode 100644 index 0000000000..2f5fd942ed --- /dev/null +++ b/space/internal/accountspace/reconciler.go @@ -0,0 +1,314 @@ +package accountspace + +import ( + "context" + "errors" + "fmt" + "sync" + + "github.com/anyproto/any-sync/app/logger" + "go.uber.org/zap" + + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" + "github.com/anyproto/anytype-heart/space/spaceinfo" +) + +var ( + ErrCtrlClosed = errors.New("space controller is closed") + // ErrModeUnreachable is returned by waits whose wanted mode is not the + // current target (e.g. WaitLoad on a space whose status dictates + // offloading). + ErrModeUnreachable = errors.New("mode unreachable") +) + +type processFactory interface { + Process(md mode.Mode) mode.Process +} + +// computeTarget is the single place mapping desired inputs to a lifecycle +// state, for all space kinds. Deletion outranks demand; without demand the +// space stays dormant (ModeInitial). +func computeTarget(status spaceinfo.AccountStatus, demand bool) mode.Mode { + switch status { + case spaceinfo.AccountStatusDeleted, spaceinfo.AccountStatusRemoving: + return mode.ModeOffloading + case spaceinfo.AccountStatusJoining: + return mode.ModeJoining + default: + if demand { + return mode.ModeLoading + } + return mode.ModeInitial + } +} + +// reconciler owns the actual state of one space: it is the only goroutine +// that starts and stops mode processes. Inputs (status, demand) are written +// latest-wins by any goroutine; the loop converges the running process to +// computeTarget(inputs) and re-reads inputs after every transition, so input +// changes are never lost and nothing ever blocks on a transition. +type reconciler struct { + factory processFactory + log logger.CtxLogger + + mu sync.Mutex + status spaceinfo.AccountStatus // desired: from the space view + demand bool // desired: someone wants the space loaded + current mode.Process // actual: running process + mode mode.Mode // actual: its mode + // gen counts input changes; a failing transition only parks failedErr if + // no input changed while it ran, so an input change never loses its + // effect to a concurrently recorded failure. + gen uint64 + // failedErr holds the last transition error. While set, the loop does not + // retry; any input change clears it (no error outlives an input change). + failedErr error + // changed is closed and replaced on every state or input change; + // waiters re-evaluate on it (broadcast). + changed chan struct{} + closeCtx context.Context + + wake chan struct{} + ctx context.Context + cancel context.CancelFunc + done chan struct{} +} + +func newReconciler(factory processFactory, log logger.CtxLogger) *reconciler { + ctx, cancel := context.WithCancel(context.Background()) + r := &reconciler{ + factory: factory, + log: log, + current: factory.Process(mode.ModeInitial), + mode: mode.ModeInitial, + changed: make(chan struct{}), + wake: make(chan struct{}, 1), + ctx: ctx, + cancel: cancel, + done: make(chan struct{}), + } + go r.run() + return r +} + +// inputChangedLocked records an input change: bumps the generation (so an +// in-flight failing transition does not park), clears any parked failure, and +// wakes the loop and the waiters. Must be called with r.mu held; callers must +// wakeUp() after unlocking. +func (r *reconciler) inputChangedLocked() { + r.gen++ + r.failedErr = nil + r.broadcastLocked() +} + +// setInputs updates the desired state; no-op if nothing changed. +func (r *reconciler) setInputs(status spaceinfo.AccountStatus, demand bool) { + r.mu.Lock() + if r.status == status && r.demand == demand && r.failedErr == nil { + r.mu.Unlock() + return + } + r.status = status + r.demand = demand + r.inputChangedLocked() + r.mu.Unlock() + r.wakeUp() +} + +func (r *reconciler) setStatus(status spaceinfo.AccountStatus) { + r.mu.Lock() + if r.status == status { + r.mu.Unlock() + return + } + r.status = status + r.inputChangedLocked() + r.mu.Unlock() + r.wakeUp() +} + +// setDemand marks the space wanted-loaded. A repeated demand on an already +// demanded space clears a parked failure (a user retry must retry), but is a +// no-op otherwise. +func (r *reconciler) setDemand() { + r.mu.Lock() + if r.demand && r.failedErr == nil { + r.mu.Unlock() + return + } + r.demand = true + r.inputChangedLocked() + r.mu.Unlock() + r.wakeUp() +} + +func (r *reconciler) getMode() mode.Mode { + r.mu.Lock() + defer r.mu.Unlock() + return r.mode +} + +// waitConverged blocks until the actual state equals the current target and +// returns the running process. It fails with the real transition error if the +// reconciler is stuck in a failed state, or when the caller ctx is done or +// the reconciler is closed. The target is re-read on every state change, so a +// waiter never hangs on a target that moved away. +func (r *reconciler) waitConverged(ctx context.Context) (mode.Process, error) { + for { + r.mu.Lock() + if r.failedErr != nil { + err := r.failedErr + r.mu.Unlock() + return nil, err + } + if target := computeTarget(r.status, r.demand); r.mode == target { + p := r.current + r.mu.Unlock() + return p, nil + } + ch := r.changed + r.mu.Unlock() + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-r.ctx.Done(): + return nil, ErrCtrlClosed + case <-ch: + } + } +} + +// waitMode blocks until the actual state equals want and returns the running +// process. It fails fast with ErrModeUnreachable when want is not the current +// target, with the real transition error when the reconciler is parked in a +// failed state, and with ErrCtrlClosed / ctx.Err on shutdown or cancellation. +func (r *reconciler) waitMode(ctx context.Context, want mode.Mode) (mode.Process, error) { + for { + r.mu.Lock() + if r.failedErr != nil { + err := r.failedErr + r.mu.Unlock() + return nil, err + } + if target := computeTarget(r.status, r.demand); target != want { + cur := r.mode + r.mu.Unlock() + return nil, fmt.Errorf("space is %s, target %s, want %s: %w", cur, target, want, ErrModeUnreachable) + } + if r.mode == want { + p := r.current + r.mu.Unlock() + return p, nil + } + ch := r.changed + r.mu.Unlock() + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-r.ctx.Done(): + return nil, ErrCtrlClosed + case <-ch: + } + } +} + +func (r *reconciler) close(ctx context.Context) { + r.mu.Lock() + r.closeCtx = ctx + r.mu.Unlock() + r.cancel() + <-r.done +} + +func (r *reconciler) wakeUp() { + select { + case r.wake <- struct{}{}: + default: + } +} + +// broadcastLocked must be called with r.mu held. +func (r *reconciler) broadcastLocked() { + close(r.changed) + r.changed = make(chan struct{}) +} + +func (r *reconciler) run() { + defer close(r.done) + for { + select { + case <-r.ctx.Done(): + r.teardown() + return + case <-r.wake: + r.reconcile() + } + } +} + +func (r *reconciler) teardown() { + r.mu.Lock() + cur := r.current + closeCtx := r.closeCtx + r.mu.Unlock() + if closeCtx == nil { + closeCtx = context.Background() + } + if cur != nil { + if err := cur.Close(closeCtx); err != nil { + r.log.Warn("close process on teardown", zap.Error(err)) + } + } + r.log.Debug("closed") +} + +// reconcile transitions toward the target until converged. Inputs are +// re-read after every transition (latest-wins); a failed transition parks the +// loop in failedErr until the next input change. +func (r *reconciler) reconcile() { + for { + if r.ctx.Err() != nil { + return + } + r.mu.Lock() + target := computeTarget(r.status, r.demand) + if target == r.mode || r.failedErr != nil { + r.mu.Unlock() + return + } + cur := r.current + curMode := r.mode + startGen := r.gen + r.mu.Unlock() + + r.log.Debug("transition", zap.Stringer("from", curMode), zap.Stringer("to", target)) + if err := cur.Close(r.ctx); err != nil { + r.log.Warn("close process", zap.Stringer("mode", curMode), zap.Error(err)) + } + next := r.factory.Process(target) + err := next.Start(r.ctx) + + r.mu.Lock() + parked := false + if err != nil { + r.log.Error("failed to start process", zap.Stringer("mode", target), zap.Error(err)) + // park only if no input changed while the transition ran; an input + // change that raced the failure must keep its effect, so the loop + // retries against the fresh inputs instead + if r.gen == startGen { + r.failedErr = err + parked = true + } + r.current = r.factory.Process(mode.ModeInitial) + r.mode = mode.ModeInitial + } else { + r.current = next + r.mode = target + } + r.broadcastLocked() + r.mu.Unlock() + if parked { + return + } + } +} diff --git a/space/internal/components/spaceoffloader/spaceoffloader.go b/space/internal/components/spaceoffloader/spaceoffloader.go index cb71d142f9..0096062588 100644 --- a/space/internal/components/spaceoffloader/spaceoffloader.go +++ b/space/internal/components/spaceoffloader/spaceoffloader.go @@ -25,7 +25,6 @@ import ( "github.com/anyproto/anytype-heart/space/deletioncontroller" "github.com/anyproto/anytype-heart/space/internal/components/dependencies" "github.com/anyproto/anytype-heart/space/internal/components/spacestatus" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" "github.com/anyproto/anytype-heart/space/spacecore/storage" "github.com/anyproto/anytype-heart/space/spaceinfo" ) @@ -88,13 +87,14 @@ func (o *spaceOffloader) Close(ctx context.Context) (err error) { if ol != nil { <-ol.loadCh } + // The offloading process closes only when the target moves away from + // offloading (the space is being restored) or at shutdown. Cancel the + // pending coordinator deletion so a restored space is not remote-deleted; + // at shutdown the in-memory queue dies anyway. + o.delController.RemoveSpaceToDelete(o.status.SpaceId()) return nil } -func (o *spaceOffloader) CanTransition(next mode.Mode) bool { - return false -} - func (o *spaceOffloader) onOffload(id string, offloadErr error) { if offloadErr != nil { log.Warn("offload error", zap.Error(offloadErr), zap.String("spaceId", id)) diff --git a/space/internal/marketplacespace/marketplace.go b/space/internal/marketplacespace/marketplace.go index c1e60fe600..a302c076da 100644 --- a/space/internal/marketplacespace/marketplace.go +++ b/space/internal/marketplacespace/marketplace.go @@ -61,6 +61,15 @@ func (s *spaceController) Mode() mode.Mode { return mode.ModeLoading } +func (s *spaceController) Demand() {} + +func (s *spaceController) WaitMode(ctx context.Context, m mode.Mode) error { + if m != mode.ModeLoading { + return fmt.Errorf("marketplace space is always loading, not %s", m) + } + return nil +} + func (s *spaceController) WaitLoad(context.Context) (sp clientspace.Space, err error) { s.reindexOnce.Do(func() { // TODO: GO-3557 Need to confirm moving ReindexMarketplaceSpace from Start to WaitLoad with mcrakhman @@ -72,10 +81,6 @@ func (s *spaceController) WaitLoad(context.Context) (sp clientspace.Space, err e return s.vs, nil } -func (s *spaceController) Current() any { - return s -} - func (s *spaceController) SpaceId() string { return addr.AnytypeMarketplaceWorkspace } diff --git a/space/internal/personalspace/personal.go b/space/internal/personalspace/personal.go deleted file mode 100644 index d41680d1cc..0000000000 --- a/space/internal/personalspace/personal.go +++ /dev/null @@ -1,219 +0,0 @@ -package personalspace - -import ( - "context" - "sync" - - "github.com/anyproto/any-sync/app" - "github.com/anyproto/any-sync/app/logger" - "go.uber.org/zap" - - "github.com/anyproto/anytype-heart/space/internal/components/personalmigration" - "github.com/anyproto/anytype-heart/space/internal/components/spacestatus" - "github.com/anyproto/anytype-heart/space/internal/spacecontroller" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/initial" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/loader" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/offloader" - "github.com/anyproto/anytype-heart/space/spacecore" - "github.com/anyproto/anytype-heart/space/spaceinfo" - "github.com/anyproto/anytype-heart/space/techspace" -) - -type Personal interface { - spacecontroller.SpaceController - WaitMigrations(ctx context.Context) error -} - -var log = logger.NewNamed("common.space.personalspace") - -type ctxKey int - -const SkipCheckSpaceViewKey ctxKey = iota - -func shouldCheckSpaceView(ctx context.Context) bool { - skip, ok := ctx.Value(SkipCheckSpaceViewKey).(bool) - return !ok || !skip -} - -func NewSpaceController(ctx context.Context, spaceId string, metadata []byte, a *app.App) (spacecontroller.SpaceController, error) { - techSpace := a.MustComponent(techspace.CName).(techspace.TechSpace) - spaceCore := a.MustComponent(spacecore.CName).(spacecore.SpaceCoreService) - var ( - exists bool - err error - ) - if shouldCheckSpaceView(ctx) { - exists, err = techSpace.SpaceViewExists(ctx, spaceId) - } - // This could happen for old accounts - if !exists || err != nil { - info := spaceinfo.NewSpacePersistentInfo(spaceId) - info.SetAccountStatus(spaceinfo.AccountStatusUnknown) - err = techSpace.SpaceViewCreate(ctx, spaceId, false, info, nil) - if err != nil { - return nil, err - } - } - newApp, err := makeStatusApp(a, spaceId) - if err != nil { - return nil, err - } - s := &spaceController{ - app: newApp, - spaceId: spaceId, - techSpace: techSpace, - status: newApp.MustComponent(spacestatus.CName).(spacestatus.SpaceStatus), - spaceCore: spaceCore, - metadata: metadata, - } - sm, err := mode.NewStateMachine(s, log.With(zap.String("spaceId", s.spaceId))) - if err != nil { - return nil, err - } - s.sm = sm - return s, nil -} - -func makeStatusApp(a *app.App, spaceId string) (*app.App, error) { - newApp := a.ChildApp() - newApp.Register(spacestatus.New(spaceId)) - err := newApp.Start(context.Background()) - if err != nil { - return nil, err - } - return newApp, nil -} - -type spaceController struct { - app *app.App - spaceId string - metadata []byte - lastUpdatedStatus spaceinfo.AccountStatus - - loader loader.Loader - spaceCore spacecore.SpaceCoreService - techSpace techspace.TechSpace - status spacestatus.SpaceStatus - - personalMigration personalmigration.Runner - - sm *mode.StateMachine - mx sync.Mutex -} - -func (s *spaceController) Start(ctx context.Context) (err error) { - switch s.status.GetPersistentStatus() { - case spaceinfo.AccountStatusDeleted: - _, err := s.sm.ChangeMode(mode.ModeOffloading) - return err - default: - _, err := s.sm.ChangeMode(mode.ModeLoading) - return err - } -} - -func (s *spaceController) Process(md mode.Mode) mode.Process { - switch md { - case mode.ModeInitial: - return initial.New() - case mode.ModeOffloading: - return offloader.New(s.app) - default: - return &personalLoader{ - newLoader: s.newLoader, - } - } -} - -func (s *spaceController) Mode() mode.Mode { - return s.sm.GetMode() -} - -func (s *spaceController) Current() any { - return s.sm.GetProcess() -} - -func (s *spaceController) SpaceId() string { - return s.spaceId -} - -func (s *spaceController) newLoader() loader.Loader { - s.mx.Lock() - s.personalMigration = personalmigration.New() - s.mx.Unlock() - return loader.New(s.app, loader.Params{ - SpaceId: s.spaceId, - IsPersonal: true, - OwnerMetadata: s.metadata, - AdditionalComps: []app.Component{ - s.personalMigration, - }, - }) -} - -func (s *spaceController) Update() error { - s.mx.Lock() - status := s.status.GetPersistentStatus() - if s.lastUpdatedStatus == status { - s.mx.Unlock() - return nil - } - s.lastUpdatedStatus = status - s.mx.Unlock() - updateStatus := func(mode mode.Mode) error { - _, err := s.sm.ChangeMode(mode) - return err - } - switch status { - case spaceinfo.AccountStatusDeleted: - return updateStatus(mode.ModeOffloading) - default: - return updateStatus(mode.ModeLoading) - } -} - -func (s *spaceController) SetPersistentInfo(ctx context.Context, info spaceinfo.SpacePersistentInfo) error { - err := s.status.SetPersistentInfo(info) - if err != nil { - return err - } - return s.Update() -} - -func (s *spaceController) SetLocalInfo(ctx context.Context, info spaceinfo.SpaceLocalInfo) error { - return s.status.SetLocalInfo(info) -} - -func (s *spaceController) Delete(ctx context.Context) error { - offloading, err := s.sm.ChangeMode(mode.ModeOffloading) - if err != nil { - return err - } - of := offloading.(offloader.Offloader) - return of.WaitOffload(ctx) -} - -func (s *spaceController) Close(ctx context.Context) error { - s.sm.Close() - // this closes status - return s.app.Close(ctx) -} - -func (s *spaceController) GetStatus() spaceinfo.AccountStatus { - return s.status.GetPersistentStatus() -} - -func (s *spaceController) GetLocalStatus() spaceinfo.LocalStatus { - return s.status.GetLocalStatus() -} - -func (s *spaceController) WaitMigrations(ctx context.Context) error { - s.mx.Lock() - if s.personalMigration == nil { - s.mx.Unlock() - return nil - } - s.mx.Unlock() - return s.personalMigration.WaitProfile(ctx) -} diff --git a/space/internal/personalspace/personalloader.go b/space/internal/personalspace/personalloader.go deleted file mode 100644 index 0ffabcacb3..0000000000 --- a/space/internal/personalspace/personalloader.go +++ /dev/null @@ -1,17 +0,0 @@ -package personalspace - -import ( - "context" - - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/loader" -) - -type personalLoader struct { - loader.Loader - newLoader func() loader.Loader -} - -func (p *personalLoader) Start(ctx context.Context) (err error) { - p.Loader = p.newLoader() - return p.Loader.Start(ctx) -} diff --git a/space/internal/shareablespace/shareable.go b/space/internal/shareablespace/shareable.go deleted file mode 100644 index 9ef5e77bdc..0000000000 --- a/space/internal/shareablespace/shareable.go +++ /dev/null @@ -1,180 +0,0 @@ -package shareablespace - -import ( - "context" - "sync" - - "github.com/anyproto/any-sync/app" - "github.com/anyproto/any-sync/app/logger" - "go.uber.org/zap" - - "github.com/anyproto/anytype-heart/space/deletioncontroller" - "github.com/anyproto/anytype-heart/space/internal/components/spacestatus" - "github.com/anyproto/anytype-heart/space/internal/spacecontroller" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/initial" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/joiner" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/loader" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/offloader" - "github.com/anyproto/anytype-heart/space/spaceinfo" -) - -var log = logger.NewNamed("common.space.shareablespace") - -type statusUpdater interface { - UpdateCoordinatorStatus() -} - -type spaceController struct { - spaceId string - app *app.App - status spacestatus.SpaceStatus - lastUpdatedStatus spaceinfo.AccountStatus - updater statusUpdater - mx sync.Mutex - - sm *mode.StateMachine -} - -func makeStatusApp(a *app.App, spaceId string) (*app.App, error) { - newApp := a.ChildApp() - newApp.Register(spacestatus.New(spaceId)) - err := newApp.Start(context.Background()) - if err != nil { - return nil, err - } - return newApp, nil -} - -func NewSpaceController( - spaceId string, - info spaceinfo.SpacePersistentInfo, - a *app.App) (spacecontroller.SpaceController, error) { - newApp, err := makeStatusApp(a, spaceId) - if err != nil { - return nil, err - } - s := &spaceController{ - spaceId: spaceId, - status: newApp.MustComponent(spacestatus.CName).(spacestatus.SpaceStatus), - lastUpdatedStatus: info.GetAccountStatus(), - app: newApp, - } - - // this is done for tests to not complicate them :-) - if updater, ok := a.Component(deletioncontroller.CName).(statusUpdater); ok { - s.updater = updater - } - sm, err := mode.NewStateMachine(s, log.With(zap.String("spaceId", spaceId))) - if err != nil { - return nil, err - } - s.sm = sm - return s, nil -} - -func (s *spaceController) SpaceId() string { - return s.spaceId -} - -func (s *spaceController) Start(ctx context.Context) error { - defer func() { - if s.updater != nil { - s.updater.UpdateCoordinatorStatus() - } - }() - switch s.status.GetPersistentStatus() { - case spaceinfo.AccountStatusDeleted: - _, err := s.sm.ChangeMode(mode.ModeOffloading) - return err - case spaceinfo.AccountStatusJoining: - _, err := s.sm.ChangeMode(mode.ModeJoining) - return err - case spaceinfo.AccountStatusRemoving: - _, err := s.sm.ChangeMode(mode.ModeOffloading) - return err - default: - _, err := s.sm.ChangeMode(mode.ModeLoading) - return err - } -} - -func (s *spaceController) Mode() mode.Mode { - return s.sm.GetMode() -} - -func (s *spaceController) Current() any { - return s.sm.GetProcess() -} - -func (s *spaceController) SetPersistentInfo(ctx context.Context, info spaceinfo.SpacePersistentInfo) error { - err := s.status.SetPersistentInfo(info) - if err != nil { - return err - } - return s.Update() -} - -func (s *spaceController) SetLocalInfo(ctx context.Context, info spaceinfo.SpaceLocalInfo) error { - return s.status.SetLocalInfo(info) -} - -func (s *spaceController) Update() error { - s.mx.Lock() - status := s.status.GetPersistentStatus() - if s.lastUpdatedStatus == status { - s.mx.Unlock() - return nil - } - s.lastUpdatedStatus = status - s.mx.Unlock() - updateStatus := func(mode mode.Mode) error { - _, err := s.sm.ChangeMode(mode) - return err - } - switch status { - case spaceinfo.AccountStatusDeleted: - return updateStatus(mode.ModeOffloading) - case spaceinfo.AccountStatusJoining: - return updateStatus(mode.ModeJoining) - case spaceinfo.AccountStatusRemoving: - return updateStatus(mode.ModeOffloading) - default: - return updateStatus(mode.ModeLoading) - } -} - -func (s *spaceController) Process(md mode.Mode) mode.Process { - switch md { - case mode.ModeInitial: - return initial.New() - case mode.ModeLoading: - return loader.New(s.app, loader.Params{ - SpaceId: s.spaceId, - }) - case mode.ModeOffloading: - return offloader.New(s.app) - case mode.ModeJoining: - return joiner.New(s.app, joiner.Params{ - SpaceId: s.spaceId, - Status: s.status, - Log: log, - }) - default: - return initial.New() - } -} - -func (s *spaceController) Close(ctx context.Context) error { - s.sm.Close() - // this closes status - return s.app.Close(ctx) -} - -func (s *spaceController) GetStatus() spaceinfo.AccountStatus { - return s.status.GetPersistentStatus() -} - -func (s *spaceController) GetLocalStatus() spaceinfo.LocalStatus { - return s.status.GetLocalStatus() -} diff --git a/space/internal/shareablespace/shareable_test.go b/space/internal/shareablespace/shareable_test.go deleted file mode 100644 index 53283a8dd7..0000000000 --- a/space/internal/shareablespace/shareable_test.go +++ /dev/null @@ -1,362 +0,0 @@ -package shareablespace - -import ( - "context" - "sync" - "sync/atomic" - "testing" - "time" - - "github.com/anyproto/any-sync/app" - "github.com/anyproto/any-sync/util/crypto" - "github.com/stretchr/testify/require" - - "github.com/anyproto/anytype-heart/pkg/lib/pb/model" - "github.com/anyproto/anytype-heart/space/internal/components/spacestatus" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/initial" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" - "github.com/anyproto/anytype-heart/space/spaceinfo" - "github.com/anyproto/anytype-heart/space/techspace" -) - -func TestSpaceController_InvitingLoading(t *testing.T) { - fx := newFixture(t, spaceinfo.AccountStatusJoining) - defer fx.stop() - err := fx.ctrl.Start(context.Background()) - require.NoError(t, err) - require.Equal(t, mode.ModeJoining, fx.ctrl.Mode()) - time.Sleep(100 * time.Millisecond) - fx.reg.Lock() - defer fx.reg.Unlock() - require.Equal(t, []mode.Mode{mode.ModeJoining, mode.ModeLoading}, fx.reg.modes) -} - -func TestSpaceController_LoadingDeleting(t *testing.T) { - fx := newFixture(t, spaceinfo.AccountStatusUnknown) - defer fx.stop() - err := fx.ctrl.Start(context.Background()) - require.NoError(t, err) - require.Equal(t, mode.ModeLoading, fx.ctrl.Mode()) - err = fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusDeleted)) - err = fx.ctrl.Update() - require.NoError(t, err) - fx.reg.Lock() - defer fx.reg.Unlock() - require.Equal(t, []mode.Mode{mode.ModeLoading, mode.ModeOffloading}, fx.reg.modes) -} - -func TestSpaceController_LoadingDeletingMultipleWaiters(t *testing.T) { - fx := newFixture(t, spaceinfo.AccountStatusUnknown) - defer fx.stop() - err := fx.ctrl.Start(context.Background()) - require.NoError(t, err) - require.Equal(t, mode.ModeLoading, fx.ctrl.Mode()) - wg := sync.WaitGroup{} - for i := 0; i < 10; i++ { - wg.Add(1) - go func() { - err := fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusDeleted)) - require.NoError(t, err) - wg.Done() - }() - } - wg.Wait() - fx.reg.Lock() - defer fx.reg.Unlock() - require.Equal(t, []mode.Mode{mode.ModeLoading, mode.ModeOffloading}, fx.reg.modes) -} - -func TestSpaceController_Deleting(t *testing.T) { - fx := newFixture(t, spaceinfo.AccountStatusDeleted) - defer fx.stop() - err := fx.ctrl.Start(context.Background()) - require.NoError(t, err) - require.Equal(t, mode.ModeOffloading, fx.ctrl.Mode()) - time.Sleep(100 * time.Millisecond) - fx.reg.Lock() - defer fx.reg.Unlock() - require.Equal(t, []mode.Mode{mode.ModeOffloading}, fx.reg.modes) -} - -func TestSpaceController_DeletingInvalid(t *testing.T) { - fx := newFixture(t, spaceinfo.AccountStatusDeleted) - defer fx.stop() - err := fx.ctrl.Start(context.Background()) - require.NoError(t, err) - require.Equal(t, mode.ModeOffloading, fx.ctrl.Mode()) - err = fx.ctrl.SetPersistentInfo(context.Background(), makePersistentInfo("spaceId", spaceinfo.AccountStatusActive)) - require.Error(t, err) - fx.reg.Lock() - defer fx.reg.Unlock() - require.Equal(t, []mode.Mode{mode.ModeOffloading}, fx.reg.modes) -} - -func makePersistentInfo(spaceId string, status spaceinfo.AccountStatus) spaceinfo.SpacePersistentInfo { - info := spaceinfo.NewSpacePersistentInfo(spaceId) - info.SetAccountStatus(status) - return info -} - -type modeRegister struct { - modes []mode.Mode - sync.Mutex -} - -func (m *modeRegister) register(mode mode.Mode) { - m.Lock() - m.modes = append(m.modes, mode) - m.Unlock() -} - -type spaceStatusStub struct { - spaceId string - localStatus spaceinfo.LocalStatus - remoteStatus spaceinfo.RemoteStatus - accountStatus spaceinfo.AccountStatus - persistentUpdater func(status spaceinfo.AccountStatus) - sync.Mutex -} - -func (s *spaceStatusStub) Init(a *app.App) (err error) { - return nil -} - -func (s *spaceStatusStub) Name() (name string) { - return spacestatus.CName -} - -func (s *spaceStatusStub) SpaceId() string { - return s.spaceId -} - -func (s *spaceStatusStub) GetLocalStatus() spaceinfo.LocalStatus { - s.Lock() - defer s.Unlock() - return s.localStatus -} - -func (s *spaceStatusStub) SetOwner(ownerIdentity string, createdDate int64) (err error) { - return -} - -func (s *spaceStatusStub) GetRemoteStatus() spaceinfo.RemoteStatus { - s.Lock() - defer s.Unlock() - return s.remoteStatus -} - -func (s *spaceStatusStub) GetPersistentStatus() spaceinfo.AccountStatus { - s.Lock() - defer s.Unlock() - return s.accountStatus -} - -func (s *spaceStatusStub) Run(ctx context.Context) (err error) { - return nil -} - -func (s *spaceStatusStub) Close(ctx context.Context) (err error) { - return nil -} - -func (s *spaceStatusStub) SetPersistentStatus(status spaceinfo.AccountStatus) (err error) { - s.Lock() - defer s.Unlock() - s.accountStatus = status - if s.persistentUpdater != nil { - s.persistentUpdater(status) - } - return nil -} - -func (s *spaceStatusStub) SetPersistentInfo(info spaceinfo.SpacePersistentInfo) (err error) { - s.Lock() - defer s.Unlock() - s.accountStatus = info.GetAccountStatus() - return -} - -func (s *spaceStatusStub) SetLocalStatus(status spaceinfo.LocalStatus) error { - s.Lock() - defer s.Unlock() - s.localStatus = status - return nil -} - -func (s *spaceStatusStub) SetLocalInfo(info spaceinfo.SpaceLocalInfo) (err error) { - s.Lock() - defer s.Unlock() - s.localStatus = info.GetLocalStatus() - return -} - -func (s *spaceStatusStub) SetAccessType(status spaceinfo.AccessType) (err error) { - return -} - -func (s *spaceStatusStub) SetAclInfo(isAclEmpty bool, pushKey crypto.PrivKey, pushEncryptionKey crypto.SymKey, spaceJoinedDate int64) (err error) { - return -} - -func (s *spaceStatusStub) GetLatestAclHeadId() string { - return "" -} - -func (s *spaceStatusStub) SetMyParticipantStatus(st model.ParticipantStatus) (err error) { - return nil -} - -func (s *spaceStatusStub) GetSpaceView() techspace.SpaceView { - return nil -} - -var _ spacestatus.SpaceStatus = (*spaceStatusStub)(nil) - -type inviting struct { - inviteReceived atomic.Bool - status spacestatus.SpaceStatus - reg *modeRegister -} - -func newInviting(status spacestatus.SpaceStatus, reg *modeRegister) mode.Process { - return &inviting{ - status: status, - reg: reg, - } -} - -func (i *inviting) Start(ctx context.Context) error { - go func() { - i.inviteReceived.Store(true) - _ = i.status.SetPersistentStatus(spaceinfo.AccountStatusActive) - }() - i.reg.register(mode.ModeJoining) - return nil -} - -func (i *inviting) Close(ctx context.Context) error { - return nil -} - -func (i *inviting) CanTransition(next mode.Mode) bool { - if next == mode.ModeLoading && !i.inviteReceived.Load() { - return false - } - return true -} - -type loading struct { - status spacestatus.SpaceStatus - reg *modeRegister -} - -func newLoading(status spacestatus.SpaceStatus, reg *modeRegister) mode.Process { - return &loading{ - status: status, - reg: reg, - } -} - -func (l *loading) Start(ctx context.Context) error { - l.reg.register(mode.ModeLoading) - return nil -} - -func (l *loading) Close(ctx context.Context) error { - return nil -} - -func (l *loading) CanTransition(next mode.Mode) bool { - return true -} - -type offloading struct { - status spacestatus.SpaceStatus - reg *modeRegister -} - -func newOffloading(status spacestatus.SpaceStatus, reg *modeRegister) mode.Process { - return &offloading{ - status: status, - reg: reg, - } -} - -func (l *offloading) Start(ctx context.Context) error { - l.reg.register(mode.ModeOffloading) - return nil -} - -func (l *offloading) Close(ctx context.Context) error { - return nil -} - -func (l *offloading) CanTransition(next mode.Mode) bool { - return false -} - -type factory struct { - status spacestatus.SpaceStatus - reg *modeRegister -} - -func (f factory) Process(md mode.Mode) mode.Process { - switch md { - case mode.ModeInitial: - return initial.New() - case mode.ModeJoining: - return newInviting(f.status, f.reg) - case mode.ModeLoading: - return newLoading(f.status, f.reg) - case mode.ModeOffloading: - return newOffloading(f.status, f.reg) - default: - panic("unhandled default case") - } -} - -type fixture struct { - f factory - s *spaceStatusStub - ctrl *spaceController - reg *modeRegister -} - -func newFixture(t *testing.T, startStatus spaceinfo.AccountStatus) *fixture { - reg := &modeRegister{} - s := &spaceStatusStub{ - spaceId: "spaceId", - accountStatus: startStatus, - } - f := factory{ - status: s, - reg: reg, - } - sm, err := mode.NewStateMachine(f, log) - require.NoError(t, err) - controller := &spaceController{ - spaceId: "spaceId", - status: s, - app: &app.App{}, - lastUpdatedStatus: startStatus, - sm: sm, - } - s.persistentUpdater = func(status spaceinfo.AccountStatus) { - go func() { - err := controller.Update() - require.NoError(t, err) - }() - } - return &fixture{ - f: factory{ - status: s, - }, - s: s, - ctrl: controller, - reg: reg, - } -} - -func (fx *fixture) stop() { - fx.ctrl.sm.Close() -} diff --git a/space/internal/spacecontroller/mock_spacecontroller/mock_SpaceController.go b/space/internal/spacecontroller/mock_spacecontroller/mock_SpaceController.go index 1d498c6ae1..c4517d7ac8 100644 --- a/space/internal/spacecontroller/mock_spacecontroller/mock_SpaceController.go +++ b/space/internal/spacecontroller/mock_spacecontroller/mock_SpaceController.go @@ -5,9 +5,12 @@ package mock_spacecontroller import ( context "context" - mode "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" + clientspace "github.com/anyproto/anytype-heart/space/clientspace" + mock "github.com/stretchr/testify/mock" + mode "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" + spaceinfo "github.com/anyproto/anytype-heart/space/spaceinfo" ) @@ -70,50 +73,35 @@ func (_c *MockSpaceController_Close_Call) RunAndReturn(run func(context.Context) return _c } -// Current provides a mock function with no fields -func (_m *MockSpaceController) Current() interface{} { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for Current") - } - - var r0 interface{} - if rf, ok := ret.Get(0).(func() interface{}); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(interface{}) - } - } - - return r0 +// Demand provides a mock function with no fields +func (_m *MockSpaceController) Demand() { + _m.Called() } -// MockSpaceController_Current_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Current' -type MockSpaceController_Current_Call struct { +// MockSpaceController_Demand_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Demand' +type MockSpaceController_Demand_Call struct { *mock.Call } -// Current is a helper method to define mock.On call -func (_e *MockSpaceController_Expecter) Current() *MockSpaceController_Current_Call { - return &MockSpaceController_Current_Call{Call: _e.mock.On("Current")} +// Demand is a helper method to define mock.On call +func (_e *MockSpaceController_Expecter) Demand() *MockSpaceController_Demand_Call { + return &MockSpaceController_Demand_Call{Call: _e.mock.On("Demand")} } -func (_c *MockSpaceController_Current_Call) Run(run func()) *MockSpaceController_Current_Call { +func (_c *MockSpaceController_Demand_Call) Run(run func()) *MockSpaceController_Demand_Call { _c.Call.Run(func(args mock.Arguments) { run() }) return _c } -func (_c *MockSpaceController_Current_Call) Return(_a0 interface{}) *MockSpaceController_Current_Call { - _c.Call.Return(_a0) +func (_c *MockSpaceController_Demand_Call) Return() *MockSpaceController_Demand_Call { + _c.Call.Return() return _c } -func (_c *MockSpaceController_Current_Call) RunAndReturn(run func() interface{}) *MockSpaceController_Current_Call { - _c.Call.Return(run) +func (_c *MockSpaceController_Demand_Call) RunAndReturn(run func()) *MockSpaceController_Demand_Call { + _c.Run(run) return _c } @@ -482,6 +470,111 @@ func (_c *MockSpaceController_Update_Call) RunAndReturn(run func() error) *MockS return _c } +// WaitLoad provides a mock function with given fields: ctx +func (_m *MockSpaceController) WaitLoad(ctx context.Context) (clientspace.Space, error) { + ret := _m.Called(ctx) + + if len(ret) == 0 { + panic("no return value specified for WaitLoad") + } + + var r0 clientspace.Space + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) (clientspace.Space, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) clientspace.Space); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(clientspace.Space) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// MockSpaceController_WaitLoad_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WaitLoad' +type MockSpaceController_WaitLoad_Call struct { + *mock.Call +} + +// WaitLoad is a helper method to define mock.On call +// - ctx context.Context +func (_e *MockSpaceController_Expecter) WaitLoad(ctx interface{}) *MockSpaceController_WaitLoad_Call { + return &MockSpaceController_WaitLoad_Call{Call: _e.mock.On("WaitLoad", ctx)} +} + +func (_c *MockSpaceController_WaitLoad_Call) Run(run func(ctx context.Context)) *MockSpaceController_WaitLoad_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context)) + }) + return _c +} + +func (_c *MockSpaceController_WaitLoad_Call) Return(_a0 clientspace.Space, _a1 error) *MockSpaceController_WaitLoad_Call { + _c.Call.Return(_a0, _a1) + return _c +} + +func (_c *MockSpaceController_WaitLoad_Call) RunAndReturn(run func(context.Context) (clientspace.Space, error)) *MockSpaceController_WaitLoad_Call { + _c.Call.Return(run) + return _c +} + +// WaitMode provides a mock function with given fields: ctx, m +func (_m *MockSpaceController) WaitMode(ctx context.Context, m mode.Mode) error { + ret := _m.Called(ctx, m) + + if len(ret) == 0 { + panic("no return value specified for WaitMode") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, mode.Mode) error); ok { + r0 = rf(ctx, m) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// MockSpaceController_WaitMode_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'WaitMode' +type MockSpaceController_WaitMode_Call struct { + *mock.Call +} + +// WaitMode is a helper method to define mock.On call +// - ctx context.Context +// - m mode.Mode +func (_e *MockSpaceController_Expecter) WaitMode(ctx interface{}, m interface{}) *MockSpaceController_WaitMode_Call { + return &MockSpaceController_WaitMode_Call{Call: _e.mock.On("WaitMode", ctx, m)} +} + +func (_c *MockSpaceController_WaitMode_Call) Run(run func(ctx context.Context, m mode.Mode)) *MockSpaceController_WaitMode_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(mode.Mode)) + }) + return _c +} + +func (_c *MockSpaceController_WaitMode_Call) Return(_a0 error) *MockSpaceController_WaitMode_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *MockSpaceController_WaitMode_Call) RunAndReturn(run func(context.Context, mode.Mode) error) *MockSpaceController_WaitMode_Call { + _c.Call.Return(run) + return _c +} + // NewMockSpaceController creates a new instance of MockSpaceController. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewMockSpaceController(t interface { diff --git a/space/internal/spacecontroller/spacecontroller.go b/space/internal/spacecontroller/spacecontroller.go index e3e2bb12de..fa158f60fa 100644 --- a/space/internal/spacecontroller/spacecontroller.go +++ b/space/internal/spacecontroller/spacecontroller.go @@ -3,15 +3,28 @@ package spacecontroller import ( "context" + "github.com/anyproto/anytype-heart/space/clientspace" "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" "github.com/anyproto/anytype-heart/space/spaceinfo" ) type SpaceController interface { SpaceId() string + // Start demands the space and blocks until its lifecycle process is + // running (loading/joining/offloading started). Start(ctx context.Context) error + // Demand marks the space as wanted-loaded without blocking. The + // controller loads it in the background unless its status dictates + // offloading/joining. + Demand() + // WaitLoad demands the space and blocks until it is fully loaded, + // returning the real load error on failure. + WaitLoad(ctx context.Context) (clientspace.Space, error) + // WaitMode blocks until the lifecycle process for mode m is running, + // surfacing the real transition error on failure. It fails fast when m is + // not the controller's current target. + WaitMode(ctx context.Context, m mode.Mode) error Mode() mode.Mode - Current() any Update() error SetPersistentInfo(ctx context.Context, info spaceinfo.SpacePersistentInfo) error SetLocalInfo(ctx context.Context, status spaceinfo.SpaceLocalInfo) error diff --git a/space/internal/spaceprocess/initial/initial.go b/space/internal/spaceprocess/initial/initial.go index 266a9e6937..3eeb29371b 100644 --- a/space/internal/spaceprocess/initial/initial.go +++ b/space/internal/spaceprocess/initial/initial.go @@ -20,7 +20,3 @@ func (i *initial) Start(ctx context.Context) error { func (i *initial) Close(ctx context.Context) error { return nil } - -func (i *initial) CanTransition(next mode.Mode) bool { - return true -} diff --git a/space/internal/spaceprocess/joiner/joiner.go b/space/internal/spaceprocess/joiner/joiner.go index d86a659a16..9b0be7bf4c 100644 --- a/space/internal/spaceprocess/joiner/joiner.go +++ b/space/internal/spaceprocess/joiner/joiner.go @@ -81,7 +81,3 @@ func (i *joiner) Start(ctx context.Context) error { func (i *joiner) Close(ctx context.Context) error { return i.app.Close(ctx) } - -func (i *joiner) CanTransition(next mode.Mode) bool { - return true -} diff --git a/space/internal/spaceprocess/loader/loader.go b/space/internal/spaceprocess/loader/loader.go index 5631c8a64c..1c165fff67 100644 --- a/space/internal/spaceprocess/loader/loader.go +++ b/space/internal/spaceprocess/loader/loader.go @@ -61,10 +61,6 @@ func (l *loader) Close(ctx context.Context) error { return l.app.Close(ctx) } -func (l *loader) CanTransition(next mode.Mode) bool { - return true -} - // wait load starts this spaceloader sub app component func (l *loader) WaitLoad(ctx context.Context) (sp clientspace.Space, err error) { spaceLoader := app.MustComponent[spaceloader.SpaceLoader](l.app) diff --git a/space/internal/spaceprocess/mode/mode.go b/space/internal/spaceprocess/mode/mode.go new file mode 100644 index 0000000000..4a769de53f --- /dev/null +++ b/space/internal/spaceprocess/mode/mode.go @@ -0,0 +1,42 @@ +// Package mode defines the lifecycle states of a space and the Process +// interface implemented by per-state component bundles (loader, joiner, +// offloader). Orchestration lives in the accountspace reconciler. +package mode + +import ( + "context" +) + +type Mode int + +const ( + ModeUnknown Mode = iota + // ModeInitial is the dormant state: the controller is registered but no + // process is running. + ModeInitial + ModeLoading + ModeOffloading + ModeJoining +) + +func (m Mode) String() string { + switch m { + case ModeInitial: + return "initial" + case ModeLoading: + return "loading" + case ModeOffloading: + return "offloading" + case ModeJoining: + return "joining" + } + return "unknown" +} + +// Process is one running lifecycle phase of a space, implemented as a child +// app bundle. Start and Close are always called from the reconciler goroutine +// of the owning space controller. +type Process interface { + Start(ctx context.Context) error + Close(ctx context.Context) error +} diff --git a/space/internal/spaceprocess/mode/statemachine.go b/space/internal/spaceprocess/mode/statemachine.go deleted file mode 100644 index 990f8ed157..0000000000 --- a/space/internal/spaceprocess/mode/statemachine.go +++ /dev/null @@ -1,198 +0,0 @@ -package mode - -import ( - "context" - "errors" - "sync" - - "github.com/anyproto/any-sync/app/logger" - "go.uber.org/zap" -) - -type Mode int - -const ( - ModeUnknown Mode = iota - ModeInitial - ModeLoading - ModeOffloading - ModeJoining -) - -type WaitResult struct { - Result Process - Error error -} - -type Process interface { - Start(ctx context.Context) error - Close(ctx context.Context) error - CanTransition(next Mode) bool -} - -var ( - ErrInvalidTransition = errors.New("invalid transition") - ErrTransitionInProcess = errors.New("transition in process") - ErrFailedToStart = errors.New("failed to start") -) - -type ProcessFactory interface { - Process(mode Mode) Process -} - -type waiter chan Process - -type StateMachine struct { - sync.Mutex - current Process - mode Mode - next Mode - waiters []waiter - factory ProcessFactory - ctx context.Context - cancel context.CancelFunc - doneCh chan struct{} - notify chan struct{} - log logger.CtxLogger -} - -func NewStateMachine(factory ProcessFactory, log logger.CtxLogger) (*StateMachine, error) { - ctx, cancel := context.WithCancel(context.Background()) - machine := &StateMachine{ - mode: ModeInitial, - next: ModeUnknown, - doneCh: make(chan struct{}), - factory: factory, - ctx: ctx, - cancel: cancel, - current: factory.Process(ModeInitial), - notify: make(chan struct{}, 1), - log: log, - } - err := machine.current.Start(machine.ctx) - if err != nil { - return nil, err - } - machine.Run() - return machine, err -} - -func (s *StateMachine) Run() { - go s.loop() -} - -func (s *StateMachine) Close() { - s.cancel() - <-s.doneCh -} - -func (s *StateMachine) GetMode() Mode { - s.Lock() - defer s.Unlock() - return s.mode -} - -func (s *StateMachine) GetProcess() Process { - s.Lock() - defer s.Unlock() - return s.current -} - -func (s *StateMachine) ChangeMode(next Mode) (proc Process, err error) { - s.log.Debug("changing", zap.Int("next", int(next))) - s.Lock() - if s.mode == next { - proc = s.current - s.Unlock() - return - } - if s.next != next && s.next != ModeUnknown { - s.Unlock() - return nil, ErrTransitionInProcess - } - if !s.current.CanTransition(next) { - s.Unlock() - return nil, ErrInvalidTransition - } - if s.next == ModeUnknown { - s.notifyChange() - } - s.next = next - wait := make(waiter) - s.waiters = append(s.waiters, wait) - s.Unlock() - s.log.Debug("notify next", zap.Int("next", int(next))) - // TODO: [MR] send error to waiter - proc = <-wait - if proc == nil { - return nil, ErrFailedToStart - } - return -} - -func (s *StateMachine) notifyChange() { - select { - case s.notify <- struct{}{}: - default: - } -} - -func (s *StateMachine) loop() { - for { - select { - case <-s.ctx.Done(): - s.Lock() - cur := s.current - ch := s.doneCh - mode := s.mode - s.Unlock() - if cur != nil { - cur.Close(s.ctx) - } - s.log.Debug("closed", zap.Int("mode", int(mode))) - close(ch) - return - case <-s.notify: - s.Lock() - cur := s.current - mode := s.mode - next := s.next - s.Unlock() - s.log.Debug("closing", zap.Int("mode", int(mode))) - cur.Close(s.ctx) - - cur = s.factory.Process(next) - s.log.Debug("starting", zap.Int("mode", int(next))) - err := cur.Start(s.ctx) - if err != nil { - s.log.Error("failed to start", zap.Error(err)) - s.Lock() - s.next = ModeUnknown - s.mode = ModeInitial - s.current = s.factory.Process(ModeInitial) - // Initial should always start - err := s.current.Start(s.ctx) - if err != nil { - s.log.Error("failed to start initial", zap.Error(err)) - } - waiters := append([]waiter{}, s.waiters...) - s.waiters = nil - s.Unlock() - for _, w := range waiters { - w <- nil - } - break - } - s.Lock() - s.mode = s.next - s.next = ModeUnknown - s.current = cur - waiters := append([]waiter{}, s.waiters...) - s.waiters = nil - s.Unlock() - for _, w := range waiters { - w <- cur - } - } - } -} diff --git a/space/internal/spaceprocess/offloader/offloader.go b/space/internal/spaceprocess/offloader/offloader.go index e0a4efb4b4..f12b307274 100644 --- a/space/internal/spaceprocess/offloader/offloader.go +++ b/space/internal/spaceprocess/offloader/offloader.go @@ -37,10 +37,6 @@ func (o *offloader) Start(ctx context.Context) error { return o.app.Start(ctx) } -func (o *offloader) CanTransition(next mode.Mode) bool { - return true -} - func (o *offloader) WaitOffload(ctx context.Context) error { return o.spaceOffloader.WaitOffload(ctx) } diff --git a/space/internal/streamablespace/streamablespace.go b/space/internal/streamablespace/streamablespace.go deleted file mode 100644 index 7ace084717..0000000000 --- a/space/internal/streamablespace/streamablespace.go +++ /dev/null @@ -1,176 +0,0 @@ -package streamablespace - -import ( - "context" - "sync" - - "github.com/anyproto/any-sync/app" - "github.com/anyproto/any-sync/app/logger" - "github.com/anyproto/any-sync/util/crypto" - "go.uber.org/zap" - - "github.com/anyproto/anytype-heart/space/internal/components/spacestatus" - "github.com/anyproto/anytype-heart/space/internal/spacecontroller" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/initial" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/loader" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/offloader" - "github.com/anyproto/anytype-heart/space/spacecore" - "github.com/anyproto/anytype-heart/space/spaceinfo" - "github.com/anyproto/anytype-heart/space/techspace" -) - -type Personal interface { - spacecontroller.SpaceController - WaitMigrations(ctx context.Context) error -} - -var log = logger.NewNamed("common.space.streamablespace") - -func NewSpaceController(ctx context.Context, spaceId string, privKey crypto.PrivKey, metadata []byte, a *app.App) (spacecontroller.SpaceController, error) { - techSpace := a.MustComponent(techspace.CName).(techspace.TechSpace) - spaceCore := a.MustComponent(spacecore.CName).(spacecore.SpaceCoreService) - newApp, err := makeStatusApp(a, spaceId) - if err != nil { - return nil, err - } - s := &spaceController{ - app: newApp, - spaceId: spaceId, - techSpace: techSpace, - status: newApp.MustComponent(spacestatus.CName).(spacestatus.SpaceStatus), - spaceCore: spaceCore, - guestKey: privKey, - metadata: metadata, - } - sm, err := mode.NewStateMachine(s, log.With(zap.String("spaceId", s.spaceId))) - if err != nil { - return nil, err - } - s.sm = sm - return s, nil -} - -func makeStatusApp(a *app.App, spaceId string) (*app.App, error) { - newApp := a.ChildApp() - newApp.Register(spacestatus.New(spaceId)) - err := newApp.Start(context.Background()) - if err != nil { - return nil, err - } - return newApp, nil -} - -type spaceController struct { - app *app.App - spaceId string - metadata []byte - lastUpdatedStatus spaceinfo.AccountStatus - - loader loader.Loader - spaceCore spacecore.SpaceCoreService - techSpace techspace.TechSpace - status spacestatus.SpaceStatus - - guestKey crypto.PrivKey - sm *mode.StateMachine - mx sync.Mutex -} - -func (s *spaceController) Start(ctx context.Context) (err error) { - switch s.status.GetPersistentStatus() { - case spaceinfo.AccountStatusDeleted: - _, err := s.sm.ChangeMode(mode.ModeOffloading) - return err - default: - _, err := s.sm.ChangeMode(mode.ModeLoading) - return err - } -} - -func (s *spaceController) Process(md mode.Mode) mode.Process { - switch md { - case mode.ModeInitial: - return initial.New() - case mode.ModeOffloading: - return offloader.New(s.app) - default: - return s.newLoader() - } -} - -func (s *spaceController) Mode() mode.Mode { - return s.sm.GetMode() -} - -func (s *spaceController) Current() any { - return s.sm.GetProcess() -} - -func (s *spaceController) SpaceId() string { - return s.spaceId -} - -func (s *spaceController) newLoader() loader.Loader { - return loader.New(s.app, loader.Params{ - SpaceId: s.spaceId, - OwnerMetadata: s.metadata, - GuestKey: s.guestKey, - }) -} - -func (s *spaceController) Update() error { - s.mx.Lock() - status := s.status.GetPersistentStatus() - if s.lastUpdatedStatus == status { - s.mx.Unlock() - return nil - } - s.lastUpdatedStatus = status - s.mx.Unlock() - updateStatus := func(mode mode.Mode) error { - _, err := s.sm.ChangeMode(mode) - return err - } - switch status { - case spaceinfo.AccountStatusDeleted: - return updateStatus(mode.ModeOffloading) - default: - return updateStatus(mode.ModeLoading) - } -} - -func (s *spaceController) SetPersistentInfo(ctx context.Context, info spaceinfo.SpacePersistentInfo) error { - err := s.status.SetPersistentInfo(info) - if err != nil { - return err - } - return s.Update() -} - -func (s *spaceController) SetLocalInfo(ctx context.Context, info spaceinfo.SpaceLocalInfo) error { - return s.status.SetLocalInfo(info) -} - -func (s *spaceController) Delete(ctx context.Context) error { - offloading, err := s.sm.ChangeMode(mode.ModeOffloading) - if err != nil { - return err - } - of := offloading.(offloader.Offloader) - return of.WaitOffload(ctx) -} - -func (s *spaceController) Close(ctx context.Context) error { - s.sm.Close() - // this closes status - return s.app.Close(ctx) -} - -func (s *spaceController) GetStatus() spaceinfo.AccountStatus { - return s.status.GetPersistentStatus() -} - -func (s *spaceController) GetLocalStatus() spaceinfo.LocalStatus { - return s.status.GetLocalStatus() -} diff --git a/space/join.go b/space/join.go index d5204ae172..4294c2c8c2 100644 --- a/space/join.go +++ b/space/join.go @@ -2,95 +2,102 @@ package space import ( "context" + "errors" + "fmt" + "github.com/anyproto/anytype-heart/space/internal/accountspace" + "github.com/anyproto/anytype-heart/space/internal/spacecontroller" "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" "github.com/anyproto/anytype-heart/space/spaceinfo" + "github.com/anyproto/anytype-heart/space/techspace" ) +// Join is unidirectional: it writes the desired state (Joining + aclHeadId) +// into the space view and waits for the watcher-registered controller to run +// the joiner. Controllers are never constructed here. func (s *service) Join(ctx context.Context, id, aclHeadId string) error { - // TODO: refactor using unidirectional model where we change/create space view and it asynchronously starts controller - s.mu.Lock() - waiter, exists := s.waiting[id] - if exists { - s.mu.Unlock() - <-waiter.wait - if waiter.err != nil { - return waiter.err - } - s.mu.Lock() - ctrl := s.spaceControllers[id] - s.mu.Unlock() - if ctrl.Mode() != mode.ModeJoining { - info := spaceinfo.NewSpacePersistentInfo(id) - info.SetAclHeadId(aclHeadId).SetAccountStatus(spaceinfo.AccountStatusJoining) - return ctrl.SetPersistentInfo(ctx, info) - } - return nil + if s.isClosing.Load() { + return ErrSpaceIsClosing } - wait := make(chan struct{}) - s.waiting[id] = controllerWaiter{ - wait: wait, + info := spaceinfo.NewSpacePersistentInfo(id) + info.SetAclHeadId(aclHeadId).SetAccountStatus(spaceinfo.AccountStatusJoining) + exists, err := s.ensureSpaceView(ctx, id, info) + if err != nil { + return err } - s.mu.Unlock() - ctrl, err := s.factory.CreateInvitingSpace(ctx, id, aclHeadId) + ctrl, err := s.waitCtrl(ctx, id) if err != nil { - s.mu.Lock() - close(wait) - s.waiting[id] = controllerWaiter{ - wait: wait, - err: err, - } - s.mu.Unlock() return err } - s.mu.Lock() - close(wait) - s.spaceControllers[ctrl.SpaceId()] = ctrl - s.mu.Unlock() - return nil + // keep the space loaded after the join completes, also in lazy mode + ctrl.Demand() + if exists && ctrl.Mode() != mode.ModeJoining { + if err := ctrl.SetPersistentInfo(ctx, info); err != nil { + return err + } + } + return s.waitIntentMode(ctx, ctrl, mode.ModeJoining) } +// InviteJoin activates a space joined through a no-approval invite: write the +// Active status into the space view and wait for the controller to start +// loading. func (s *service) InviteJoin(ctx context.Context, id, aclHeadId string) error { - // TODO: refactor using unidirectional model where we change/create space view and it asynchronously starts controller - s.mu.Lock() - waiter, exists := s.waiting[id] + if s.isClosing.Load() { + return ErrSpaceIsClosing + } + info := spaceinfo.NewSpacePersistentInfo(id) + info.SetAclHeadId(aclHeadId).SetAccountStatus(spaceinfo.AccountStatusActive) + exists, err := s.ensureSpaceView(ctx, id, info) + if err != nil { + return err + } + ctrl, err := s.waitCtrl(ctx, id) + if err != nil { + return err + } + ctrl.Demand() if exists { - s.mu.Unlock() - <-waiter.wait - if waiter.err != nil { - return waiter.err + if err := ctrl.SetPersistentInfo(ctx, info); err != nil { + return err } - s.mu.Lock() - ctrl := s.spaceControllers[id] - s.mu.Unlock() - if ctrl.Mode() != mode.ModeLoading { - info := spaceinfo.NewSpacePersistentInfo(id) - info.SetAclHeadId(aclHeadId).SetAccountStatus(spaceinfo.AccountStatusActive) - return ctrl.SetPersistentInfo(ctx, info) - } - return nil } - wait := make(chan struct{}) - s.waiting[id] = controllerWaiter{ - wait: wait, - } - s.mu.Unlock() - ctrl, err := s.factory.CreateActiveSpace(ctx, id, aclHeadId) + return s.waitIntentMode(ctx, ctrl, mode.ModeLoading) +} + +// ensureSpaceView creates the space view with the given info if it does not +// exist. Returns whether the view already existed; a creation race +// (ErrSpaceViewExists) counts as existing so the caller still writes its +// intent into the view. +func (s *service) ensureSpaceView(ctx context.Context, id string, info spaceinfo.SpacePersistentInfo) (exists bool, err error) { + exists, err = s.techSpace.SpaceViewExists(ctx, id) if err != nil { - s.mu.Lock() - close(wait) - s.waiting[id] = controllerWaiter{ - wait: wait, - err: err, + return false, fmt.Errorf("check space view: %w", err) + } + if exists { + return true, nil + } + if err := s.techSpace.SpaceViewCreate(ctx, id, true, info, nil); err != nil { + if errors.Is(err, techspace.ErrSpaceViewExists) { + return true, nil } - s.mu.Unlock() - return err + return false, fmt.Errorf("create space view: %w", err) + } + return false, nil +} + +// waitIntentMode waits until the controller runs the process the intent asked +// for, surfacing real start errors. ErrModeUnreachable means the status moved +// on (e.g. the join was already accepted), which is success for the intent. +func (s *service) waitIntentMode(ctx context.Context, ctrl spacecontroller.SpaceController, m mode.Mode) error { + err := ctrl.WaitMode(ctx, m) + switch { + case err == nil, errors.Is(err, accountspace.ErrModeUnreachable): + return nil + case errors.Is(err, accountspace.ErrCtrlClosed): + return ErrSpaceIsClosing } - s.mu.Lock() - close(wait) - s.spaceControllers[ctrl.SpaceId()] = ctrl - s.mu.Unlock() - return nil + return err } func (s *service) CancelLeave(ctx context.Context, id string) error { diff --git a/space/load.go b/space/load.go index ba1f58649c..59e1d59847 100644 --- a/space/load.go +++ b/space/load.go @@ -7,76 +7,112 @@ import ( "github.com/anyproto/any-sync/commonspace/spacestorage" "github.com/anyproto/any-sync/commonspace/spacesyncproto" + "go.uber.org/zap" "github.com/anyproto/anytype-heart/space/clientspace" + "github.com/anyproto/anytype-heart/space/internal/accountspace" "github.com/anyproto/anytype-heart/space/internal/spacecontroller" - "github.com/anyproto/anytype-heart/space/internal/spaceprocess/loader" "github.com/anyproto/anytype-heart/space/spaceinfo" ) -type controllerWaiter struct { - wait chan struct{} - err error +// registerCtrl inserts a controller into the registry and wakes everyone +// blocked in waitCtrl. +func (s *service) registerCtrl(id string, ctrl spacecontroller.SpaceController) { + s.mu.Lock() + s.spaceControllers[id] = ctrl + s.registryChangedLocked() + s.mu.Unlock() } -func (s *service) getCtrl(ctx context.Context, spaceId string) (ctrl spacecontroller.SpaceController, err error) { - // Lazy multi-space loading: build the space now if it was deferred at - // startup. Idempotent / no-op if already started or in eager mode. - s.ensureSpaceStarted(spaceId) - s.mu.Lock() - if ctrl, ok := s.spaceControllers[spaceId]; ok { - s.mu.Unlock() - return ctrl, nil - } - if w, ok := s.waiting[spaceId]; ok { - s.mu.Unlock() - select { - case <-w.wait: - case <-ctx.Done(): - return nil, ctx.Err() - } +// registryChangedLocked broadcasts a registry change; must be called with +// s.mu held. +func (s *service) registryChangedLocked() { + close(s.regChanged) + s.regChanged = make(chan struct{}) +} + +// waitCtrl blocks until a controller for the space is registered (the watcher +// registers one for every space view). It returns the last registration error +// for the id, if any; the error is cleared on the next registration attempt, +// so a transient failure does not poison the id for the session. +func (s *service) waitCtrl(ctx context.Context, id string) (spacecontroller.SpaceController, error) { + for { s.mu.Lock() - err := s.waiting[spaceId].err - if err != nil { + if ctrl, ok := s.spaceControllers[id]; ok { + s.mu.Unlock() + return ctrl, nil + } + if err, ok := s.regErr[id]; ok { s.mu.Unlock() return nil, err } - ctrl := s.spaceControllers[spaceId] + ch := s.regChanged s.mu.Unlock() - return ctrl, nil + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-s.ctx.Done(): + return nil, ErrSpaceIsClosing + case <-ch: + } } - s.mu.Unlock() - return nil, ErrSpaceNotExists } -func (s *service) startStatus(ctx context.Context, info spaceinfo.SpacePersistentInfo) (ctrl spacecontroller.SpaceController, err error) { +// getCtrl returns the controller for the space without blocking on +// registration: a plain map lookup (plus the last registration error). Wait +// is the blocking variant. +func (s *service) getCtrl(spaceId string) (ctrl spacecontroller.SpaceController, err error) { s.mu.Lock() - if ctrl, ok := s.spaceControllers[info.SpaceID]; ok { - s.mu.Unlock() + defer s.mu.Unlock() + if ctrl, ok := s.spaceControllers[spaceId]; ok { return ctrl, nil } - if w, ok := s.waiting[info.SpaceID]; ok { + if err, ok := s.regErr[spaceId]; ok { + return nil, err + } + return nil, ErrSpaceNotExists +} + +// startStatus registers a controller for the space (idempotently and +// single-flight per id: a concurrent call for the same id waits for the +// first). Whether the space also starts loading is the lazy-mode demand +// decision: eager mode and the preferred space demand immediately; other +// spaces stay dormant until released or fetched via Get/Wait. Status-driven +// work (offloading, joining) proceeds regardless of demand. +func (s *service) startStatus(ctx context.Context, info spaceinfo.SpacePersistentInfo) (ctrl spacecontroller.SpaceController, err error) { + for { + s.mu.Lock() + if ctrl, ok := s.spaceControllers[info.SpaceID]; ok { + s.mu.Unlock() + return ctrl, nil + } + building, inFlight := s.constructing[info.SpaceID] + if !inFlight { + break // still holding s.mu + } s.mu.Unlock() select { - case <-w.wait: case <-ctx.Done(): return nil, ctx.Err() + case <-s.ctx.Done(): + return nil, ErrSpaceIsClosing + case <-building: } - s.mu.Lock() - err := s.waiting[info.SpaceID].err - if err != nil { - s.mu.Unlock() - return nil, err - } - ctrl := s.spaceControllers[info.SpaceID] - s.mu.Unlock() - return ctrl, nil - } - wait := make(chan struct{}) - s.waiting[info.SpaceID] = controllerWaiter{ - wait: wait, + // re-check: either the controller is registered now or the attempt + // failed and we become the next attempt } + building := make(chan struct{}) + s.constructing[info.SpaceID] = building + // a new attempt supersedes a previous failure + delete(s.regErr, info.SpaceID) + demandNow := !s.lazyMode || s.released || info.SpaceID == s.preferredSpaceId s.mu.Unlock() + defer func() { + s.mu.Lock() + delete(s.constructing, info.SpaceID) + close(building) + s.mu.Unlock() + }() if info.SpaceID == s.personalSpaceId { ctrl, err = s.factory.NewPersonalSpace(ctx, s.accountMetadataPayload) } else if info.EncodedKey == "" { @@ -84,30 +120,55 @@ func (s *service) startStatus(ctx context.Context, info spaceinfo.SpacePersisten } else { ctrl, err = s.factory.NewStreamableSpace(ctx, info.SpaceID, info, s.accountMetadataPayload) } + if err == nil && demandNow { + if err = ctrl.Start(ctx); err != nil { + if closeErr := ctrl.Close(ctx); closeErr != nil { + log.Warn("close controller after failed start", zap.Error(closeErr)) + } + } + } s.mu.Lock() - close(wait) - if err != nil { - s.waiting[info.SpaceID] = controllerWaiter{ - wait: wait, - err: err, + if err == nil && s.isClosing.Load() { + // Close() may have snapshotted the registry already; do not insert a + // controller it will never close + err = ErrSpaceIsClosing + s.mu.Unlock() + if closeErr := ctrl.Close(ctx); closeErr != nil { + log.Warn("close controller registered during shutdown", zap.Error(closeErr)) } + s.mu.Lock() + } + if err != nil { + s.regErr[info.SpaceID] = err + s.registryChangedLocked() s.mu.Unlock() return nil, err } s.spaceControllers[info.SpaceID] = ctrl + // the release may have happened between the demand decision and the + // insertion above; in that case the snapshot in releaseAll missed this + // controller, so demand it here (idempotent) + demandLate := s.released && !demandNow + s.registryChangedLocked() s.mu.Unlock() + if demandLate { + ctrl.Demand() + } return ctrl, nil } func (s *service) waitLoad(ctx context.Context, ctrl spacecontroller.SpaceController) (sp clientspace.Space, err error) { - if ld, ok := ctrl.Current().(loader.LoadWaiter); ok { - sp, err = ld.WaitLoad(ctx) - if err != nil { - err = convertSpaceError(err) + sp, err = ctrl.WaitLoad(ctx) + if err != nil { + switch { + case errors.Is(err, accountspace.ErrModeUnreachable): + return nil, fmt.Errorf("failed to load space, mode is %d: %w", ctrl.Mode(), ErrFailedToLoad) + case errors.Is(err, accountspace.ErrCtrlClosed): + return nil, ErrSpaceIsClosing } - return + return nil, convertSpaceError(err) } - return nil, fmt.Errorf("failed to load space, mode is %d: %w", ctrl.Mode(), ErrFailedToLoad) + return sp, nil } func convertSpaceError(err error) error { diff --git a/space/service.go b/space/service.go index ceb5a3fdd3..da2311e6fc 100644 --- a/space/service.go +++ b/space/service.go @@ -54,7 +54,6 @@ import ( "github.com/anyproto/anytype-heart/space/clientspace" "github.com/anyproto/anytype-heart/space/internal/components/aclobjectmanager" "github.com/anyproto/anytype-heart/space/internal/components/dependencies" - "github.com/anyproto/anytype-heart/space/internal/personalspace" "github.com/anyproto/anytype-heart/space/internal/spacecontroller" "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" "github.com/anyproto/anytype-heart/space/spacecore" @@ -71,12 +70,10 @@ const CName = "client.space" var log = logger.NewNamed(CName) var ( - waitSpaceDelay = 500 * time.Millisecond loadTechSpaceDeadline = 15 * time.Second - // Tunables for client-driven lazy multi-space loading. Vars (not consts) so - // tests and benchmarks can override them. + // preloadRemainingSpacesTimeout bounds how long lazy mode defers loading + // the non-preferred spaces. Var (not const) so tests can override it. preloadRemainingSpacesTimeout = 10 * time.Second - preloadConcurrency = 2 ) var ( @@ -148,21 +145,26 @@ type service struct { newAccount bool autoJoinStreamSpace string spaceControllers map[string]spacecontroller.SpaceController - waiting map[string]controllerWaiter - // Client-driven lazy multi-space loading (spec 2026-05-17). Latest - // spaceViewStatus per space, cached while deferred so the backlog can be - // drained later (RPC / timer / preferred-space failure). - deferredStatuses map[string]spaceViewStatus - preferredSpaceId string // from config; "" => eager (today's behavior) + marketplaceCtrl spacecontroller.SpaceController + // regChanged is closed and replaced (under s.mu) whenever the registry + // changes; waitCtrl blocks on it instead of polling. + regChanged chan struct{} + // regErr holds the last registration error per space id; cleared when the + // next registration attempt starts, so failures are retryable. + regErr map[string]error + // constructing single-flights controller construction per space id. + constructing map[string]chan struct{} + preferredSpaceId string // from config; "" => eager (load all at startup) // lazyMode is set exactly once in initAccount before watcher.Run() and is // never mutated afterwards, so it is safe to read without s.mu (the // happens-before from watcher.Run() / goroutine spawn covers all readers). - lazyMode bool - releasing bool // guarded by s.mu; true once the backlog is being drained - preloadOnce sync.Once // single release trigger (RPC | timer | dynamic fallback) - preloadCh chan struct{} // closed by triggerRelease() - applySpaceStatusHook func(spaceViewStatus) // test seam; nil in production - startStatusHook func(spaceinfo.SpacePersistentInfo) // test seam; nil in production + lazyMode bool + // released is true once the lazy backlog has been released (RPC | timer | + // preferred-space fallback); registrations after that demand immediately. + // Guarded by s.mu. + released bool + preloadOnce sync.Once // single release trigger + preloadCh chan struct{} // closed by triggerRelease() accountMetadataSymKey crypto.SymKey accountMetadataPayload []byte repKey uint64 @@ -205,8 +207,9 @@ func (s *service) Init(a *app.App) (err error) { s.spaceLoaderListener = app.MustComponent[aclobjectmanager.SpaceLoaderListener](a) s.identityService = app.MustComponent[dependencies.IdentityService](a) s.inboxSender = app.MustComponent[inboxservice.Sender](a) - s.waiting = make(map[string]controllerWaiter) - s.deferredStatuses = make(map[string]spaceViewStatus) + s.regChanged = make(chan struct{}) + s.regErr = make(map[string]error) + s.constructing = make(map[string]chan struct{}) s.preferredSpaceId = s.config.PreferredSpaceId s.preloadCh = make(chan struct{}) s.techSpaceReady = make(chan struct{}) @@ -262,7 +265,7 @@ func (s *service) createTechSpaceForOldAccounts(ctx context.Context) (err error) return fmt.Errorf("init tech space: %w", err) } // skipping check for space view because we don't have it - ctx = context.WithValue(ctx, personalspace.SkipCheckSpaceViewKey, true) + ctx = context.WithValue(ctx, spacefactory.SkipCheckSpaceViewKey, true) _, err = s.startStatus(ctx, spaceinfo.NewSpacePersistentInfo(s.personalSpaceId)) if err != nil { return fmt.Errorf("start personal space: %w", err) @@ -324,7 +327,7 @@ func (s *service) initAccount(ctx context.Context) (err error) { case <-s.ctx.Done(): return } - s.drainDeferred(s.ctx) + s.releaseAll() }() } s.techSpace.StartSync() @@ -345,6 +348,12 @@ func (s *service) createAccount(ctx context.Context) (err error) { if err != nil { return fmt.Errorf("init tech space: %w", err) } + // the watcher must run before the first space is created: controllers are + // registered exclusively through space view events + err = s.watcher.Run() + if err != nil { + return fmt.Errorf("run watcher: %w", err) + } if s.autoJoinStreamSpace == "" { firstSpace, err := s.create(ctx, nil) if err != nil { @@ -364,10 +373,6 @@ func (s *service) createAccount(ctx context.Context) (err error) { s.tryToJoinSpaceStream() } - err = s.watcher.Run() - if err != nil { - return fmt.Errorf("run watcher: %w", err) - } s.techSpace.StartSync() // only persist networkId after successful space init err = s.config.PersistAccountNetworkId() @@ -390,22 +395,59 @@ func (s *service) Create(ctx context.Context, description *spaceinfo.SpaceDescri } +// Wait returns the space once it is loaded, waiting (event-driven) for its +// controller to be registered when the space view exists but the watcher has +// not picked it up yet. func (s *service) Wait(ctx context.Context, spaceId string) (sp clientspace.Space, err error) { - waiter := newSpaceWaiter(s, s.ctx, waitSpaceDelay) - return waiter.waitSpace(ctx, spaceId) + if spaceId == s.techSpaceId { + return s.getTechSpace(ctx) + } + if spaceId == addr.AnytypeMarketplaceWorkspace { + return s.getMarketplace(ctx) + } + if ctrl, err := s.getCtrl(spaceId); err == nil { + return s.waitLoad(ctx, ctrl) + } + if s.techSpace == nil { + return nil, ErrSpaceNotExists + } + exists, err := s.techSpace.SpaceViewExists(ctx, spaceId) + if err != nil { + return nil, fmt.Errorf("check space view: %w", err) + } + if !exists { + return nil, ErrSpaceNotExists + } + ctrl, err := s.waitCtrl(ctx, spaceId) + if err != nil { + return nil, err + } + return s.waitLoad(ctx, ctrl) } +// Get returns the space once it is loaded. Unlike Wait it never blocks on +// controller registration (no view lookup, no I/O for unknown ids). func (s *service) Get(ctx context.Context, spaceId string) (sp clientspace.Space, err error) { if spaceId == s.techSpaceId { return s.getTechSpace(ctx) } - ctrl, err := s.getCtrl(ctx, spaceId) + if spaceId == addr.AnytypeMarketplaceWorkspace { + return s.getMarketplace(ctx) + } + ctrl, err := s.getCtrl(spaceId) if err != nil { return nil, err } return s.waitLoad(ctx, ctrl) } +func (s *service) getMarketplace(ctx context.Context) (clientspace.Space, error) { + if s.marketplaceCtrl == nil { + return nil, ErrSpaceNotExists + } + return s.marketplaceCtrl.WaitLoad(ctx) +} + func (s *service) UpdateSharedLimits(ctx context.Context, limits int) error { return s.techSpace.DoAccountObject(ctx, func(accObj techspace.AccountObject) error { return accObj.SetSharedSpacesLimit(limits) @@ -447,7 +489,7 @@ func (s *service) onSpaceStatusUpdated(spaceStatus spaceViewStatus) { return } s.maybeReleaseOnPreferredBroken(spaceStatus) - s.decideAndApplySpaceStatus(spaceStatus) + s.applySpaceStatus(spaceStatus) }() } @@ -466,34 +508,6 @@ func (s *service) maybeReleaseOnPreferredBroken(spaceStatus spaceViewStatus) { } } -// decideAndApplySpaceStatus is the production defer-or-build decision (called -// from onSpaceStatusUpdated). B2: the releasing read, the deferredStatuses -// write, and the branch choice are ONE critical section, atomic w.r.t. -// drainDeferred (which sets releasing+snapshots+clears under the same lock). -// Never read under lock then branch after unlock. -func (s *service) decideAndApplySpaceStatus(spaceStatus spaceViewStatus) { - s.mu.Lock() - _, alreadyStarted := s.spaceControllers[spaceStatus.spaceId] - _, alreadyWaiting := s.waiting[spaceStatus.spaceId] - shouldDefer := s.lazyMode && - spaceStatus.spaceId != s.preferredSpaceId && - !s.releasing && - !alreadyStarted && - !alreadyWaiting - if shouldDefer { - s.deferredStatuses[spaceStatus.spaceId] = spaceStatus - s.mu.Unlock() - log.Debug("lazy space build: deferring space until released", zap.String("spaceId", spaceStatus.spaceId)) - return - } - s.mu.Unlock() - if s.applySpaceStatusHook != nil { - s.applySpaceStatusHook(spaceStatus) - return - } - s.applySpaceStatus(spaceStatus) -} - // computeLazyMode decides, once, whether to defer non-preferred spaces. // B1 (accepted): if the preferred space's view is not on this device, // techSpace.SpaceViewExists may do a remote lookup (<=15s) before returning @@ -512,9 +526,9 @@ func (s *service) computeLazyMode(ctx context.Context, techSpace *clientspace.Te } // applySpaceStatus creates (idempotently) the space controller for the given -// status and pushes the persistent info into it. This is the eager-build body -// extracted from onSpaceStatusUpdated so it can also be invoked on demand by -// ensureSpaceStarted for deferred (non-personal) spaces. +// status and pushes the persistent info into it. In lazy mode non-preferred +// controllers register dormant: they offload/join according to their status +// but do not load until demanded (Get/Wait/preload release). func (s *service) applySpaceStatus(spaceStatus spaceViewStatus) { if s.isClosing.Load() { return @@ -526,9 +540,7 @@ func (s *service) applySpaceStatus(spaceStatus spaceViewStatus) { return } // startStatus returns (nil, err) on any factory error, so guard against a - // nil ctrl when err is ErrSpaceDeleted (mirrors ensureSpaceStarted) to - // avoid a nil-pointer deref now that this body is also invoked from - // drainDeferred workers and on-demand promotion. + // nil ctrl when err is ErrSpaceDeleted. if err != nil { return } @@ -538,93 +550,9 @@ func (s *service) applySpaceStatus(spaceStatus spaceViewStatus) { } } -// ensureSpaceStarted promotes a deferred space on demand (Wait/Get/workspaceOpen). -// E2: in lazy mode the watcher no longer eagerly creates controllers, so if no -// status is cached yet we must derive+build instead of no-op (otherwise -// waitSpace blocks until the caller ctx is cancelled). Eager mode keeps the -// original no-op (the watcher creates the controller). -func (s *service) ensureSpaceStarted(spaceId string) { - s.mu.Lock() - _, ctrlOk := s.spaceControllers[spaceId] - _, waitingOk := s.waiting[spaceId] - status, hasStatus := s.deferredStatuses[spaceId] - if hasStatus { - // Remove it from the backlog so a later drainDeferred does not snapshot - // and rebuild it; the delete shares this critical section with - // drainDeferred's snapshot+clear so the entry cannot be lost. - delete(s.deferredStatuses, spaceId) - } - s.mu.Unlock() - if ctrlOk || waitingOk { - return - } - if hasStatus { - s.applySpaceStatus(status) - return - } - if !s.lazyMode { - return - } - log.Debug("lazy space build: promoting space on demand (derived)", zap.String("spaceId", spaceId)) - // Resolve the real persistent space-view info (EncodedKey/guestKey, - // accountStatus, aclHeadId) instead of building from a zero value. A - // zero-value info has EncodedKey=="", which startStatus dispatches to - // NewShareableSpace (load.go), so a guest/streamable space would be - // permanently built as the wrong controller type with no signing key and - // would never self-correct. statusToInfo carries the same fields for the - // cached-status path; here we read them from the space view directly. - info, ok := s.resolveDerivedInfo(spaceId) - if !ok { - // The space view is absent (unknown id / not synced yet) or unreadable. - // Do not build from zero-value info: the failed build would poison - // s.waiting (startStatus caches the error, so a later Join/InviteJoin - // of this id fails for the rest of the session) and Get would return a - // techspace error instead of ErrSpaceNotExists. Bail out and let the - // caller fail with ErrSpaceNotExists, same as eager mode. - return - } - if s.startStatusHook != nil { - s.startStatusHook(info) - return - } - ctrl, err := s.startStatus(s.ctx, info) - if err != nil && !errors.Is(err, ErrSpaceDeleted) { - log.Warn("ensureSpaceStarted startStatus error", zap.Error(err)) - return - } - if err == nil { - if err = ctrl.Update(); err != nil { - log.Warn("ensureSpaceStarted ctrl.Update error", zap.Error(err)) - } - } -} - -// resolveDerivedInfo reads the persistent space-view info (account status, -// aclHeadId, encoded guest key) for a space promoted on demand before any -// status was cached. Preserving EncodedKey is what lets startStatus pick the -// correct controller type (NewStreamableSpace for guest/stream spaces vs -// NewShareableSpace), mirroring statusToInfo for the cached-status path. -// Returns ok=false when the view cannot be read (no view for this id, or a -// transient techspace error); the caller must not build in that case. -func (s *service) resolveDerivedInfo(spaceId string) (info spaceinfo.SpacePersistentInfo, ok bool) { - info = spaceinfo.NewSpacePersistentInfo(spaceId) - if s.techSpace == nil { - return info, false - } - err := s.techSpace.DoSpaceView(s.ctx, spaceId, func(spaceView techspace.SpaceView) error { - info = spaceView.GetPersistentInfo() - return nil - }) - if err != nil { - log.Warn("ensureSpaceStarted resolve persistent info", zap.String("spaceId", spaceId), zap.Error(err)) - return spaceinfo.NewSpacePersistentInfo(spaceId), false - } - return info, true -} - -// triggerRelease is the single idempotent "release the deferred backlog" -// signal, shared by the AccountPreloadRemainingSpaces RPC, the safety timer, -// and the preferred-space dynamic fallback. +// triggerRelease is the single idempotent "release the lazy backlog" signal, +// shared by the AccountPreloadRemainingSpaces RPC, the safety timer, and the +// preferred-space dynamic fallback. func (s *service) triggerRelease() { s.preloadOnce.Do(func() { close(s.preloadCh) @@ -632,55 +560,30 @@ func (s *service) triggerRelease() { } // PreloadRemainingSpaces releases spaces deferred by lazy mode. Idempotent and -// safe to call before any status is cached. +// safe to call at any time. func (s *service) PreloadRemainingSpaces(ctx context.Context) error { s.triggerRelease() return nil } -// drainDeferred releases the deferred backlog. B2: set releasing + snapshot + -// clear is ONE critical section, atomic w.r.t. decideAndApplySpaceStatus's -// decision. Builds with bounded concurrency. Bails out if the service is -// closing so a late timer-triggered drain cannot build controllers that -// Close() has already stopped tracking (applySpaceStatus re-checks per build). -func (s *service) drainDeferred(ctx context.Context) { +// releaseAll demands every registered space. Registrations that race with the +// release are covered by startStatus: it re-checks released after inserting +// the controller, so a controller is either in the snapshot taken here or +// demands itself. +func (s *service) releaseAll() { if s.isClosing.Load() { return } s.mu.Lock() - s.releasing = true - snapshot := make([]spaceViewStatus, 0, len(s.deferredStatuses)) - for _, st := range s.deferredStatuses { - snapshot = append(snapshot, st) + s.released = true + ctrls := make([]spacecontroller.SpaceController, 0, len(s.spaceControllers)) + for _, ctrl := range s.spaceControllers { + ctrls = append(ctrls, ctrl) } - s.deferredStatuses = make(map[string]spaceViewStatus) s.mu.Unlock() - - if len(snapshot) == 0 { - return - } - sem := make(chan struct{}, preloadConcurrency) - var wg sync.WaitGroup - for _, st := range snapshot { - select { - case <-ctx.Done(): - wg.Wait() - return - default: - } - wg.Add(1) - sem <- struct{}{} - go func(st spaceViewStatus) { - defer wg.Done() - defer func() { <-sem }() - if s.applySpaceStatusHook != nil { - s.applySpaceStatusHook(st) - return - } - s.applySpaceStatus(st) - }(st) + for _, ctrl := range ctrls { + ctrl.Demand() } - wg.Wait() } func (s *service) SpaceViewSetOneToOneIdentity(spaceId string, identity string) { @@ -740,10 +643,13 @@ func (s *service) Close(ctx context.Context) error { } s.isClosing.Store(true) s.mu.Lock() - ctrls := make([]spacecontroller.SpaceController, 0, len(s.spaceControllers)) + ctrls := make([]spacecontroller.SpaceController, 0, len(s.spaceControllers)+1) for _, ctrl := range s.spaceControllers { ctrls = append(ctrls, ctrl) } + if s.marketplaceCtrl != nil { + ctrls = append(ctrls, s.marketplaceCtrl) + } s.mu.Unlock() wg := sync.WaitGroup{} @@ -769,16 +675,13 @@ func (s *service) AllSpaceIds() (ids []string) { s.mu.Lock() defer s.mu.Unlock() for id := range s.spaceControllers { - if id == addr.AnytypeMarketplaceWorkspace { - continue - } ids = append(ids, id) } return } -// AllLoadedSpaceIds returns IDs of spaces that are fully loaded (in ModeLoading state). -// Excludes marketplace space and spaces that are being offloaded, deleted, or joining. +// AllLoadedSpaceIds returns IDs of spaces that are loading or loaded. +// Excludes dormant spaces and spaces that are offloading or joining. func (s *service) AllLoadedSpaceIds() (ids []string) { s.mu.Lock() defer s.mu.Unlock() @@ -786,9 +689,6 @@ func (s *service) AllLoadedSpaceIds() (ids []string) { if c.Mode() != mode.ModeLoading { continue } - if id == addr.AnytypeMarketplaceWorkspace { - continue - } ids = append(ids, id) } return diff --git a/space/service_test.go b/space/service_test.go index 87212124b5..8cb52ade48 100644 --- a/space/service_test.go +++ b/space/service_test.go @@ -120,6 +120,7 @@ func TestService_Init(t *testing.T) { fx.factory.EXPECT().CreateAndSetTechSpace(mock.Anything).Return(&clientspace.TechSpace{TechSpace: fx.techSpace}, nil) prCtrl := mock_spacecontroller.NewMockSpaceController(t) fx.factory.EXPECT().NewPersonalSpace(mock.Anything, mock.Anything).Return(prCtrl, nil) + prCtrl.EXPECT().Start(mock.Anything).Return(nil) prCtrl.EXPECT().Close(mock.Anything).Return(nil) fx.techSpace.EXPECT().StartSync() }) @@ -131,6 +132,7 @@ func TestService_Init(t *testing.T) { fx.factory.EXPECT().CreateAndSetTechSpace(mock.Anything).Return(&clientspace.TechSpace{TechSpace: fx.techSpace}, nil) prCtrl := mock_spacecontroller.NewMockSpaceController(t) fx.factory.EXPECT().NewPersonalSpace(mock.Anything, mock.Anything).Return(prCtrl, nil) + prCtrl.EXPECT().Start(mock.Anything).Return(nil) prCtrl.EXPECT().Close(mock.Anything).Return(nil) fx.techSpace.EXPECT().StartSync() }) @@ -289,14 +291,6 @@ type fixture struct { objectStore *objectstore.StoreFixture } -type lwMock struct { - sp clientspace.Space -} - -func (l lwMock) WaitLoad(ctx context.Context) (sp clientspace.Space, err error) { - return l.sp, nil -} - func (fx *fixture) expectRun(t *testing.T, expectOldAccount func(t *testing.T, fx *fixture)) { fx.spaceCore.EXPECT().DeriveID(mock.Anything, spacedomain.SpaceTypeRegular).Return(fx.spaceId, nil).Times(1) fx.spaceCore.EXPECT().DeriveID(mock.Anything, spacedomain.SpaceTypeTech).Return("techSpaceId", nil).Times(1) @@ -313,14 +307,25 @@ func (fx *fixture) expectRun(t *testing.T, expectOldAccount func(t *testing.T, f if expectOldAccount == nil { fx.factory.EXPECT().CreateAndSetTechSpace(mock.Anything).Return(&clientspace.TechSpace{TechSpace: ts}, nil) prCtrl := mock_spacecontroller.NewMockSpaceController(t) - prCtrl.EXPECT().SpaceId().Return(fx.spaceId) + prCtrl.EXPECT().SpaceId().Return(fx.spaceId).Maybe() commonSpace := mock_commonspace.NewMockSpace(fx.ctrl) commonSpace.EXPECT().Id().Return(fx.spaceId).AnyTimes() fx.spaceCore.EXPECT().Create(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&spacecore.AnySpace{Space: commonSpace}, nil) - fx.factory.EXPECT().CreateShareableSpace(mock.Anything, mock.Anything, mock.Anything).Return(prCtrl, nil) - lw := lwMock{clientSpace} + // the factory creates only the space view; the watcher picks the view + // up and registers the controller (the first space id equals the + // personal space id, so it dispatches to NewPersonalSpace) + fx.factory.EXPECT().CreateShareableSpace(mock.Anything, fx.spaceId, mock.Anything).RunAndReturn( + func(_ context.Context, id string, _ *spaceinfo.SpaceDescription) error { + fx.objectStore.AddObjects(t, fx.service.techSpaceId, []objectstore.TestObject{ + givenSpaceViewObject("spaceView.first", id, "creator", spaceinfo.AccountStatusUnknown, spaceinfo.RemoteStatusUnknown, spaceinfo.LocalStatusUnknown, ""), + }) + return nil + }) + fx.factory.EXPECT().NewPersonalSpace(mock.Anything, mock.Anything).Return(prCtrl, nil) + prCtrl.EXPECT().Start(mock.Anything).Return(nil) + prCtrl.EXPECT().Update().Return(nil).Maybe() clientSpace.EXPECT().Id().Return(fx.spaceId) - prCtrl.EXPECT().Current().Return(lw) + prCtrl.EXPECT().WaitLoad(mock.Anything).Return(clientSpace, nil) prCtrl.EXPECT().Close(mock.Anything).Return(nil) ts.EXPECT().StartSync() } else { @@ -408,6 +413,7 @@ func TestService_onSpaceStatusUpdated(t *testing.T) { personalCtrl.EXPECT().SpaceId().Return(fx.spaceId).Maybe() personalCtrl.EXPECT().Close(mock.Anything).Return(nil).Maybe() fx.factory.EXPECT().NewPersonalSpace(mock.Anything, fx.service.accountMetadataPayload).Return(personalCtrl, nil) + personalCtrl.EXPECT().Start(mock.Anything).Return(nil) personalCtrl.EXPECT().Update().Run(func() { close(done) }).Return(nil).Once() @@ -431,6 +437,7 @@ func TestService_onSpaceStatusUpdated(t *testing.T) { shareableCtrl.EXPECT().SpaceId().Return(shareableSpaceId).Maybe() shareableCtrl.EXPECT().Close(mock.Anything).Return(nil).Maybe() fx.factory.EXPECT().NewShareableSpace(mock.Anything, shareableSpaceId, mock.Anything).Return(shareableCtrl, nil) + shareableCtrl.EXPECT().Start(mock.Anything).Return(nil) shareableCtrl.EXPECT().Update().Run(func() { close(done) }).Return(nil).Once() @@ -454,6 +461,7 @@ func TestService_onSpaceStatusUpdated(t *testing.T) { streamableCtrl.EXPECT().SpaceId().Return(streamableSpaceId).Maybe() streamableCtrl.EXPECT().Close(mock.Anything).Return(nil).Maybe() fx.factory.EXPECT().NewStreamableSpace(mock.Anything, streamableSpaceId, mock.Anything, fx.service.accountMetadataPayload).Return(streamableCtrl, nil) + streamableCtrl.EXPECT().Start(mock.Anything).Return(nil) streamableCtrl.EXPECT().Update().Run(func() { close(done) }).Return(nil).Once() @@ -505,6 +513,7 @@ func TestService_onSpaceStatusUpdated(t *testing.T) { shareableCtrl.EXPECT().SpaceId().Return(alreadyDeletedSpaceId).Maybe() shareableCtrl.EXPECT().Close(mock.Anything).Return(nil).Maybe() fx.factory.EXPECT().NewShareableSpace(mock.Anything, alreadyDeletedSpaceId, mock.Anything).Return(shareableCtrl, nil) + shareableCtrl.EXPECT().Start(mock.Anything).Return(nil) shareableCtrl.EXPECT().Update().Run(func() { close(done) }).Return(nil).Once() @@ -543,6 +552,7 @@ func TestService_onSpaceStatusUpdated(t *testing.T) { updateErrorCtrl.EXPECT().SpaceId().Return(updateErrorSpaceId).Maybe() updateErrorCtrl.EXPECT().Close(mock.Anything).Return(nil).Maybe() fx.factory.EXPECT().NewShareableSpace(mock.Anything, updateErrorSpaceId, mock.Anything).Return(updateErrorCtrl, nil) + updateErrorCtrl.EXPECT().Start(mock.Anything).Return(nil) updateErrorCtrl.EXPECT().Update().Return(fmt.Errorf("update error")) fx.objectStore.AddObjects(t, fx.service.techSpaceId, []objectstore.TestObject{ diff --git a/space/space_lazy_test.go b/space/space_lazy_test.go index 0c433664c5..916ac9efd9 100644 --- a/space/space_lazy_test.go +++ b/space/space_lazy_test.go @@ -2,12 +2,9 @@ package space import ( "context" - "errors" "strconv" "sync" - "sync/atomic" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -18,7 +15,6 @@ import ( "github.com/anyproto/anytype-heart/space/internal/spacecontroller/mock_spacecontroller" "github.com/anyproto/anytype-heart/space/spacefactory/mock_spacefactory" "github.com/anyproto/anytype-heart/space/spaceinfo" - "github.com/anyproto/anytype-heart/space/techspace" "github.com/anyproto/anytype-heart/space/techspace/mock_techspace" ) @@ -60,9 +56,11 @@ func newLazyServiceForStatus(t *testing.T) *service { s := New().(*service) s.ctx = context.Background() s.spaceControllers = map[string]spacecontroller.SpaceController{} - s.waiting = map[string]controllerWaiter{} - s.deferredStatuses = map[string]spaceViewStatus{} + s.regChanged = make(chan struct{}) + s.regErr = map[string]error{} + s.constructing = map[string]chan struct{}{} s.preloadCh = make(chan struct{}) + s.personalSpaceId = "personal.id" return s } @@ -76,122 +74,190 @@ func statusFor(spaceId string, local spaceinfo.LocalStatus, account spaceinfo.Ac } } -func TestOnSpaceStatusUpdated_Defer(t *testing.T) { +// TestStartStatus_LazyRegistersDormant: in lazy mode a non-preferred space is +// registered (controller constructed, status-driven work possible) but not +// started — Start would begin loading, which must wait for demand. +func TestStartStatus_LazyRegistersDormant(t *testing.T) { s := newLazyServiceForStatus(t) s.lazyMode = true s.preferredSpaceId = "preferred" - s.decideAndApplySpaceStatus(statusFor("other", spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive)) + factory := mock_spacefactory.NewMockSpaceFactory(t) + // no Start expectation: the strict mock fails the test if Start is called + ctrl := mock_spacecontroller.NewMockSpaceController(t) + factory.EXPECT().NewShareableSpace(mock.Anything, "other", mock.Anything).Return(ctrl, nil) + s.factory = factory + + got, err := s.startStatus(context.Background(), spaceinfo.NewSpacePersistentInfo("other")) + require.NoError(t, err) + require.NotNil(t, got) s.mu.Lock() - _, deferred := s.deferredStatuses["other"] - _, built := s.spaceControllers["other"] + _, registered := s.spaceControllers["other"] s.mu.Unlock() - assert.True(t, deferred, "non-preferred space must be cached as deferred") - assert.False(t, built, "non-preferred space must NOT be built") + assert.True(t, registered, "non-preferred space must be registered dormant") } -func TestOnSpaceStatusUpdated_PreferredBrokenReleases(t *testing.T) { +// TestStartStatus_LazyStartsPreferred: the preferred space starts (loads) +// immediately even in lazy mode. +func TestStartStatus_LazyStartsPreferred(t *testing.T) { s := newLazyServiceForStatus(t) s.lazyMode = true s.preferredSpaceId = "preferred" - s.maybeReleaseOnPreferredBroken(statusFor("preferred", spaceinfo.LocalStatusMissing, spaceinfo.AccountStatusActive)) + factory := mock_spacefactory.NewMockSpaceFactory(t) + ctrl := mock_spacecontroller.NewMockSpaceController(t) + ctrl.EXPECT().Start(mock.Anything).Return(nil).Once() + factory.EXPECT().NewShareableSpace(mock.Anything, "preferred", mock.Anything).Return(ctrl, nil) + s.factory = factory - select { - case <-s.preloadCh: - default: - t.Fatal("preferred space Missing must trigger release") + _, err := s.startStatus(context.Background(), spaceinfo.NewSpacePersistentInfo("preferred")) + require.NoError(t, err) +} + +// TestStartStatus_EagerStartsEverything guards the backward-compat promise: +// with no preferred space (lazyMode=false) every space starts immediately. +func TestStartStatus_EagerStartsEverything(t *testing.T) { + s := newLazyServiceForStatus(t) + s.lazyMode = false + + factory := mock_spacefactory.NewMockSpaceFactory(t) + s.factory = factory + for i := 0; i < 3; i++ { + id := "s" + strconv.Itoa(i) + ctrl := mock_spacecontroller.NewMockSpaceController(t) + ctrl.EXPECT().Start(mock.Anything).Return(nil).Once() + factory.EXPECT().NewShareableSpace(mock.Anything, id, mock.Anything).Return(ctrl, nil) + _, err := s.startStatus(context.Background(), spaceinfo.NewSpacePersistentInfo(id)) + require.NoError(t, err) } } -func TestOnSpaceStatusUpdated_JoiningDoesNotRelease(t *testing.T) { +// TestStartStatus_AfterReleaseStartsImmediately: once the backlog is released, +// newly registered spaces start immediately even in lazy mode. +func TestStartStatus_AfterReleaseStartsImmediately(t *testing.T) { s := newLazyServiceForStatus(t) s.lazyMode = true s.preferredSpaceId = "preferred" + s.releaseAll() - s.maybeReleaseOnPreferredBroken(statusFor("preferred", spaceinfo.LocalStatusLoading, spaceinfo.AccountStatusJoining)) + factory := mock_spacefactory.NewMockSpaceFactory(t) + ctrl := mock_spacecontroller.NewMockSpaceController(t) + ctrl.EXPECT().Start(mock.Anything).Return(nil).Once() + factory.EXPECT().NewShareableSpace(mock.Anything, "late", mock.Anything).Return(ctrl, nil) + s.factory = factory - select { - case <-s.preloadCh: - t.Fatal("Joining must NOT trigger release") - default: - } + _, err := s.startStatus(context.Background(), spaceinfo.NewSpacePersistentInfo("late")) + require.NoError(t, err) } -func TestDrainDeferred_BuildsSnapshotAndClears(t *testing.T) { +// TestReleaseAll_DemandsRegistered: releasing demands every dormant controller. +func TestReleaseAll_DemandsRegistered(t *testing.T) { s := newLazyServiceForStatus(t) s.lazyMode = true s.preferredSpaceId = "preferred" - built := make(chan string, 8) - s.applySpaceStatusHook = func(st spaceViewStatus) { built <- st.spaceId } - + ctrlA := mock_spacecontroller.NewMockSpaceController(t) + ctrlA.EXPECT().Demand().Once() + ctrlB := mock_spacecontroller.NewMockSpaceController(t) + ctrlB.EXPECT().Demand().Once() s.mu.Lock() - s.deferredStatuses["a"] = statusFor("a", spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive) - s.deferredStatuses["b"] = statusFor("b", spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive) + s.spaceControllers["a"] = ctrlA + s.spaceControllers["b"] = ctrlB s.mu.Unlock() - s.drainDeferred(context.Background()) - - got := map[string]bool{} - for i := 0; i < 2; i++ { - got[<-built] = true - } - assert.True(t, got["a"] && got["b"], "drain must build every deferred space") + s.releaseAll() s.mu.Lock() - assert.True(t, s.releasing) - assert.Empty(t, s.deferredStatuses, "deferred map cleared") + released := s.released s.mu.Unlock() + assert.True(t, released) } -func TestDrainDeferred_NoStrandedRace(t *testing.T) { +// TestStartStatus_RegisterReleaseRace: a registration racing releaseAll must +// never strand a space dormant — it is demanded either by the release +// snapshot, by the demand decision (released already set), or by the +// late-release re-check after insertion. +func TestStartStatus_RegisterReleaseRace(t *testing.T) { + const n = 50 s := newLazyServiceForStatus(t) s.lazyMode = true s.preferredSpaceId = "preferred" - var mu sync.Mutex - built := map[string]bool{} - s.applySpaceStatusHook = func(st spaceViewStatus) { - mu.Lock() - built[st.spaceId] = true - mu.Unlock() - } + var demanded sync.Map + factory := mock_spacefactory.NewMockSpaceFactory(t) + factory.EXPECT().NewShareableSpace(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( + func(_ context.Context, id string, _ spaceinfo.SpacePersistentInfo) (spacecontroller.SpaceController, error) { + ctrl := mock_spacecontroller.NewMockSpaceController(t) + ctrl.EXPECT().Start(mock.Anything).RunAndReturn(func(context.Context) error { + demanded.Store(id, true) + return nil + }).Maybe() + ctrl.EXPECT().Demand().Run(func() { + demanded.Store(id, true) + }).Maybe() + return ctrl, nil + }) + s.factory = factory var wg sync.WaitGroup - for i := 0; i < 50; i++ { + for i := 0; i < n; i++ { id := "s" + strconv.Itoa(i) wg.Add(1) go func() { defer wg.Done() - s.decideAndApplySpaceStatus(statusFor(id, spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive)) + _, err := s.startStatus(context.Background(), spaceinfo.NewSpacePersistentInfo(id)) + require.NoError(t, err) }() } - // Single release trigger, exactly as production does it (one drainDeferred - // behind the sync.Once). Join it before asserting: drainDeferred returns - // only after its bounded workers have built the whole snapshot, so the - // B2 invariant is observable deterministically rather than racing the - // background workers. - d1 := make(chan struct{}) - go func() { s.drainDeferred(context.Background()); close(d1) }() - wg.Wait() // every applier's decision (defer or inline build) finished - <-d1 // first drain + all its workers finished - // Mop-up: spaces an applier deferred while the drain had already passed - // its snapshot see releasing==true and build inline, so nothing should - // remain; this second call must be a safe no-op (closes the B2 window). - s.drainDeferred(context.Background()) + releaseDone := make(chan struct{}) + go func() { + s.releaseAll() + close(releaseDone) + }() + wg.Wait() + <-releaseDone + + for i := 0; i < n; i++ { + id := "s" + strconv.Itoa(i) + _, ok := demanded.Load(id) + assert.True(t, ok, "space "+id+" must be demanded, never stranded dormant") + } +} - s.mu.Lock() - leftover := len(s.deferredStatuses) - s.mu.Unlock() - require.Zero(t, leftover, "no space may remain queued after the backlog is drained") +// TestStartStatus_SingleFlight: concurrent registrations for the same id must +// construct exactly one controller; the losers wait and reuse it. +func TestStartStatus_SingleFlight(t *testing.T) { + const n = 20 + s := newLazyServiceForStatus(t) + s.lazyMode = true // dormant path: no Start expectations needed + s.preferredSpaceId = "preferred" - mu.Lock() - defer mu.Unlock() - for i := 0; i < 50; i++ { - id := "s" + strconv.Itoa(i) - assert.True(t, built[id], "space "+id+" must be built, never stranded") + factory := mock_spacefactory.NewMockSpaceFactory(t) + ctrl := mock_spacecontroller.NewMockSpaceController(t) + started := make(chan struct{}) + factory.EXPECT().NewShareableSpace(mock.Anything, "same", mock.Anything).RunAndReturn( + func(context.Context, string, spaceinfo.SpacePersistentInfo) (spacecontroller.SpaceController, error) { + <-started // hold every racer in the construction window + return ctrl, nil + }).Once() // a second construction fails the test + s.factory = factory + + var wg sync.WaitGroup + results := make([]spacecontroller.SpaceController, n) + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + got, err := s.startStatus(context.Background(), spaceinfo.NewSpacePersistentInfo("same")) + require.NoError(t, err) + results[i] = got + }(i) + } + close(started) + wg.Wait() + for i := 0; i < n; i++ { + assert.Same(t, ctrl, results[i]) } } @@ -206,29 +272,6 @@ func TestPreloadRemainingSpaces_Idempotent(t *testing.T) { } } -func TestEnsureSpaceStarted_DeriveWhenNoCachedStatus(t *testing.T) { - s := newLazyServiceWithSpaceView(t, "not-yet-cached", spaceinfo.NewSpacePersistentInfo("not-yet-cached")) - - called := make(chan string, 1) - s.startStatusHook = func(info spaceinfo.SpacePersistentInfo) { called <- info.SpaceID } - - s.ensureSpaceStarted("not-yet-cached") - - select { - case got := <-called: - assert.Equal(t, "not-yet-cached", got) - default: - t.Fatal("ensureSpaceStarted must derive+build when status not cached (E2)") - } -} - -func TestEnsureSpaceStarted_EagerModeNoOp(t *testing.T) { - s := newLazyServiceForStatus(t) - s.lazyMode = false - s.startStatusHook = func(info spaceinfo.SpacePersistentInfo) { t.Fatal("must not build in eager mode") } - s.ensureSpaceStarted("whatever") // no cached status, eager => preserve old no-op behavior -} - // statusForRemote builds a spaceViewStatus with an explicit remoteStatus // (statusFor hardcodes RemoteStatusOk). func statusForRemote(spaceId string, local spaceinfo.LocalStatus, account spaceinfo.AccountStatus, remote spaceinfo.RemoteStatus) spaceViewStatus { @@ -237,64 +280,9 @@ func statusForRemote(spaceId string, local spaceinfo.LocalStatus, account spacei return st } -// TestEagerMode_NoDeferral guards the spec §9 backward-compat promise: -// preferredSpaceId=="" (lazyMode=false) must never defer — every space builds -// inline, byte-identical to pre-feature behavior. -func TestEagerMode_NoDeferral(t *testing.T) { - s := newLazyServiceForStatus(t) - s.lazyMode = false - s.preferredSpaceId = "" - - var mu sync.Mutex - built := map[string]bool{} - s.applySpaceStatusHook = func(st spaceViewStatus) { - mu.Lock() - built[st.spaceId] = true - mu.Unlock() - } - - for i := 0; i < 3; i++ { - s.decideAndApplySpaceStatus(statusFor("s"+strconv.Itoa(i), spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive)) - } - - s.mu.Lock() - assert.Empty(t, s.deferredStatuses, "eager mode must not defer any space") - assert.False(t, s.releasing) - s.mu.Unlock() - mu.Lock() - defer mu.Unlock() - for i := 0; i < 3; i++ { - assert.True(t, built["s"+strconv.Itoa(i)], "eager mode must build every space inline") - } -} - -// TestOnSpaceStatusUpdated_PreferredBuildsNotDeferred closes the §8.2 positive -// half: in lazy mode the preferred space itself builds immediately, never -// deferred. -func TestOnSpaceStatusUpdated_PreferredBuildsNotDeferred(t *testing.T) { - s := newLazyServiceForStatus(t) - s.lazyMode = true - s.preferredSpaceId = "preferred" - - built := make(chan string, 1) - s.applySpaceStatusHook = func(st spaceViewStatus) { built <- st.spaceId } - - s.decideAndApplySpaceStatus(statusFor("preferred", spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive)) - - select { - case got := <-built: - assert.Equal(t, "preferred", got) - default: - t.Fatal("preferred space must build immediately, not be deferred") - } - s.mu.Lock() - _, deferred := s.deferredStatuses["preferred"] - s.mu.Unlock() - assert.False(t, deferred, "preferred space must never be cached as deferred") -} - -// TestOnSpaceStatusUpdated_PreferredBroken_AllVariants covers every spec §3 -// dynamic-fallback trigger plus the non-trigger guards. +// TestOnSpaceStatusUpdated_PreferredBroken_AllVariants covers every dynamic +// fallback trigger plus the non-trigger guards: a broken preferred space +// collapses lazy mode and releases the backlog. func TestOnSpaceStatusUpdated_PreferredBroken_AllVariants(t *testing.T) { cases := []struct { name string @@ -336,218 +324,3 @@ func TestOnSpaceStatusUpdated_PreferredBroken_AllVariants(t *testing.T) { }) } } - -// TestDrainDeferred_BoundedConcurrency asserts the core feature property -// (spec §8.5): the backlog drains with at most preloadConcurrency builds in -// flight, replacing the unbounded ~10·N eager fan-out. -func TestDrainDeferred_BoundedConcurrency(t *testing.T) { - oldK := preloadConcurrency - preloadConcurrency = 2 - defer func() { preloadConcurrency = oldK }() - - s := newLazyServiceForStatus(t) - s.lazyMode = true - s.preferredSpaceId = "preferred" - - var inflight, maxInflight atomic.Int32 - release := make(chan struct{}) - s.applySpaceStatusHook = func(st spaceViewStatus) { - cur := inflight.Add(1) - for { - m := maxInflight.Load() - if cur <= m || maxInflight.CompareAndSwap(m, cur) { - break - } - } - <-release - inflight.Add(-1) - } - - s.mu.Lock() - for i := 0; i < 10; i++ { - id := "s" + strconv.Itoa(i) - s.deferredStatuses[id] = statusFor(id, spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive) - } - s.mu.Unlock() - - done := make(chan struct{}) - go func() { s.drainDeferred(context.Background()); close(done) }() - - // The first preloadConcurrency workers must both enter and block before we - // release them, proving the pool actually caps concurrency. - require.Eventually(t, func() bool { - return maxInflight.Load() >= int32(preloadConcurrency) - }, 2*time.Second, time.Millisecond) - close(release) - <-done - - assert.LessOrEqual(t, maxInflight.Load(), int32(preloadConcurrency), - "never more than preloadConcurrency builds in flight") - assert.Equal(t, int32(0), inflight.Load()) - s.mu.Lock() - assert.Empty(t, s.deferredStatuses) - s.mu.Unlock() -} - -// newLazyServiceWithSpaceView wires a tech space whose DoSpaceView returns a -// space view carrying the given persistent info, so the on-demand derive path -// can resolve real EncodedKey/accountStatus/aclHeadId instead of a zero value. -func newLazyServiceWithSpaceView(t *testing.T, spaceId string, info spaceinfo.SpacePersistentInfo) *service { - s := newLazyServiceForStatus(t) - s.lazyMode = true - - view := mock_techspace.NewMockSpaceView(t) - view.EXPECT().GetPersistentInfo().Return(info) - ts := mock_techspace.NewMockTechSpace(t) - ts.EXPECT().DoSpaceView(mock.Anything, spaceId, mock.Anything). - RunAndReturn(func(_ context.Context, _ string, apply func(techspace.SpaceView) error) error { - return apply(view) - }) - s.techSpace = &clientspace.TechSpace{TechSpace: ts} - return s -} - -// TestEnsureSpaceStarted_DerivePreservesGuestKey is the regression for the -// top medium finding: a guest/streamable space promoted on demand (no cached -// status yet) must keep its EncodedKey/guestKey, accountStatus and aclHeadId so -// startStatus dispatches to NewStreamableSpace rather than building a keyless -// shareable controller. Before the fix the derive path passed a zero-value -// SpacePersistentInfo (EncodedKey==""), so this asserts the resolved info. -func TestEnsureSpaceStarted_DerivePreservesGuestKey(t *testing.T) { - const spaceId = "stream.space" - want := spaceinfo.NewSpacePersistentInfo(spaceId) - want.SetAccountStatus(spaceinfo.AccountStatusActive). - SetAclHeadId("acl-head-1"). - SetEncodedKey("guest-priv-key") - - s := newLazyServiceWithSpaceView(t, spaceId, want) - - got := make(chan spaceinfo.SpacePersistentInfo, 1) - s.startStatusHook = func(info spaceinfo.SpacePersistentInfo) { got <- info } - - s.ensureSpaceStarted(spaceId) - - select { - case info := <-got: - assert.Equal(t, spaceId, info.SpaceID) - assert.Equal(t, "guest-priv-key", info.EncodedKey, - "derived promotion must preserve guestKey so it builds as a streamable space") - assert.Equal(t, "acl-head-1", info.AclHeadId) - assert.Equal(t, spaceinfo.AccountStatusActive, info.GetAccountStatus()) - default: - t.Fatal("ensureSpaceStarted must derive+build when status not cached (E2)") - } -} - -// TestEnsureSpaceStarted_DeriveBuildsStreamableNotShareable drives the real -// startStatus dispatch (no startStatusHook) and asserts the on-demand promotion -// of a guest space chooses the streamable factory, never the shareable one. -func TestEnsureSpaceStarted_DeriveBuildsStreamableNotShareable(t *testing.T) { - const spaceId = "stream.space" - info := spaceinfo.NewSpacePersistentInfo(spaceId) - info.SetAccountStatus(spaceinfo.AccountStatusActive). - SetEncodedKey("guest-priv-key") - - s := newLazyServiceWithSpaceView(t, spaceId, info) - s.personalSpaceId = "personal.id" // ensure the personal-space branch is not taken - - factory := mock_spacefactory.NewMockSpaceFactory(t) - ctrl := mock_spacecontroller.NewMockSpaceController(t) - ctrl.EXPECT().Update().Return(nil) - factory.EXPECT(). - NewStreamableSpace(mock.Anything, spaceId, mock.MatchedBy(func(i spaceinfo.SpacePersistentInfo) bool { - return i.EncodedKey == "guest-priv-key" - }), mock.Anything). - Return(ctrl, nil) - // NewShareableSpace is intentionally NOT expected: the mock fails the test - // if it is called, locking in that a guest space is never mis-built. - s.factory = factory - - s.ensureSpaceStarted(spaceId) - - s.mu.Lock() - _, built := s.spaceControllers[spaceId] - s.mu.Unlock() - assert.True(t, built, "streamable controller must be registered after on-demand promotion") -} - -// TestEnsureSpaceStarted_CachedStatusRemovedFromBacklog locks in that promoting -// a cached deferred space on demand removes it from deferredStatuses so a later -// drainDeferred does not snapshot and rebuild it. -func TestEnsureSpaceStarted_CachedStatusRemovedFromBacklog(t *testing.T) { - const spaceId = "cached" - s := newLazyServiceForStatus(t) - s.lazyMode = true - s.preferredSpaceId = "preferred" - s.personalSpaceId = "personal.id" - - factory := mock_spacefactory.NewMockSpaceFactory(t) - ctrl := mock_spacecontroller.NewMockSpaceController(t) - ctrl.EXPECT().Update().Return(nil) - // A cached status with an empty guestKey builds as a shareable space; the - // mock fails if it is called more than once, proving no redundant rebuild. - factory.EXPECT().NewShareableSpace(mock.Anything, spaceId, mock.Anything).Return(ctrl, nil).Once() - s.factory = factory - - s.mu.Lock() - s.deferredStatuses[spaceId] = statusFor(spaceId, spaceinfo.LocalStatusOk, spaceinfo.AccountStatusActive) - s.mu.Unlock() - - s.ensureSpaceStarted(spaceId) - - s.mu.Lock() - _, stillDeferred := s.deferredStatuses[spaceId] - _, built := s.spaceControllers[spaceId] - s.mu.Unlock() - assert.True(t, built, "cached deferred space must be built on demand") - assert.False(t, stillDeferred, "promoted space must be removed from the deferred backlog") - - // A later drain must not rebuild it (NewShareableSpace.Once() would fail). - s.drainDeferred(context.Background()) -} - -// TestEnsureSpaceStarted_UnresolvableViewDoesNotBuild is the regression for the -// high review finding: in lazy mode, promoting a space whose space view cannot -// be resolved (not synced yet, unknown id, or a transient techspace error) must -// NOT build a controller from zero-value info. Building would (1) replace -// Get's ErrSpaceNotExists with a techspace error, (2) permanently poison -// s.waiting with the failed build so a later Join/InviteJoin of that space id -// fails for the rest of the session, and (3) on a transient error mis-build a -// guest space as a keyless shareable controller. -func TestEnsureSpaceStarted_UnresolvableViewDoesNotBuild(t *testing.T) { - for _, tc := range []struct { - name string - err error - }{ - {"view not exists", techspace.ErrSpaceViewNotExists}, - {"transient resolve error", errors.New("objectstore closed")}, - } { - t.Run(tc.name, func(t *testing.T) { - const spaceId = "unknown.space" - s := newLazyServiceForStatus(t) - s.lazyMode = true - s.personalSpaceId = "personal.id" - - ts := mock_techspace.NewMockTechSpace(t) - ts.EXPECT().DoSpaceView(mock.Anything, spaceId, mock.Anything).Return(tc.err) - s.techSpace = &clientspace.TechSpace{TechSpace: ts} - - // No expectations: the mock fails the test if any factory method is - // called, locking in that an unresolvable space is never built. - s.factory = mock_spacefactory.NewMockSpaceFactory(t) - - s.ensureSpaceStarted(spaceId) - - s.mu.Lock() - _, built := s.spaceControllers[spaceId] - _, waiting := s.waiting[spaceId] - s.mu.Unlock() - assert.False(t, built, "unresolvable space must not be built") - assert.False(t, waiting, "unresolvable space must not poison the waiting map") - - // Get must keep returning ErrSpaceNotExists, exactly as eager mode does. - _, err := s.getCtrl(context.Background(), spaceId) - require.ErrorIs(t, err, ErrSpaceNotExists) - }) - } -} diff --git a/space/spacefactory/mock_spacefactory/mock_SpaceFactory.go b/space/spacefactory/mock_spacefactory/mock_SpaceFactory.go index c471c66888..dd78c13293 100644 --- a/space/spacefactory/mock_spacefactory/mock_SpaceFactory.go +++ b/space/spacefactory/mock_spacefactory/mock_SpaceFactory.go @@ -30,66 +30,6 @@ func (_m *MockSpaceFactory) EXPECT() *MockSpaceFactory_Expecter { return &MockSpaceFactory_Expecter{mock: &_m.Mock} } -// CreateActiveSpace provides a mock function with given fields: ctx, id, aclHeadId -func (_m *MockSpaceFactory) CreateActiveSpace(ctx context.Context, id string, aclHeadId string) (spacecontroller.SpaceController, error) { - ret := _m.Called(ctx, id, aclHeadId) - - if len(ret) == 0 { - panic("no return value specified for CreateActiveSpace") - } - - var r0 spacecontroller.SpaceController - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) (spacecontroller.SpaceController, error)); ok { - return rf(ctx, id, aclHeadId) - } - if rf, ok := ret.Get(0).(func(context.Context, string, string) spacecontroller.SpaceController); ok { - r0 = rf(ctx, id, aclHeadId) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spacecontroller.SpaceController) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { - r1 = rf(ctx, id, aclHeadId) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockSpaceFactory_CreateActiveSpace_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateActiveSpace' -type MockSpaceFactory_CreateActiveSpace_Call struct { - *mock.Call -} - -// CreateActiveSpace is a helper method to define mock.On call -// - ctx context.Context -// - id string -// - aclHeadId string -func (_e *MockSpaceFactory_Expecter) CreateActiveSpace(ctx interface{}, id interface{}, aclHeadId interface{}) *MockSpaceFactory_CreateActiveSpace_Call { - return &MockSpaceFactory_CreateActiveSpace_Call{Call: _e.mock.On("CreateActiveSpace", ctx, id, aclHeadId)} -} - -func (_c *MockSpaceFactory_CreateActiveSpace_Call) Run(run func(ctx context.Context, id string, aclHeadId string)) *MockSpaceFactory_CreateActiveSpace_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string)) - }) - return _c -} - -func (_c *MockSpaceFactory_CreateActiveSpace_Call) Return(sp spacecontroller.SpaceController, err error) *MockSpaceFactory_CreateActiveSpace_Call { - _c.Call.Return(sp, err) - return _c -} - -func (_c *MockSpaceFactory_CreateActiveSpace_Call) RunAndReturn(run func(context.Context, string, string) (spacecontroller.SpaceController, error)) *MockSpaceFactory_CreateActiveSpace_Call { - _c.Call.Return(run) - return _c -} - // CreateAndSetTechSpace provides a mock function with given fields: ctx func (_m *MockSpaceFactory) CreateAndSetTechSpace(ctx context.Context) (*clientspace.TechSpace, error) { ret := _m.Called(ctx) @@ -148,66 +88,6 @@ func (_c *MockSpaceFactory_CreateAndSetTechSpace_Call) RunAndReturn(run func(con return _c } -// CreateInvitingSpace provides a mock function with given fields: ctx, id, aclHeadId -func (_m *MockSpaceFactory) CreateInvitingSpace(ctx context.Context, id string, aclHeadId string) (spacecontroller.SpaceController, error) { - ret := _m.Called(ctx, id, aclHeadId) - - if len(ret) == 0 { - panic("no return value specified for CreateInvitingSpace") - } - - var r0 spacecontroller.SpaceController - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) (spacecontroller.SpaceController, error)); ok { - return rf(ctx, id, aclHeadId) - } - if rf, ok := ret.Get(0).(func(context.Context, string, string) spacecontroller.SpaceController); ok { - r0 = rf(ctx, id, aclHeadId) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spacecontroller.SpaceController) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { - r1 = rf(ctx, id, aclHeadId) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockSpaceFactory_CreateInvitingSpace_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateInvitingSpace' -type MockSpaceFactory_CreateInvitingSpace_Call struct { - *mock.Call -} - -// CreateInvitingSpace is a helper method to define mock.On call -// - ctx context.Context -// - id string -// - aclHeadId string -func (_e *MockSpaceFactory_Expecter) CreateInvitingSpace(ctx interface{}, id interface{}, aclHeadId interface{}) *MockSpaceFactory_CreateInvitingSpace_Call { - return &MockSpaceFactory_CreateInvitingSpace_Call{Call: _e.mock.On("CreateInvitingSpace", ctx, id, aclHeadId)} -} - -func (_c *MockSpaceFactory_CreateInvitingSpace_Call) Run(run func(ctx context.Context, id string, aclHeadId string)) *MockSpaceFactory_CreateInvitingSpace_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(string), args[2].(string)) - }) - return _c -} - -func (_c *MockSpaceFactory_CreateInvitingSpace_Call) Return(sp spacecontroller.SpaceController, err error) *MockSpaceFactory_CreateInvitingSpace_Call { - _c.Call.Return(sp, err) - return _c -} - -func (_c *MockSpaceFactory_CreateInvitingSpace_Call) RunAndReturn(run func(context.Context, string, string) (spacecontroller.SpaceController, error)) *MockSpaceFactory_CreateInvitingSpace_Call { - _c.Call.Return(run) - return _c -} - // CreateMarketplaceSpace provides a mock function with given fields: ctx func (_m *MockSpaceFactory) CreateMarketplaceSpace(ctx context.Context) (spacecontroller.SpaceController, error) { ret := _m.Called(ctx) @@ -267,33 +147,21 @@ func (_c *MockSpaceFactory_CreateMarketplaceSpace_Call) RunAndReturn(run func(co } // CreateOneToOneSpace provides a mock function with given fields: ctx, id, description, participantData -func (_m *MockSpaceFactory) CreateOneToOneSpace(ctx context.Context, id string, description *spaceinfo.SpaceDescription, participantData spaceinfo.OneToOneParticipantData) (spacecontroller.SpaceController, error) { +func (_m *MockSpaceFactory) CreateOneToOneSpace(ctx context.Context, id string, description *spaceinfo.SpaceDescription, participantData spaceinfo.OneToOneParticipantData) error { ret := _m.Called(ctx, id, description, participantData) if len(ret) == 0 { panic("no return value specified for CreateOneToOneSpace") } - var r0 spacecontroller.SpaceController - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, *spaceinfo.SpaceDescription, spaceinfo.OneToOneParticipantData) (spacecontroller.SpaceController, error)); ok { - return rf(ctx, id, description, participantData) - } - if rf, ok := ret.Get(0).(func(context.Context, string, *spaceinfo.SpaceDescription, spaceinfo.OneToOneParticipantData) spacecontroller.SpaceController); ok { + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, *spaceinfo.SpaceDescription, spaceinfo.OneToOneParticipantData) error); ok { r0 = rf(ctx, id, description, participantData) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spacecontroller.SpaceController) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string, *spaceinfo.SpaceDescription, spaceinfo.OneToOneParticipantData) error); ok { - r1 = rf(ctx, id, description, participantData) - } else { - r1 = ret.Error(1) + r0 = ret.Error(0) } - return r0, r1 + return r0 } // MockSpaceFactory_CreateOneToOneSpace_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateOneToOneSpace' @@ -317,103 +185,32 @@ func (_c *MockSpaceFactory_CreateOneToOneSpace_Call) Run(run func(ctx context.Co return _c } -func (_c *MockSpaceFactory_CreateOneToOneSpace_Call) Return(sp spacecontroller.SpaceController, err error) *MockSpaceFactory_CreateOneToOneSpace_Call { - _c.Call.Return(sp, err) +func (_c *MockSpaceFactory_CreateOneToOneSpace_Call) Return(_a0 error) *MockSpaceFactory_CreateOneToOneSpace_Call { + _c.Call.Return(_a0) return _c } -func (_c *MockSpaceFactory_CreateOneToOneSpace_Call) RunAndReturn(run func(context.Context, string, *spaceinfo.SpaceDescription, spaceinfo.OneToOneParticipantData) (spacecontroller.SpaceController, error)) *MockSpaceFactory_CreateOneToOneSpace_Call { - _c.Call.Return(run) - return _c -} - -// CreatePersonalSpace provides a mock function with given fields: ctx, metadata -func (_m *MockSpaceFactory) CreatePersonalSpace(ctx context.Context, metadata []byte) (spacecontroller.SpaceController, error) { - ret := _m.Called(ctx, metadata) - - if len(ret) == 0 { - panic("no return value specified for CreatePersonalSpace") - } - - var r0 spacecontroller.SpaceController - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, []byte) (spacecontroller.SpaceController, error)); ok { - return rf(ctx, metadata) - } - if rf, ok := ret.Get(0).(func(context.Context, []byte) spacecontroller.SpaceController); ok { - r0 = rf(ctx, metadata) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spacecontroller.SpaceController) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, []byte) error); ok { - r1 = rf(ctx, metadata) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// MockSpaceFactory_CreatePersonalSpace_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreatePersonalSpace' -type MockSpaceFactory_CreatePersonalSpace_Call struct { - *mock.Call -} - -// CreatePersonalSpace is a helper method to define mock.On call -// - ctx context.Context -// - metadata []byte -func (_e *MockSpaceFactory_Expecter) CreatePersonalSpace(ctx interface{}, metadata interface{}) *MockSpaceFactory_CreatePersonalSpace_Call { - return &MockSpaceFactory_CreatePersonalSpace_Call{Call: _e.mock.On("CreatePersonalSpace", ctx, metadata)} -} - -func (_c *MockSpaceFactory_CreatePersonalSpace_Call) Run(run func(ctx context.Context, metadata []byte)) *MockSpaceFactory_CreatePersonalSpace_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].([]byte)) - }) - return _c -} - -func (_c *MockSpaceFactory_CreatePersonalSpace_Call) Return(sp spacecontroller.SpaceController, err error) *MockSpaceFactory_CreatePersonalSpace_Call { - _c.Call.Return(sp, err) - return _c -} - -func (_c *MockSpaceFactory_CreatePersonalSpace_Call) RunAndReturn(run func(context.Context, []byte) (spacecontroller.SpaceController, error)) *MockSpaceFactory_CreatePersonalSpace_Call { +func (_c *MockSpaceFactory_CreateOneToOneSpace_Call) RunAndReturn(run func(context.Context, string, *spaceinfo.SpaceDescription, spaceinfo.OneToOneParticipantData) error) *MockSpaceFactory_CreateOneToOneSpace_Call { _c.Call.Return(run) return _c } // CreateShareableSpace provides a mock function with given fields: ctx, id, desc -func (_m *MockSpaceFactory) CreateShareableSpace(ctx context.Context, id string, desc *spaceinfo.SpaceDescription) (spacecontroller.SpaceController, error) { +func (_m *MockSpaceFactory) CreateShareableSpace(ctx context.Context, id string, desc *spaceinfo.SpaceDescription) error { ret := _m.Called(ctx, id, desc) if len(ret) == 0 { panic("no return value specified for CreateShareableSpace") } - var r0 spacecontroller.SpaceController - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, string, *spaceinfo.SpaceDescription) (spacecontroller.SpaceController, error)); ok { - return rf(ctx, id, desc) - } - if rf, ok := ret.Get(0).(func(context.Context, string, *spaceinfo.SpaceDescription) spacecontroller.SpaceController); ok { + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, *spaceinfo.SpaceDescription) error); ok { r0 = rf(ctx, id, desc) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spacecontroller.SpaceController) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string, *spaceinfo.SpaceDescription) error); ok { - r1 = rf(ctx, id, desc) - } else { - r1 = ret.Error(1) + r0 = ret.Error(0) } - return r0, r1 + return r0 } // MockSpaceFactory_CreateShareableSpace_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateShareableSpace' @@ -436,44 +233,32 @@ func (_c *MockSpaceFactory_CreateShareableSpace_Call) Run(run func(ctx context.C return _c } -func (_c *MockSpaceFactory_CreateShareableSpace_Call) Return(sp spacecontroller.SpaceController, err error) *MockSpaceFactory_CreateShareableSpace_Call { - _c.Call.Return(sp, err) +func (_c *MockSpaceFactory_CreateShareableSpace_Call) Return(_a0 error) *MockSpaceFactory_CreateShareableSpace_Call { + _c.Call.Return(_a0) return _c } -func (_c *MockSpaceFactory_CreateShareableSpace_Call) RunAndReturn(run func(context.Context, string, *spaceinfo.SpaceDescription) (spacecontroller.SpaceController, error)) *MockSpaceFactory_CreateShareableSpace_Call { +func (_c *MockSpaceFactory_CreateShareableSpace_Call) RunAndReturn(run func(context.Context, string, *spaceinfo.SpaceDescription) error) *MockSpaceFactory_CreateShareableSpace_Call { _c.Call.Return(run) return _c } -// CreateStreamableSpace provides a mock function with given fields: ctx, privKey, id, metadata -func (_m *MockSpaceFactory) CreateStreamableSpace(ctx context.Context, privKey crypto.PrivKey, id string, metadata []byte) (spacecontroller.SpaceController, error) { - ret := _m.Called(ctx, privKey, id, metadata) +// CreateStreamableSpace provides a mock function with given fields: ctx, privKey, id +func (_m *MockSpaceFactory) CreateStreamableSpace(ctx context.Context, privKey crypto.PrivKey, id string) error { + ret := _m.Called(ctx, privKey, id) if len(ret) == 0 { panic("no return value specified for CreateStreamableSpace") } - var r0 spacecontroller.SpaceController - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, crypto.PrivKey, string, []byte) (spacecontroller.SpaceController, error)); ok { - return rf(ctx, privKey, id, metadata) - } - if rf, ok := ret.Get(0).(func(context.Context, crypto.PrivKey, string, []byte) spacecontroller.SpaceController); ok { - r0 = rf(ctx, privKey, id, metadata) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(spacecontroller.SpaceController) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, crypto.PrivKey, string, []byte) error); ok { - r1 = rf(ctx, privKey, id, metadata) + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, crypto.PrivKey, string) error); ok { + r0 = rf(ctx, privKey, id) } else { - r1 = ret.Error(1) + r0 = ret.Error(0) } - return r0, r1 + return r0 } // MockSpaceFactory_CreateStreamableSpace_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'CreateStreamableSpace' @@ -485,24 +270,23 @@ type MockSpaceFactory_CreateStreamableSpace_Call struct { // - ctx context.Context // - privKey crypto.PrivKey // - id string -// - metadata []byte -func (_e *MockSpaceFactory_Expecter) CreateStreamableSpace(ctx interface{}, privKey interface{}, id interface{}, metadata interface{}) *MockSpaceFactory_CreateStreamableSpace_Call { - return &MockSpaceFactory_CreateStreamableSpace_Call{Call: _e.mock.On("CreateStreamableSpace", ctx, privKey, id, metadata)} +func (_e *MockSpaceFactory_Expecter) CreateStreamableSpace(ctx interface{}, privKey interface{}, id interface{}) *MockSpaceFactory_CreateStreamableSpace_Call { + return &MockSpaceFactory_CreateStreamableSpace_Call{Call: _e.mock.On("CreateStreamableSpace", ctx, privKey, id)} } -func (_c *MockSpaceFactory_CreateStreamableSpace_Call) Run(run func(ctx context.Context, privKey crypto.PrivKey, id string, metadata []byte)) *MockSpaceFactory_CreateStreamableSpace_Call { +func (_c *MockSpaceFactory_CreateStreamableSpace_Call) Run(run func(ctx context.Context, privKey crypto.PrivKey, id string)) *MockSpaceFactory_CreateStreamableSpace_Call { _c.Call.Run(func(args mock.Arguments) { - run(args[0].(context.Context), args[1].(crypto.PrivKey), args[2].(string), args[3].([]byte)) + run(args[0].(context.Context), args[1].(crypto.PrivKey), args[2].(string)) }) return _c } -func (_c *MockSpaceFactory_CreateStreamableSpace_Call) Return(_a0 spacecontroller.SpaceController, _a1 error) *MockSpaceFactory_CreateStreamableSpace_Call { - _c.Call.Return(_a0, _a1) +func (_c *MockSpaceFactory_CreateStreamableSpace_Call) Return(_a0 error) *MockSpaceFactory_CreateStreamableSpace_Call { + _c.Call.Return(_a0) return _c } -func (_c *MockSpaceFactory_CreateStreamableSpace_Call) RunAndReturn(run func(context.Context, crypto.PrivKey, string, []byte) (spacecontroller.SpaceController, error)) *MockSpaceFactory_CreateStreamableSpace_Call { +func (_c *MockSpaceFactory_CreateStreamableSpace_Call) RunAndReturn(run func(context.Context, crypto.PrivKey, string) error) *MockSpaceFactory_CreateStreamableSpace_Call { _c.Call.Return(run) return _c } diff --git a/space/spacefactory/spacefactory.go b/space/spacefactory/spacefactory.go index c6594cecab..911ba1270a 100644 --- a/space/spacefactory/spacefactory.go +++ b/space/spacefactory/spacefactory.go @@ -36,12 +36,11 @@ import ( "github.com/anyproto/anytype-heart/core/block/object/objectcache" "github.com/anyproto/anytype-heart/space/clientspace" + "github.com/anyproto/anytype-heart/space/internal/accountspace" dependencies "github.com/anyproto/anytype-heart/space/internal/components/dependencies" + "github.com/anyproto/anytype-heart/space/internal/components/personalmigration" "github.com/anyproto/anytype-heart/space/internal/marketplacespace" - "github.com/anyproto/anytype-heart/space/internal/personalspace" - "github.com/anyproto/anytype-heart/space/internal/shareablespace" "github.com/anyproto/anytype-heart/space/internal/spacecontroller" - "github.com/anyproto/anytype-heart/space/internal/streamablespace" "github.com/anyproto/anytype-heart/space/spacecore" "github.com/anyproto/anytype-heart/space/spacecore/storage" "github.com/anyproto/anytype-heart/space/spacecore/storage/anystorage" @@ -50,24 +49,37 @@ import ( "github.com/anyproto/anytype-heart/space/techspace" ) +// SpaceFactory constructs space controllers (New*) and creates the durable +// artifacts of new spaces — storage marks and space views (Create*). +// Controllers are registered exclusively through space view events, so the +// Create* methods do not build controllers; the watcher does. type SpaceFactory interface { app.Component - CreatePersonalSpace(ctx context.Context, metadata []byte) (sp spacecontroller.SpaceController, err error) NewPersonalSpace(ctx context.Context, metadata []byte) (spacecontroller.SpaceController, error) - CreateShareableSpace(ctx context.Context, id string, desc *spaceinfo.SpaceDescription) (sp spacecontroller.SpaceController, err error) NewShareableSpace(ctx context.Context, id string, info spaceinfo.SpacePersistentInfo) (spacecontroller.SpaceController, error) - CreateStreamableSpace(ctx context.Context, privKey crypto.PrivKey, id string, metadata []byte) (spacecontroller.SpaceController, error) NewStreamableSpace(ctx context.Context, id string, info spaceinfo.SpacePersistentInfo, metadata []byte) (spacecontroller.SpaceController, error) - CreateActiveSpace(ctx context.Context, id, aclHeadId string) (sp spacecontroller.SpaceController, err error) + CreateShareableSpace(ctx context.Context, id string, desc *spaceinfo.SpaceDescription) error + CreateStreamableSpace(ctx context.Context, privKey crypto.PrivKey, id string) error + CreateOneToOneSpace(ctx context.Context, id string, description *spaceinfo.SpaceDescription, participantData spaceinfo.OneToOneParticipantData) error CreateMarketplaceSpace(ctx context.Context) (sp spacecontroller.SpaceController, err error) CreateAndSetTechSpace(ctx context.Context) (*clientspace.TechSpace, error) LoadAndSetTechSpace(ctx context.Context) (*clientspace.TechSpace, error) - CreateInvitingSpace(ctx context.Context, id, aclHeadId string) (sp spacecontroller.SpaceController, err error) - CreateOneToOneSpace(ctx context.Context, id string, description *spaceinfo.SpaceDescription, participantData spaceinfo.OneToOneParticipantData) (sp spacecontroller.SpaceController, err error) } const CName = "client.space.spacefactory" +type ctxKey int + +// SkipCheckSpaceViewKey, when set to true in ctx, makes NewPersonalSpace create +// the personal space view unconditionally instead of probing for it first. +// Used when restoring old accounts that never had a space view. +const SkipCheckSpaceViewKey ctxKey = iota + +func shouldCheckSpaceView(ctx context.Context) bool { + skip, ok := ctx.Value(SkipCheckSpaceViewKey).(bool) + return !ok || !skip +} + type spaceFactory struct { app *app.App spaceCore spacecore.SpaceCoreService @@ -99,42 +111,47 @@ func (s *spaceFactory) Init(a *app.App) (err error) { return } -func (s *spaceFactory) CreatePersonalSpace(ctx context.Context, metadata []byte) (sp spacecontroller.SpaceController, err error) { - coreSpace, err := s.spaceCore.Derive(ctx, spacedomain.SpaceTypeRegular) - if err != nil { - return - } - err = coreSpace.Storage().(anystorage.ClientSpaceStorage).MarkSpaceCreated(ctx) +func (s *spaceFactory) NewPersonalSpace(ctx context.Context, metadata []byte) (ctrl spacecontroller.SpaceController, err error) { + id, err := s.spaceCore.DeriveID(ctx, spacedomain.SpaceTypeRegular) if err != nil { - return - } - info := spaceinfo.NewSpacePersistentInfo(coreSpace.Id()) - info.SetAccountStatus(spaceinfo.AccountStatusUnknown) - if err := s.techSpace.SpaceViewCreate(ctx, coreSpace.Id(), true, info, nil); err != nil { - if errors.Is(err, techspace.ErrSpaceViewExists) { - return s.NewPersonalSpace(ctx, metadata) - } return nil, err } - ctrl, err := personalspace.NewSpaceController(ctx, coreSpace.Id(), metadata, s.app) - if err != nil { + if err = s.ensurePersonalSpaceView(ctx, id); err != nil { return nil, err } - err = ctrl.Start(ctx) - return ctrl, err + return s.newPersonalController(id, metadata) } -func (s *spaceFactory) NewPersonalSpace(ctx context.Context, metadata []byte) (ctrl spacecontroller.SpaceController, err error) { - id, err := s.spaceCore.DeriveID(ctx, spacedomain.SpaceTypeRegular) - if err != nil { - return nil, err - } - ctrl, err = personalspace.NewSpaceController(ctx, id, metadata, s.app) - if err != nil { - return nil, err +// ensurePersonalSpaceView creates the personal space view if it is missing. +// Old accounts predate space views, so the view may need to be created on +// first start; the spacestatus component requires it to exist. +func (s *spaceFactory) ensurePersonalSpaceView(ctx context.Context, id string) error { + var ( + exists bool + err error + ) + if shouldCheckSpaceView(ctx) { + exists, err = s.techSpace.SpaceViewExists(ctx, id) + } + if !exists || err != nil { + info := spaceinfo.NewSpacePersistentInfo(id) + info.SetAccountStatus(spaceinfo.AccountStatusUnknown) + if err := s.techSpace.SpaceViewCreate(ctx, id, false, info, nil); err != nil { + return err + } } - err = ctrl.Start(ctx) - return ctrl, err + return nil +} + +func (s *spaceFactory) newPersonalController(id string, metadata []byte) (spacecontroller.SpaceController, error) { + return accountspace.NewSpaceController(accountspace.Descriptor{ + SpaceId: id, + IsPersonal: true, + OwnerMetadata: metadata, + ExtraLoaderComponents: func() []app.Component { + return []app.Component{personalmigration.New()} + }, + }, s.app) } func (s *spaceFactory) CreateAndSetTechSpace(ctx context.Context) (*clientspace.TechSpace, error) { @@ -210,56 +227,18 @@ func (s *spaceFactory) LoadAndSetTechSpace(ctx context.Context) (*clientspace.Te } func (s *spaceFactory) NewShareableSpace(ctx context.Context, id string, info spaceinfo.SpacePersistentInfo) (spacecontroller.SpaceController, error) { - ctrl, err := shareablespace.NewSpaceController(id, info, s.app) - if err != nil { - return nil, err - } - err = ctrl.Start(ctx) - return ctrl, err -} - -func (s *spaceFactory) CreateInvitingSpace(ctx context.Context, id, aclHeadId string) (sp spacecontroller.SpaceController, err error) { - exists, err := s.techSpace.SpaceViewExists(ctx, id) - if err != nil { - return - } - info := spaceinfo.NewSpacePersistentInfo(id) - info.SetAclHeadId(aclHeadId).SetAccountStatus(spaceinfo.AccountStatusJoining) - if !exists { - if err := s.techSpace.SpaceViewCreate(ctx, id, true, info, nil); err != nil { - return nil, err - } - } - ctrl, err := shareablespace.NewSpaceController(id, info, s.app) - if err != nil { - return nil, err - } - err = ctrl.Start(ctx) - return ctrl, err + return s.newShareableController(id) } -func (s *spaceFactory) CreateActiveSpace(ctx context.Context, id, aclHeadId string) (sp spacecontroller.SpaceController, err error) { - exists, err := s.techSpace.SpaceViewExists(ctx, id) - if err != nil { - return - } - info := spaceinfo.NewSpacePersistentInfo(id) - info.SetAclHeadId(aclHeadId).SetAccountStatus(spaceinfo.AccountStatusActive) - if !exists { - if err := s.techSpace.SpaceViewCreate(ctx, id, true, info, nil); err != nil { - return nil, err - } - } - ctrl, err := shareablespace.NewSpaceController(id, info, s.app) - if err != nil { - return nil, err - } - err = ctrl.Start(ctx) - return ctrl, err +func (s *spaceFactory) newShareableController(id string) (spacecontroller.SpaceController, error) { + return accountspace.NewSpaceController(accountspace.Descriptor{ + SpaceId: id, + }, s.app) } -// creates regular shared space -func (s *spaceFactory) CreateShareableSpace(ctx context.Context, id string, spaceDesc *spaceinfo.SpaceDescription) (sp spacecontroller.SpaceController, err error) { +// CreateShareableSpace marks the freshly created space storage and creates +// its space view; the watcher registers the controller from the view event. +func (s *spaceFactory) CreateShareableSpace(ctx context.Context, id string, spaceDesc *spaceinfo.SpaceDescription) (err error) { coreSpace, err := s.spaceCore.Get(ctx, id) if err != nil { return @@ -270,29 +249,20 @@ func (s *spaceFactory) CreateShareableSpace(ctx context.Context, id string, spac } info := spaceinfo.NewSpacePersistentInfo(id) info.SetAccountStatus(spaceinfo.AccountStatusUnknown) - if err := s.techSpace.SpaceViewCreate(ctx, id, true, info, spaceDesc); err != nil { - return nil, err - } - ctrl, err := shareablespace.NewSpaceController(id, info, s.app) - if err != nil { - return nil, err - } - err = ctrl.Start(ctx) - return ctrl, err + return s.techSpace.SpaceViewCreate(ctx, id, true, info, spaceDesc) } -func (s *spaceFactory) CreateStreamableSpace(ctx context.Context, privKey crypto.PrivKey, id string, metadata []byte) (spacecontroller.SpaceController, error) { +// CreateStreamableSpace creates a space view carrying the encoded guest key; +// the watcher registers a streamable controller from the view event. +func (s *spaceFactory) CreateStreamableSpace(ctx context.Context, privKey crypto.PrivKey, id string) error { encodedKey, err := crypto.EncodeKeyToString(privKey) if err != nil { - return nil, err + return fmt.Errorf("encode guest key: %w", err) } info := spaceinfo.NewSpacePersistentInfo(id) info.SetAccountStatus(spaceinfo.AccountStatusUnknown). SetEncodedKey(encodedKey) - if err := s.techSpace.SpaceViewCreate(ctx, id, false, info, nil); err != nil { - return nil, err - } - return s.NewStreamableSpace(ctx, id, info, metadata) + return s.techSpace.SpaceViewCreate(ctx, id, false, info, nil) } func (s *spaceFactory) NewStreamableSpace(ctx context.Context, id string, info spaceinfo.SpacePersistentInfo, metadata []byte) (spacecontroller.SpaceController, error) { @@ -300,21 +270,26 @@ func (s *spaceFactory) NewStreamableSpace(ctx context.Context, id string, info s info.EncodedKey, crypto.UnmarshalEd25519PrivateKey, nil) - ctrl, err := streamablespace.NewSpaceController(ctx, id, decodedSignKey, metadata, s.app) if err != nil { - return nil, err + return nil, fmt.Errorf("decode streamable space key: %w", err) } - err = ctrl.Start(ctx) - return ctrl, err + return accountspace.NewSpaceController(accountspace.Descriptor{ + SpaceId: id, + GuestKey: decodedSignKey, + OwnerMetadata: metadata, + }, s.app) } +// CreateMarketplaceSpace constructs the marketplace controller; the caller +// starts it (initMarketplaceSpace). func (s *spaceFactory) CreateMarketplaceSpace(ctx context.Context) (sp spacecontroller.SpaceController, err error) { - ctrl := marketplacespace.NewSpaceController(s.app, s.personalSpaceId) - err = ctrl.Start(ctx) - return ctrl, err + return marketplacespace.NewSpaceController(s.app, s.personalSpaceId), nil } -func (s *spaceFactory) CreateOneToOneSpace(ctx context.Context, spaceId string, description *spaceinfo.SpaceDescription, participantData spaceinfo.OneToOneParticipantData) (sp spacecontroller.SpaceController, err error) { +// CreateOneToOneSpace marks the one-to-one space storage and creates or +// repairs its space view; the watcher registers the controller from the view +// event. +func (s *spaceFactory) CreateOneToOneSpace(ctx context.Context, spaceId string, description *spaceinfo.SpaceDescription, participantData spaceinfo.OneToOneParticipantData) (err error) { oneToOneSpace, err := s.spaceCore.Get(ctx, spaceId) if err != nil { return @@ -335,14 +310,14 @@ func (s *spaceFactory) CreateOneToOneSpace(ctx context.Context, spaceId string, spaceView, err := s.techSpace.GetSpaceView(ctx, spaceId) if err != nil { if !errors.Is(err, techspace.ErrSpaceViewNotExists) { - return nil, fmt.Errorf("get space view: %w", err) + return fmt.Errorf("get space view: %w", err) } } // nolint: nestif if spaceView == nil { if err := s.techSpace.SpaceViewCreate(ctx, spaceId, true, info, description); err != nil { - return nil, err + return err } } else { // check if space is active @@ -353,20 +328,15 @@ func (s *spaceFactory) CreateOneToOneSpace(ctx context.Context, spaceId string, localInfo.SetLocalStatus(spaceinfo.LocalStatusUnknown) localInfo.SetRemoteStatus(spaceinfo.RemoteStatusUnknown) if err := spaceView.SetSpaceLocalInfo(localInfo); err != nil { - return nil, err + return err } if err := spaceView.SetSpacePersistentInfo(info); err != nil { - return nil, err + return err } } } - ctrl, err := shareablespace.NewSpaceController(spaceId, info, s.app) - if err != nil { - return nil, err - } - err = ctrl.Start(ctx) - return ctrl, err + return nil } func (s *spaceFactory) Name() (name string) { diff --git a/space/streamable.go b/space/streamable.go index 065b31621c..666b56c969 100644 --- a/space/streamable.go +++ b/space/streamable.go @@ -2,37 +2,33 @@ package space import ( "context" + "fmt" "github.com/anyproto/any-sync/util/crypto" + + "github.com/anyproto/anytype-heart/space/internal/spaceprocess/mode" ) +// AddStreamable is unidirectional: it creates the space view carrying the +// guest key and waits for the watcher-registered controller, which the view's +// EncodedKey makes streamable. func (s *service) AddStreamable(ctx context.Context, id string, guestKey crypto.PrivKey) (err error) { - s.mu.Lock() - waiter, exists := s.waiting[id] - if exists { - s.mu.Unlock() - <-waiter.wait - return waiter.err - } - wait := make(chan struct{}) - s.waiting[id] = controllerWaiter{ - wait: wait, + if s.isClosing.Load() { + return ErrSpaceIsClosing } - s.mu.Unlock() - ctrl, err := s.factory.CreateStreamableSpace(ctx, guestKey, id, s.accountMetadataPayload) + exists, err := s.techSpace.SpaceViewExists(ctx, id) if err != nil { - s.mu.Lock() - close(wait) - s.waiting[id] = controllerWaiter{ - wait: wait, - err: err, + return fmt.Errorf("check space view: %w", err) + } + if !exists { + if err := s.factory.CreateStreamableSpace(ctx, guestKey, id); err != nil { + return err } - s.mu.Unlock() + } + ctrl, err := s.waitCtrl(ctx, id) + if err != nil { return err } - s.mu.Lock() - close(wait) - s.spaceControllers[ctrl.SpaceId()] = ctrl - s.mu.Unlock() - return nil + ctrl.Demand() + return s.waitIntentMode(ctx, ctrl, mode.ModeLoading) } diff --git a/space/waiter.go b/space/waiter.go deleted file mode 100644 index 5fe315a8ea..0000000000 --- a/space/waiter.go +++ /dev/null @@ -1,69 +0,0 @@ -package space - -import ( - "context" - "fmt" - "time" - - "github.com/anyproto/anytype-heart/pkg/lib/localstore/addr" - "github.com/anyproto/anytype-heart/space/clientspace" -) - -func (s *service) checkControllerExists(spaceId string) bool { - s.mu.Lock() - _, ctrlOk := s.spaceControllers[spaceId] - _, waitingOk := s.waiting[spaceId] - s.mu.Unlock() - return ctrlOk || waitingOk -} - -type waiterService interface { - TechSpace() *clientspace.TechSpace - Get(ctx context.Context, spaceId string) (clientspace.Space, error) - checkControllerExists(spaceId string) bool - ensureSpaceStarted(spaceId string) -} - -type spaceWaiter struct { - svc waiterService - svcCtx context.Context - retryDelay time.Duration -} - -func newSpaceWaiter(svc waiterService, svcCtx context.Context, retryDelay time.Duration) *spaceWaiter { - return &spaceWaiter{svc: svc, svcCtx: svcCtx, retryDelay: retryDelay} -} - -func (w *spaceWaiter) waitSpace(ctx context.Context, spaceId string) (sp clientspace.Space, err error) { - techSpace := w.svc.TechSpace() - if spaceId == techSpace.TechSpaceId() { - return techSpace, nil - } - // if there is no such space view then there is no space - if spaceId != addr.AnytypeMarketplaceWorkspace { - exists, err := techSpace.SpaceViewExists(ctx, spaceId) - if err != nil { - // func returns error only on derive - return nil, fmt.Errorf("space view derive error: %w", err) - } - if !exists { - return nil, ErrSpaceNotExists - } - } - // Lazy multi-space loading: in lazy mode the watcher does not eagerly - // build deferred spaces, so promote this one on demand before waiting - // for its controller to appear. No-op in eager mode. - w.svc.ensureSpaceStarted(spaceId) - // we should wait a bit until the controller is created - for !w.svc.checkControllerExists(spaceId) { - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-w.svcCtx.Done(): - return nil, w.svcCtx.Err() - case <-time.After(w.retryDelay): - break - } - } - return w.svc.Get(ctx, spaceId) -} diff --git a/space/waiter_test.go b/space/waiter_test.go deleted file mode 100644 index 4cbfa5985c..0000000000 --- a/space/waiter_test.go +++ /dev/null @@ -1,114 +0,0 @@ -package space - -import ( - "context" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/anyproto/anytype-heart/space/clientspace" - "github.com/anyproto/anytype-heart/space/clientspace/mock_clientspace" - "github.com/anyproto/anytype-heart/space/techspace" - "github.com/anyproto/anytype-heart/space/techspace/mock_techspace" -) - -func TestWaiter_Wait(t *testing.T) { - t.Run("space exists", func(t *testing.T) { - mockTechSpace := mock_techspace.NewMockTechSpace(t) - mockClientSpace := mock_clientspace.NewMockSpace(t) - retryDelay := time.Millisecond - stub := &waiterStub{ - clientSpace: mockClientSpace, - techSpace: mockTechSpace, - exists: []bool{true}, - } - wtr := newSpaceWaiter(stub, ctx, retryDelay) - mockTechSpace.EXPECT().TechSpaceId().Return("techSpaceId") - mockTechSpace.EXPECT().SpaceViewExists(ctx, "spaceId").Return(true, nil) - res, err := wtr.waitSpace(ctx, "spaceId") - require.NoError(t, err) - require.NotNil(t, res) - }) - t.Run("wait multiple times", func(t *testing.T) { - mockTechSpace := mock_techspace.NewMockTechSpace(t) - mockClientSpace := mock_clientspace.NewMockSpace(t) - retryDelay := time.Millisecond - stub := &waiterStub{ - clientSpace: mockClientSpace, - techSpace: mockTechSpace, - exists: []bool{false, false, true}, - } - wtr := newSpaceWaiter(stub, ctx, retryDelay) - mockTechSpace.EXPECT().TechSpaceId().Return("techSpaceId") - mockTechSpace.EXPECT().SpaceViewExists(ctx, "spaceId").Return(true, nil) - res, err := wtr.waitSpace(ctx, "spaceId") - require.NoError(t, err) - require.NotNil(t, res) - require.Equal(t, 3, stub.cntr) - }) - t.Run("cancel wait, service closed", func(t *testing.T) { - mockTechSpace := mock_techspace.NewMockTechSpace(t) - mockClientSpace := mock_clientspace.NewMockSpace(t) - retryDelay := time.Second - stub := &waiterStub{ - clientSpace: mockClientSpace, - techSpace: mockTechSpace, - exists: []bool{false, false, true}, - } - cancelCtx, cancel := context.WithCancel(context.Background()) - wtr := newSpaceWaiter(stub, cancelCtx, retryDelay) - mockTechSpace.EXPECT().TechSpaceId().Return("techSpaceId") - mockTechSpace.EXPECT().SpaceViewExists(ctx, "spaceId").Return(true, nil) - cancel() - res, err := wtr.waitSpace(ctx, "spaceId") - require.Error(t, err) - require.Nil(t, res) - require.Equal(t, 1, stub.cntr) - }) - t.Run("space view not exists", func(t *testing.T) { - mockTechSpace := mock_techspace.NewMockTechSpace(t) - mockClientSpace := mock_clientspace.NewMockSpace(t) - retryDelay := time.Millisecond - stub := &waiterStub{ - clientSpace: mockClientSpace, - techSpace: mockTechSpace, - exists: []bool{true}, - } - wtr := newSpaceWaiter(stub, ctx, retryDelay) - mockTechSpace.EXPECT().TechSpaceId().Return("techSpaceId") - mockTechSpace.EXPECT().SpaceViewExists(ctx, "spaceId").Return(false, nil) - _, err := wtr.waitSpace(ctx, "spaceId") - require.Equal(t, ErrSpaceNotExists, err) - }) -} - -type waiterStub struct { - techSpace techspace.TechSpace - clientSpace clientspace.Space - err error - exists []bool - cntr int -} - -func (w *waiterStub) TechSpace() *clientspace.TechSpace { - return &clientspace.TechSpace{ - TechSpace: w.techSpace, - } -} - -func (w *waiterStub) Get(ctx context.Context, spaceId string) (clientspace.Space, error) { - if w.err != nil { - return nil, w.err - } - return w.clientSpace, nil -} - -func (w *waiterStub) ensureSpaceStarted(spaceId string) {} - -func (w *waiterStub) checkControllerExists(spaceId string) bool { - defer func() { - w.cntr++ - }() - return w.exists[w.cntr] -}