diff --git a/client/bin/app.ts b/client/bin/app.ts index 8f5c383..98f753f 100755 --- a/client/bin/app.ts +++ b/client/bin/app.ts @@ -6,9 +6,9 @@ import { InoPosRef } from '../lib/gumnut/types.ts'; import { connectToPulse } from '../lib/pulse/conn.ts'; import type { InPacket, OutPacket } from '../lib/ymodel/types.ts'; -const FIXED_STRING = 'fixed-string'; -const FIXED_CURSORS = 'cursors'; -const FIXED_NUM = 'num'; +const FIXED_STRING = 'n:100'; +const FIXED_CURSORS = 'n:101'; +const FIXED_NUM = 'n:102'; const c = new AbortController(); const e = new TermEditor(c.signal); @@ -135,6 +135,13 @@ conn.ready.catch((err) => { const sess = conn.joinSession(c.signal, 'fake-doc-id', 'bar'); let version = 0; +// TODO: we can write data to whatever "local" negative ID we want +// this shows up with our actual clientID to other users +floor.perform(function* (t) { + const whatever = t.byIno('c:-1:whatever'); + whatever.dataUpdate(0, 0, 'hello', 'there'); +}); + for (;;) { await Promise.race([sess.in.wait(), floor.wait()]); if (sess.in.closed) { diff --git a/client/lib/gumnut/data.ts b/client/lib/gumnut/data.ts index 8d53786..421a5a6 100644 --- a/client/lib/gumnut/data.ts +++ b/client/lib/gumnut/data.ts @@ -1,8 +1,6 @@ import { SimpleCache, todoSignal } from 'thorish'; import type { FloorReadApi, GumnutFloor } from './floor.ts'; -import { emptySymbol, type GumnutData, InoPosRef } from './types.ts'; - -const leadingRef = '^'; +import { emptySymbol, type GumnutData, InoPosRef, InoRef, InoRefImpl } from './types.ts'; export type DataTypeInterface = DataMap | DataArray | DataString; @@ -16,7 +14,7 @@ export type DataType = | string | bigint // not-regular-JS types - | InoPosRef + | InoPosRef // TODO: special, we should do... something? | typeof emptySymbol; export function buildDataApi(gf: GumnutFloor): DataApi { @@ -25,10 +23,12 @@ export function buildDataApi(gf: GumnutFloor): DataApi { export interface DataApi { root(): DataMap; + newMap(): DataMap; } class DataApiImpl implements DataApi { public readonly byId: (id: string) => DataTypeInterface; + private internalId = 0; constructor(public readonly gf: GumnutFloor) { this.byId = this.cache.get.bind(this.cache); @@ -38,37 +38,32 @@ class DataApiImpl implements DataApi { if (name.endsWith(':m')) { return new DataMapImpl(this, name); } - throw new Error(`TODO: unhandled ${name}`); + throw new Error(`TODO: unhandled ${JSON.stringify(name)}`); }); root(): DataMap { - return this.cache.get('root:m') as DataMap; + return this.cache.get('n:0:m') as DataMap; + } + + newMap(): DataMap { + const id = ++this.internalId; + return new DataMapImpl(this, `n:-${id.toString(16)}:m`); } /** * Exists to steal/ref to other "...data" types. */ convertToDataType(data: GumnutData): DataType { - if (typeof data === 'string') { - if (data[0] === leadingRef) { - if (data[1] !== leadingRef) { - return this.byId(data.slice(1)); - } - data = data.slice(1); // unescape leading ref - } - return data; // intern string + if (data instanceof InoRef) { + return this.byId(data.target); } - return data; } fromDataType(data: DataType): GumnutData { - if (typeof data === 'string' && data[0] === leadingRef) { - return leadingRef + data; // escape leading ref - } - if (data instanceof DataMap) { - return leadingRef + (data as DataMapImpl).name; + const name = (data as DataMapImpl).name; + return new InoRefImpl(name); // FIXME: should this be == comparable? } else if (data instanceof DataArray) { throw 'TODO DataArray'; } else if (data instanceof DataString) { @@ -154,9 +149,11 @@ class DataMapImpl extends DataMap { } // not found, insert - ino.dataUpdate(d.length, 0, k, v); + const res = ino.dataUpdate(d.length, 0, k, v); outer.refreshCache(); // TODO: we can be more surgical? yield [outer.name]; + + // FIXME: if we retry txn, we need to abort `res` - currently void! } }); } diff --git a/client/lib/gumnut/types.ts b/client/lib/gumnut/types.ts index 98e1abf..e24b32d 100644 --- a/client/lib/gumnut/types.ts +++ b/client/lib/gumnut/types.ts @@ -10,20 +10,46 @@ export type GumnutData = | number | bigint | string // intern string + | InoRef | InoPosRef | typeof emptySymbol; // explicit void, when op is run without set export const emptySymbol = Symbol('zero'); -export abstract class InoPosRef { +const ctorPrivateSymbol = Symbol('private'); + +/** + * Describes a reference to another inode. + */ +export abstract class InoRef { + constructor(privateSymbol: Symbol) { + if (ctorPrivateSymbol !== privateSymbol) { + throw new Error(`cannot directly create InoRef`); + } + } + + /** + * The other inode being referenced. + */ public abstract readonly target: string; } +/** + * Describes a reference to another inode and an explicit position within that inode, which will change over time. + */ +export abstract class InoPosRef extends InoRef {} + +export class InoRefImpl extends InoRef { + constructor(public readonly target: string) { + super(ctorPrivateSymbol); + } +} + export class InoPosRefImpl extends InoPosRef { constructor( public readonly target: string, public readonly id: Id, ) { - super(); + super(ctorPrivateSymbol); } } diff --git a/client/lib/gumnut/wire.ts b/client/lib/gumnut/wire.ts index efd5671..f2c080f 100644 --- a/client/lib/gumnut/wire.ts +++ b/client/lib/gumnut/wire.ts @@ -1,6 +1,6 @@ import { stringToArray } from '../shared/array.ts'; import type { Id } from '../ymodel/internal/shared.ts'; -import { emptySymbol, InoPosRefImpl, type GumnutData } from './types.ts'; +import { emptySymbol, InoPosRefImpl, InoRefImpl, type GumnutData } from './types.ts'; /** * Encodes a single {@link GumnutData}. @@ -50,6 +50,9 @@ export function encodeGumnutData(lookupId: (inode: string, id: Id) => number, da // we need to deref the ID here - on the way up to the server const pos = lookupId(data.target, data.id); return { i: data.target, p: pos }; + } else if (data instanceof InoRefImpl) { + // other inode ref + return { i: data.target }; } else if (data === emptySymbol) { throw new Error(`can't encode emptySymbol`); } @@ -111,10 +114,15 @@ export function decodeGumnutData( return undefined; } throw new Error(`unhandled known: ${raw.x}`); - } else if ('i' in raw && 'p' in raw) { - // we got a real-space pos, convert to ID - const id = resolvePos(raw.i, raw.p); - return new InoPosRefImpl(raw.i, id); + } else if ('i' in raw) { + if ('p' in raw) { + // we got a real-space pos, convert to ID + const id = resolvePos(raw.i, raw.p); + return new InoPosRefImpl(raw.i, id); + } + + // otherwise this is just another inode ref + return new InoRefImpl(raw.i); } else if ('s' in raw) { return raw.s; } diff --git a/server/pkg/model/doc/doc.go b/server/pkg/model/doc/doc.go index 8087f40..690f932 100644 --- a/server/pkg/model/doc/doc.go +++ b/server/pkg/model/doc/doc.go @@ -187,9 +187,17 @@ func (d *docImpl) moveForward(handle, fromVersion int, gd *raw.GumnutData) (err } pos := int(gd.Data) // stores as uint64 but we change to int64 - user might have given -ve - node := d.byIno[gd.String] + + var node *node.Node + if gd.String != "" { + node = d.byIno[gd.String] + } if node == nil { - return // bad node + if pos < 0 { + // we targeted invalid/zero node? - maybe user targeted mapped away node + return ErrSetOp + } + return // bad or zero node } update, _ := node.TransformPos(handle, fromVersion, pos) diff --git a/server/pkg/model/node/map.go b/server/pkg/model/node/map.go new file mode 100644 index 0000000..32cf034 --- /dev/null +++ b/server/pkg/model/node/map.go @@ -0,0 +1,32 @@ +package node + +// Map calls the mapper for all found inodes here (in patch or in data). +// If any changes are found, returns new data. +func Map(src map[string]Patch, mapper func(inode string) (out string, allowSet bool)) (out map[string]Patch) { + out = make(map[string]Patch, len(src)) + + // remap keyed inodes + for id, patch := range src { + if id == "" { + continue // invalid + } + + update, allowSet := mapper(id) + if update == "" || !allowSet { + continue // deleted OR cannot be targeted + } + + id = update + + // TODO: can't compare slices :shrug: - just replace always + patch.Set = patch.Set.MapInode(func(inode string) (out string) { + out, _ = mapper(inode) + return out + }) + + // TODO: we might clobber other items _if_ mapper returns something we have + out[id] = patch + } + + return out +} diff --git a/server/pkg/model/raw/gumnut.go b/server/pkg/model/raw/gumnut.go index 29bcaa9..af543c4 100644 --- a/server/pkg/model/raw/gumnut.go +++ b/server/pkg/model/raw/gumnut.go @@ -40,6 +40,14 @@ func (gd *GumnutData) JSRune() (r uint16, ok bool) { return r, true } +// MatchInode returns the inode string here, or blank (invalid/none). +func (gd *GumnutData) MatchInode() (s string) { + if gd.Type == DataTypeInode || gd.Type == DataTypePosRef { + return gd.String + } + return +} + // IsZero returns whether this is the explicitly uninitialized data. func (gd *GumnutData) IsZero() (is bool) { return gd.Type == DataTypeEmpty @@ -148,7 +156,7 @@ func (g *GumnutData) UnmarshalJSON(b []byte) (err error) { Known *string `json:"x"` BigInt string `json:"b"` // bigint value Inf *int `json:"inf"` - Inode *string `json:"i"` + Inode string `json:"i"` // must be non-zero Pos *int64 `json:"p"` } err = json.Unmarshal(b, &tmp) @@ -181,14 +189,14 @@ func (g *GumnutData) UnmarshalJSON(b []byte) (err error) { g.Data = math.Float64bits(math.Inf(int(*tmp.Inf))) } - case tmp.Pos != nil && tmp.Inode != nil: + case tmp.Pos != nil && tmp.Inode != "": g.Type = DataTypePosRef - g.String = *tmp.Inode + g.String = tmp.Inode g.Data = uint64(*tmp.Pos) - case tmp.Inode != nil: + case tmp.Inode != "": g.Type = DataTypeInode - g.String = *tmp.Inode + g.String = tmp.Inode default: return ErrEncoding diff --git a/server/pkg/model/raw/set.go b/server/pkg/model/raw/set.go index 93a78ae..b20c458 100644 --- a/server/pkg/model/raw/set.go +++ b/server/pkg/model/raw/set.go @@ -29,6 +29,51 @@ func (s SetOp) IterData() (i iter.Seq[*GumnutData]) { } } +// MapInode maps all inode references here. +// It returns the same (if no changes) or copied (if changes) SetOp. +func (s SetOp) MapInode(mapper func(inode string) (out string)) (out SetOp) { + var change bool + + // find IF there is a chance + for gd := range s.IterData() { + inode := gd.MatchInode() + if inode == "" { + continue + } + + update := mapper(inode) + if update != inode { + change = true + break + } + } + if !change { + return s + } + + // now clone + // we need to do this to avoid modifying arrays inline (multiple users) + out = make(SetOp, len(s)) + for i, x := range s { + out[i].Skip = x.Skip + out[i].Data = make([]GumnutData, len(x.Data)) + + for j, gd := range x.Data { + inode := gd.MatchInode() + if inode != "" { + update := mapper(inode) + if update != inode { + gd.String = update + } + } + + out[i].Data[j] = gd + } + } + + return out +} + // Length returns the +ve length here. // If this targets -ve ops, these are ignored. func (arr SetOp) Length() (length int) { diff --git a/server/pkg/safedoc/inode.go b/server/pkg/safedoc/inode.go index 7b829ce..4e1a4d7 100644 --- a/server/pkg/safedoc/inode.go +++ b/server/pkg/safedoc/inode.go @@ -2,64 +2,120 @@ package safedoc import ( "fmt" - "maps" - "slices" + "strconv" "strings" - - "github.com/gumnutdev/foiled/server/pkg/model/node" - "github.com/gumnutdev/foiled/server/pkg/model/raw" ) -func inodeHandleActive(handleID int) (s string) { - return fmt.Sprintf("handle-active/%x:m", handleID) -} +func (ds *docSession) inboundMapInode(clientID int, inode string) (out string, allowSet bool) { + if strings.HasPrefix(inode, "s:") { + // scratch space + return inode, true + } + + parsed, _ := ParseStructuredIno(inode) + switch parsed.Type { + case "c": + // client-local data -// mapInodeIn updates inodes requested to be changed by a session. -// It returns false if there are invalid inode targets here. -func (ds *docSession) mapInodeIn(update map[string]node.Patch) (ok bool) { - handleActive := inodeHandleActive(ds.handleID) + if parsed.Value >= 0 { + return inode, false // can't write to other client space + } - for id, patch := range update { - if id == "" { - return false - } else if strings.HasPrefix(id, "handle-active/") { - return false + prev, _ := ds.localToGlobalClient.Get(parsed.Value) + if prev == 0 { + ds.localToGlobalClient.Put(parsed.Value, clientID) + } else if prev != clientID { + return // invalid } - // rewrite "self" target to be the actual ID - for gd := range patch.Set.IterData() { - if gd.Type == raw.DataTypeInode && gd.String == "handle-active/self:m" { - gd.String = handleActive - } + parsed.Value = clientID + return parsed.String(), true + + case "n": + // node data + + if parsed.Value >= 0 { + return inode, true // normal write to root node } + + // ensure we alloc a new -ve => +ve + value, _ := ds.allocs.Get(parsed.Value) + if value == 0 { + value = int(ds.sd.lastAlloc.Add(1)) + ds.allocs.Put(parsed.Value, value) + } + parsed.Value = value + return parsed.String(), true } - return true + // disallow everything else - allowlist only! + return } -// mapInodeOut modifies what gets broadcast to this session. -// The passed map is already local and can be modified inline. -func (ds *docSession) mapInodeOut(update map[string]node.Patch) { - handleActive := inodeHandleActive(ds.handleID) - - for _, patch := range update { - // rewrite "self" target to be the actual ID - for gd := range patch.Set.IterData() { - // FIXME: we need a better way to say - we're modifying this, copy all the way down :\ - if gd.Type == raw.DataTypeInode && gd.String == handleActive { - gd.String = "handle-active/self:m" - } +func (ds *docSession) outboundMapInode(inode string) (out string, allowSet bool) { + parsed, _ := ParseStructuredIno(inode) + + switch parsed.Type { + case "c": + self, _ := ds.localToGlobalClient.GetFar(parsed.Value) + if self < 0 { + // match our own IDs + parsed.Value = self + return parsed.String(), true + } + + case "n": + value, _ := ds.allocs.GetFar(parsed.Value) + if value < 0 { + // match our own IDs + parsed.Value = value + return parsed.String(), true } } - keys := slices.Collect(maps.Keys(update)) + return inode, true +} - for _, id := range keys { - if id == handleActive { - // broadcast self ownership under unique ID - update["handle-active/self:m"] = update[id] - delete(update, id) - continue - } +type StructuredIno struct { + Type string + Value int + Suffix string +} + +func ParseStructuredIno(inode string) (ino StructuredIno, ok bool) { + parts := strings.Split(inode, ":") + if len(parts) < 2 { + return + } + + var out StructuredIno + out.Type = parts[0] + if len(out.Type) == 0 { + return + } + + val, _ := strconv.ParseInt(parts[1], 16, 64) + out.Value = int(val) + + if len(parts) > 2 { + out.Suffix = strings.Join(parts[2:], ":") + } + + if out.String() != inode { + return // must map 1:1 + } + return out, true +} + +func (pi StructuredIno) String() (out string) { + if strings.Contains(pi.Type, ":") { + return "" // TODO: basically - invalid, can't escape } + out = fmt.Sprintf("%s:%x", pi.Type, pi.Value) + + if pi.Suffix != "" { + out += ":" + pi.Suffix + } + + return out } diff --git a/server/pkg/safedoc/inode_test.go b/server/pkg/safedoc/inode_test.go new file mode 100644 index 0000000..e54f397 --- /dev/null +++ b/server/pkg/safedoc/inode_test.go @@ -0,0 +1,164 @@ +package safedoc + +import ( + "reflect" + "testing" +) + +func TestStructuredIno(t *testing.T) { + tests := []struct { + input string + expected StructuredIno + ok bool + }{ + {"c:1", StructuredIno{Type: "c", Value: 1}, true}, + {"c:-1", StructuredIno{Type: "c", Value: -1}, true}, + {"test:abc", StructuredIno{Type: "test", Value: 0xabc}, true}, + {"type:123:suffix", StructuredIno{Type: "type", Value: 0x123, Suffix: "suffix"}, true}, + {"type:123:suffix:with:colons", StructuredIno{Type: "type", Value: 0x123, Suffix: "suffix:with:colons"}, true}, + + // Invalid cases + {"", StructuredIno{}, false}, + {"no_colon", StructuredIno{}, false}, + {":1", StructuredIno{}, false}, // Empty type + {"c:invalid_hex", StructuredIno{}, false}, // ParseInt returns 0, String() returns "c:0", mismatch + {"c:1: ", StructuredIno{Type: "c", Value: 1, Suffix: " "}, true}, + {"c:01", StructuredIno{}, false}, // Not minimal hex + {"type::suffix", StructuredIno{}, false}, + {"type:1:", StructuredIno{}, false}, + {"type:1:a:b:c", StructuredIno{Type: "type", Value: 1, Suffix: "a:b:c"}, true}, + } + + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + got, ok := ParseStructuredIno(tc.input) + if ok != tc.ok { + t.Errorf("ParseStructuredIno(%q) ok = %v, want %v", tc.input, ok, tc.ok) + } + if ok && !reflect.DeepEqual(got, tc.expected) { + t.Errorf("ParseStructuredIno(%q) = %+v, want %+v", tc.input, got, tc.expected) + } + + if ok { + s := got.String() + if s != tc.input { + t.Errorf("StructuredIno.String() = %q, want %q", s, tc.input) + } + } + }) + } +} + +func TestDocSession_InboundMapInode(t *testing.T) { + ds := &docSession{ + sd: &safeDoc{}, + } + + t.Run("negative-c", func(t *testing.T) { + inode := "c:-1" + clientID := 123 + out, allow := ds.inboundMapInode(clientID, inode) + if !allow { + t.Errorf("expected allow=true") + } + if out != "c:7b" { + t.Errorf("expected out=c:7b, got %q", out) + } + + // Repeat same + out, allow = ds.inboundMapInode(clientID, inode) + if !allow || out != "c:7b" { + t.Errorf("expected same result on second call") + } + + // Different client for same negative ID should fail + out, allow = ds.inboundMapInode(456, inode) + if allow { + t.Errorf("expected allow=false for different client") + } + }) + + t.Run("positive-c", func(t *testing.T) { + inode := "c:1" + out, allow := ds.inboundMapInode(123, inode) + if allow { + t.Errorf("expected allow=false for positive-c inode (can't write to other client space)") + } + if out != inode { + t.Errorf("expected out=input, got %q", out) + } + }) + + t.Run("negative-n", func(t *testing.T) { + inode := "n:-1" + out, allow := ds.inboundMapInode(1, inode) + if !allow { + t.Errorf("expected allow=true") + } + // lastAlloc starts at 0, so first alloc is 1 + if out != "n:1" { + t.Errorf("expected out=n:1, got %q", out) + } + + // Same negative ID should return same allocated ID + out, allow = ds.inboundMapInode(1, inode) + if out != "n:1" { + t.Errorf("expected out=n:1 again, got %q", out) + } + + // Different negative ID should return next allocated ID + out, allow = ds.inboundMapInode(1, "n:-2") + if out != "n:2" { + t.Errorf("expected out=n:2, got %q", out) + } + }) + + t.Run("other-type", func(t *testing.T) { + _, allow := ds.inboundMapInode(1, "whateverr") + if allow { + t.Errorf("expected allow=false for other types") + } + }) +} + +func TestDocSession_OutboundMapInode(t *testing.T) { + ds := &docSession{} + + // Setup a mapping: negative -1 -> global 123 + ds.localToGlobalClient.Put(-1, 123) + // Setup an alloc: negative -1 -> global 456 + ds.allocs.Put(-1, 456) + + t.Run("map-back-c", func(t *testing.T) { + inode := "c:7b" // 7b is 123 + out, allow := ds.outboundMapInode(inode) + if !allow { + t.Errorf("expected allow=true") + } + if out != "c:-1" { + t.Errorf("expected out=c:-1, got %q", out) + } + }) + + t.Run("map-back-n", func(t *testing.T) { + inode := "n:1c8" // 1c8 is 456 + out, allow := ds.outboundMapInode(inode) + if !allow { + t.Errorf("expected allow=true") + } + if out != "n:-1" { + t.Errorf("expected out=n:-1, got %q", out) + } + }) + + t.Run("unmapped", func(t *testing.T) { + inode := "c:fff" + out, allow := ds.outboundMapInode(inode) + if !allow { + t.Errorf("expected allow=true") + } + if out != inode { + t.Errorf("expected out=input, got %q", out) + } + }) +} diff --git a/server/pkg/safedoc/safedoc.go b/server/pkg/safedoc/safedoc.go index 98fd37d..31f0d7e 100644 --- a/server/pkg/safedoc/safedoc.go +++ b/server/pkg/safedoc/safedoc.go @@ -2,14 +2,15 @@ package safedoc import ( "context" - "encoding/json" "sync" + "sync/atomic" "time" "github.com/gumnutdev/foiled/server/pkg/alloc" "github.com/gumnutdev/foiled/server/pkg/model/doc" "github.com/gumnutdev/foiled/server/pkg/model/node" "github.com/gumnutdev/foiled/server/pkg/model/raw" + "github.com/samthor/thorgo/bimap" "github.com/samthor/thorgo/queue" ) @@ -21,9 +22,10 @@ type opResult struct { } type safeDoc struct { - lock sync.Mutex - doc doc.Doc - q queue.Queue[opResult] + lock sync.Mutex + doc doc.Doc + q queue.Queue[opResult] + lastAlloc atomic.Int64 } // serverApply must be called under lock. @@ -60,7 +62,6 @@ func (sd *safeDoc) Join(ctx context.Context) (ds Session) { first: &OutPacket{Version: data.TargetVersion, Update: data.Update}, l: sd.q.Join(ctx), } - impl.join() context.AfterFunc(ctx, func() { sd.lock.Lock() @@ -72,64 +73,59 @@ func (sd *safeDoc) Join(ctx context.Context) (ds Session) { } type docSession struct { - sd *safeDoc - - handleID int // unique generated here - first *OutPacket - l queue.Listener[opResult] - - lastSeq int -} - -// join is called when the context has joined this valid doc. -func (ds *docSession) join() { - handleActive := inodeHandleActive(ds.handleID) - - ds.sd.serverApply(map[string]node.Patch{ - handleActive: { - Set: raw.SetOp{ - {Data: []raw.GumnutData{ - {Type: raw.DataTypeString, String: "active"}, - raw.GumnutDataTrue, - }}, - }, - }, - }) + sd *safeDoc // target doc + handleID int // unique here for doc access + first *OutPacket // if non-nil, pending data to broadcast + l queue.Listener[opResult] // listener for future data + lastSeq int // next op must have lastSeq+1 + + localToGlobalClient bimap.Map[int, int] // user-provided -ve client ID -> actual global ID + allocs bimap.Map[int, int] } // part is called when the context passed in Join is done. func (ds *docSession) part() { ds.sd.doc.ClearHandle(ds.handleID) - handleActive := inodeHandleActive(ds.handleID) + // we need to remove all client data _for us_ curr := ds.sd.doc.Read() - length := curr.Update[handleActive].Set.Length() - - if length == 0 { - return + for inode := range curr.Update { + parsed, _ := ParseStructuredIno(inode) + switch parsed.Type { + case "c": + self, _ := ds.localToGlobalClient.GetFar(parsed.Value) + if self < 0 { + // we found one; remove it + ds.sd.serverApply(map[string]node.Patch{ + inode: { + Run: raw.OpRun{ + {Range: []int{0, curr.Update[inode].Set.Length()}}, + }, + }, + }) + } + } } - - ds.sd.serverApply(map[string]node.Patch{ - handleActive: { - Run: raw.OpRun{ - {Range: []int{0, length}}, - }, - }, - }) } func (ds *docSession) Handle() (handle int) { return ds.handleID } -func (ds *docSession) Perform(in InPacket, clientID int) (err error) { +func (ds *docSession) Perform(clientID int, in InPacket) (err error) { if in.Seq != ds.lastSeq+1 { return ErrSeq } - if !ds.mapInodeIn(in.Update) { + // map user inode references on the way in + mapped := node.Map(in.Update, func(inode string) (out string, allowSet bool) { + return ds.inboundMapInode(clientID, inode) + }) + if len(mapped) != len(in.Update) { + // low-tech solution to bad data; if we removed an item then error out return ErrPerform } + in.Update = mapped ds.sd.lock.Lock() defer ds.sd.lock.Unlock() @@ -199,30 +195,15 @@ retry: default: // this is actions performed by someone else - update := copyUpdate(next.dw.Update) - ds.mapInodeOut(update) - if len(update) == 0 { - goto retry // we got filtered out - } - p.Version = next.dw.TargetVersion - p.Update = update + p.Update = node.Map(next.dw.Update, ds.outboundMapInode) clientID = next.clientID + if len(p.Update) == 0 { + goto retry // we got filtered out + } + // TODO: we could merge Patch that are not acks here (merge => append) return clientID, p, true } } - -func copyUpdate(update map[string]node.Patch) (out map[string]node.Patch) { - // FIXME: gross, but since we might modify this inline, we need to copy the whole thing :| - b, err := json.Marshal(update) - if err != nil { - panic(err) - } - err = json.Unmarshal(b, &out) - if err != nil { - panic(err) - } - return out -} diff --git a/server/pkg/safedoc/safedoc_test.go b/server/pkg/safedoc/safedoc_test.go new file mode 100644 index 0000000..984c958 --- /dev/null +++ b/server/pkg/safedoc/safedoc_test.go @@ -0,0 +1,132 @@ +package safedoc + +import ( + "context" + "testing" + + "github.com/gumnutdev/foiled/server/pkg/model/node" + "github.com/gumnutdev/foiled/server/pkg/model/raw" +) + +func TestIntegration(t *testing.T) { + sd := New() + ctx1, cancel1 := context.WithCancel(context.Background()) + defer cancel1() + ctx2, cancel2 := context.WithCancel(context.Background()) + defer cancel2() + + ds1 := sd.Join(ctx1) + ds2 := sd.Join(ctx2) + + // Initial Join should give version 0 + _, p1, ok := ds1.Next() + if !ok || p1.Version != 0 { + t.Errorf("ds1 expected version 0, got v=%d, ok=%v", p1.Version, ok) + } + _, p2, ok := ds2.Next() + if !ok || p2.Version != 0 { + t.Errorf("ds2 expected version 0, got v=%d, ok=%v", p2.Version, ok) + } + + // ds1 performs an op with a client-local inode "c:-1" + // and a connection-local inode "n:-1" + err := ds1.Perform(100, InPacket{ + TargetVersion: 0, + Seq: 1, + Update: map[string]node.Patch{ + "c:-1": { + Set: raw.SetOp{{Data: raw.MapString("client data")}}, + }, + "n:-1": { + Set: raw.SetOp{{Data: raw.MapString("new node")}}, + }, + }, + }) + if err != nil { + t.Fatalf("ds1 Perform failed: %v", err) + } + + // ds1 should get an ack + client, ack, ok := ds1.Next() + if !ok || client != 0 || ack.Seq != 1 || ack.Version != 1 { + t.Fatalf("ds1 expected ack, got client=%d, seq=%d, v=%d", client, ack.Seq, ack.Version) + } + // ACKs do not contain the Update payload; their purpose is just to ack seq/version. + if len(ack.Update) != 0 { + t.Errorf("ds1 expected ack to have no updates, got %d", len(ack.Update)) + } + + // ds2 should get the update + client, update, ok := ds2.Next() + if !ok || client != 100 || update.Version != 1 { + t.Fatalf("ds2 expected update from 100, got client=%d, v=%d", client, update.Version) + } + // For ds2, c:-1 from ds1 (global clientID 100) should be mapped to "c:64" (100 in hex) + if _, ok := update.Update["c:64"]; !ok { + t.Errorf("ds2 expected update for 'c:64'") + } + // For ds2, n:-1 from ds1 should be a global ID (n:1) + if _, ok := update.Update["n:1"]; !ok { + t.Errorf("ds2 expected update for 'n:1'") + } + + // Now ds2 performs an op on the normal node "n:1" + // (Writing to another client's "c:" space is not allowed and would be filtered) + err = ds2.Perform(200, InPacket{ + TargetVersion: 1, + Seq: 1, + Update: map[string]node.Patch{ + "n:1": { + Set: raw.SetOp{{Data: raw.MapString("node data changed")}}, + }, + }, + }) + if err != nil { + t.Fatalf("ds2 Perform failed: %v", err) + } + + // ds2 attempts to write to ds1's space, should fail with ErrPerform + err = ds2.Perform(200, InPacket{ + TargetVersion: 2, + Seq: 2, + Update: map[string]node.Patch{ + "c:64": { // 100 in hex, which is ds1's clientID + Set: raw.SetOp{{Data: raw.MapString("forbidden write")}}, + }, + }, + }) + if err != ErrPerform { + t.Errorf("expected ErrPerform, got %v", err) + } + + // ds2 attempts to write with empty inode string, should fail with ErrPerform + err = ds2.Perform(200, InPacket{ + TargetVersion: 2, + Seq: 2, // Previous successful op was Seq 1. Previous failed op was also Seq 2. + Update: map[string]node.Patch{ + "": { + Set: raw.SetOp{{Data: raw.MapString("invalid inode")}}, + }, + }, + }) + if err != ErrPerform { + t.Errorf("expected ErrPerform for empty inode, got %v", err) + } + + // ds2 gets ack + _, ack2, _ := ds2.Next() + if ack2.Version != 2 { + t.Errorf("ds2 expected version 2, got %d", ack2.Version) + } + + // ds1 gets update + client, update1, ok := ds1.Next() + if !ok || client != 200 || update1.Version != 2 { + t.Fatalf("ds1 expected update from 200, got client=%d, v=%d", client, update1.Version) + } + + // For ds1, "n:1" should map back to its local "n:-1" + if _, ok := update1.Update["n:-1"]; !ok { + t.Errorf("ds1 expected update for 'n:-1'") + } +} diff --git a/server/pkg/safedoc/types.go b/server/pkg/safedoc/types.go index 0cb9fc6..486c555 100644 --- a/server/pkg/safedoc/types.go +++ b/server/pkg/safedoc/types.go @@ -5,11 +5,15 @@ import ( ) type Doc interface { + // Join allows a connection to join this Doc, read ops and perform ops. Join(ctx context.Context) (ds Session) } type Session interface { - Next() (clientID int, p OutPacket, ok bool) - Perform(in InPacket, clientID int) (err error) - Handle() (handle int) + // Next returns the next outbound packet and its origin (client). + // If ok is false, this context has died. + Next() (client int, p OutPacket, ok bool) + + // Perform performs work as this unique client. + Perform(client int, in InPacket) (err error) } diff --git a/server/pkg/server/server.go b/server/pkg/server/server.go index e8f31e4..a4ad6f2 100644 --- a/server/pkg/server/server.go +++ b/server/pkg/server/server.go @@ -3,7 +3,6 @@ package server import ( "context" "encoding/json" - "log" "sync" "github.com/gumnutdev/foiled/server/pkg/host" @@ -45,12 +44,12 @@ func (s *Server) Handle(arg host.DocArg, tr transport.TypeTransport[host.Session doc := s.getDoc(arg.Doc) ds := doc.Join(tr.Context()) - handle := ds.Handle() + // handle := ds.Handle() - log.Printf("new handle=%d for doc=%+v", handle, arg) - defer func() { - log.Printf("closing handle=%d for doc=%+v err=%v", handle, arg, err) - }() + // log.Printf("new handle=%d for doc=%+v", handle, arg) + // defer func() { + // log.Printf("closing handle=%d for doc=%+v err=%v", handle, arg, err) + // }() eg, groupCtx := errgroup.WithContext(tr.Context()) @@ -66,7 +65,7 @@ func (s *Server) Handle(arg host.DocArg, tr transport.TypeTransport[host.Session return err } - err = ds.Perform(in, sp.Client) + err = ds.Perform(sp.Client, in) if err == safedoc.ErrBarrier { // do nothing } else if err != nil { diff --git a/tests/integration/basic.test.ts b/tests/integration/basic.test.ts index 390a1a3..c36e992 100644 --- a/tests/integration/basic.test.ts +++ b/tests/integration/basic.test.ts @@ -9,21 +9,21 @@ test('basic', async (t) => { const doc2 = await connectToGumnutLow(t.signal, { docId: doc1.docId }); doc1.floor.perform(function* (t) { - const api = t.byIno('abc'); + const api = t.byIno('s:abc'); api.dataUpdate(0, 0, ...Array(7).fill(null)); }); await checkCond(() => { - assert.strictEqual(doc2.floor.byIno('abc').length(), 7); + assert.strictEqual(doc2.floor.byIno('s:abc').length(), 7); }); doc2.floor.perform(function* (t) { - const api = t.byIno('abc'); + const api = t.byIno('s:abc'); api.set(1, 'interned string', -100n, -Infinity, undefined); }); await checkCond(() => { - const ino = doc1.floor.byIno('abc'); + const ino = doc1.floor.byIno('s:abc'); const check = ino.data(); assert.deepStrictEqual(check, [ @@ -43,12 +43,12 @@ test('string', async (t) => { const doc2 = await connectToGumnutLow(t.signal, { docId: doc1.docId }); doc1.floor.perform(function* (t) { - const api = t.byIno('abc'); + const api = t.byIno('s:abc'); api.stringUpdate(0, 0, 'hello'); }); await checkCond(() => { - assert.strictEqual(doc2.floor.byIno('abc').string(), 'hello'); + assert.strictEqual(doc2.floor.byIno('s:abc').string(), 'hello'); }); }); @@ -57,12 +57,12 @@ test('naïve ref over time', async (t) => { // create initial state: 'foo' with 50 items, 'bar' containing single pointing back 10 doc1.floor.perform(function* (t) { - const apiFoo = t.byIno('foo'); + const apiFoo = t.byIno('s:foo'); const arr = new Array(50).fill(7); apiFoo.dataUpdate(0, 0, ...arr); - const apiBar = t.byIno('bar'); + const apiBar = t.byIno('s:bar'); apiBar.dataUpdate(0, 0, apiFoo.refAt(40)); const wasRef = apiBar.at(0); @@ -73,28 +73,28 @@ test('naïve ref over time', async (t) => { // check resolved pos on new conn const doc2 = await connectToGumnutLow(t.signal, { docId: doc1.docId }); await checkCond(() => { - assert.strictEqual(doc2.floor.byIno('foo').length(), 50); + assert.strictEqual(doc2.floor.byIno('s:foo').length(), 50); - const ref = doc2.floor.byIno('bar').at(0); + const ref = doc2.floor.byIno('s:bar').at(0); assert.ok(ref instanceof InoPosRef); - assert.strictEqual(ref.target, 'foo'); + assert.strictEqual(ref.target, 's:foo'); assert.strictEqual(doc2.floor.resolveRef(ref), 40); }); // remove some data, in theory shifting ref back too doc2.floor.perform(function* (t) { - const apiFoo = t.byIno('foo'); + const apiFoo = t.byIno('s:foo'); apiFoo.dataUpdate(0, 25); }); // check resolved pos const doc3 = await connectToGumnutLow(t.signal, { docId: doc1.docId }); await checkCond(() => { - assert.strictEqual(doc3.floor.byIno('foo').length(), 25); + assert.strictEqual(doc3.floor.byIno('s:foo').length(), 25); - const ref = doc3.floor.byIno('bar').at(0); + const ref = doc3.floor.byIno('s:bar').at(0); assert.ok(ref instanceof InoPosRef); - assert.strictEqual(ref.target, 'foo'); + assert.strictEqual(ref.target, 's:foo'); assert.strictEqual(doc3.floor.resolveRef(ref), 15); }); }); diff --git a/tests/integration/data.test.ts b/tests/integration/data.test.ts index 16f7043..64327e7 100644 --- a/tests/integration/data.test.ts +++ b/tests/integration/data.test.ts @@ -1,7 +1,7 @@ import { test } from 'node:test'; import * as assert from 'node:assert'; import { connectToGumnutLow } from './util.ts'; -import { buildDataApi } from '../../client/lib/gumnut/data.ts'; +import { buildDataApi, DataMap } from '../../client/lib/gumnut/data.ts'; import { checkCond } from 'thorish'; test('data', async (t) => { @@ -22,3 +22,23 @@ test('data', async (t) => { assert.strictEqual(root.get('def'), root); }); }); + +test('sub', async (t) => { + const doc1 = await connectToGumnutLow(t.signal); + const doc2 = await connectToGumnutLow(t.signal, { docId: doc1.docId }); + + const data1 = buildDataApi(doc1.floor); + const data2 = buildDataApi(doc2.floor); + + const data1sub = data1.newMap(); + data1.root().set('abc', data1sub); + data1sub.set('def', 123); + + await checkCond(() => { + const root = data2.root(); + + const sub = root.get('abc'); + assert.ok(sub instanceof DataMap); + assert.strictEqual(sub.get('def'), 123); + }); +});