From 7a457318f4f96b69c328feb9476bc137500155eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Veres?= Date: Thu, 4 Apr 2024 20:48:15 +0200 Subject: [PATCH 1/2] fix: mutable remote refs --- packages/ogre/src/repository.test.ts | 53 +++++++++++++++++++++++++--- packages/ogre/src/repository.ts | 38 +++++++++++--------- packages/ogre/src/utils.ts | 21 +++++++++++ 3 files changed, 91 insertions(+), 21 deletions(-) diff --git a/packages/ogre/src/repository.test.ts b/packages/ogre/src/repository.test.ts index c694725..1bd501c 100644 --- a/packages/ogre/src/repository.test.ts +++ b/packages/ogre/src/repository.test.ts @@ -123,8 +123,10 @@ test("restore", async (t) => { "incorrect # of changelog entries", ); }); +}); - test("restoring from history", async (t) => { +test("history", async (t) => { + t.test("successful restore", async (t) => { const [repo, obj] = await getBaseline(); updateHeaderData(obj); await repo.commit("header data", testAuthor); @@ -140,9 +142,51 @@ test("restore", async (t) => { t.matchOnly(obj, obj2, "restored object does not equal last version."); }); -}); -test("history", async (t) => { + t.test("remoteRefs doesn't change on commit", async (t) => { + const repo = new Repository({}, {}); + updateHeaderData(repo.data); + await repo.commit("header data", testAuthor); + + const history = repo.getHistory(); + + const r2 = new Repository({}, { history }); + const remoteBeforeChange = r2.remote(); + + r2.data.name = "a different name"; + await r2.commit("changed name", testAuthor); + + const remoteAfterChange = r2.remote(); + + const history2 = r2.getHistory(); + + t.not( + remoteBeforeChange, + history.refs, + "input history refs and remote before change should not be the same object", + ); + t.matchOnlyStrict( + remoteBeforeChange, + history.refs, + "remote before change is not the same as history input", + ); + t.matchOnlyStrict( + remoteAfterChange, + remoteBeforeChange, + "remote before and after change are not the same", + ); + t.notMatchOnlyStrict( + history2.refs, + remoteAfterChange, + "history refs must not be the same as static remotes", + ); + t.notMatchOnlyStrict( + history.refs, + history2.refs, + "histories must not match anymore", + ); + }); + t.test("history contains HEAD ref", async (t) => { const [repo] = await getBaseline(); @@ -161,10 +205,9 @@ test("history", async (t) => { () => new Repository(co, { history: { - original: co, refs: new Map(), commits: [], - } as History, + }, }), { message: "unreachable: 'HEAD' is not present", diff --git a/packages/ogre/src/repository.ts b/packages/ogre/src/repository.ts index 69270f7..1de5cd1 100644 --- a/packages/ogre/src/repository.ts +++ b/packages/ogre/src/repository.ts @@ -21,8 +21,10 @@ import { createHeadRefValue, getLastRefPathElement, headValueRefPrefix, + immutableRefCopy, localHeadPathPrefix, mapPath, + mutableMapCopy, REFS_HEAD_KEY, REFS_MAIN_KEY, refsAtCommit, @@ -92,9 +94,10 @@ export interface RepositoryObject { reset(mode?: "soft" | "hard", shaish?: string): void; /** - * Returns the remote references from the initialization of the repository + * Returns the remote references from the initialization of the repository. + * The returned map is a readonly of remote. */ - remote(): Map | undefined; + remote(): ReadonlyMap> | undefined; } /** @@ -105,22 +108,23 @@ export class Repository { constructor(obj: Partial, options: RepositoryOptions) { // FIXME: move this to refs/remote as git would do? - this.remoteRefs = options.history?.refs; + + this.remoteRefs = immutableRefCopy(options.history?.refs); this.original = deepClone(obj); // store js ref, so obj can still be modified without going through repo.data this.data = obj as T; this.observer = observe(obj as T); - this.refs = - options.history?.refs ?? - new Map([ - [ - REFS_HEAD_KEY, - { - name: REFS_HEAD_KEY, - value: `ref: ${REFS_MAIN_KEY}`, - }, - ], - ]); + this.refs = options.history?.refs + ? mutableMapCopy(options.history?.refs) + : new Map([ + [ + REFS_HEAD_KEY, + { + name: REFS_HEAD_KEY, + value: `ref: ${REFS_MAIN_KEY}`, + }, + ], + ]); this.commits = options.history?.commits ?? []; @@ -138,14 +142,16 @@ export class Repository data: T; // stores the remote state upon initialization - private readonly remoteRefs: Map | undefined; + private readonly remoteRefs: + | ReadonlyMap> + | undefined; private observer: Observer; private readonly refs: Map; private readonly commits: Commit[]; - remote(): Map | undefined { + remote(): ReadonlyMap> | undefined { return this.remoteRefs; } diff --git a/packages/ogre/src/utils.ts b/packages/ogre/src/utils.ts index 799a555..6be4067 100644 --- a/packages/ogre/src/utils.ts +++ b/packages/ogre/src/utils.ts @@ -217,3 +217,24 @@ export const printChange = (chg: Operation) => { */ export const getLastRefPathElement = (thePath: string) => thePath.substring(thePath.lastIndexOf("/") + 1); + +export const immutableRefCopy = ( + map: Map | undefined, +) => { + if (!map) { + return undefined; + } + const m = new Map>(); + for (const [key, value] of map) { + m.set(key, { ...value }); + } + return m as ReadonlyMap>; +}; + +export const mutableMapCopy = (map: Map) => { + const m = new Map(); + for (const [key, value] of map) { + m.set(key, { ...value }); + } + return m; +}; From 5f575db36ff15d7007c85a8218a28d302ed2c428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Veres?= Date: Thu, 4 Apr 2024 20:50:43 +0200 Subject: [PATCH 2/2] refactor: rename generic func --- packages/ogre/src/repository.ts | 4 ++-- packages/ogre/src/utils.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ogre/src/repository.ts b/packages/ogre/src/repository.ts index 1de5cd1..30c059e 100644 --- a/packages/ogre/src/repository.ts +++ b/packages/ogre/src/repository.ts @@ -21,7 +21,7 @@ import { createHeadRefValue, getLastRefPathElement, headValueRefPrefix, - immutableRefCopy, + immutableMapCopy, localHeadPathPrefix, mapPath, mutableMapCopy, @@ -109,7 +109,7 @@ export class Repository constructor(obj: Partial, options: RepositoryOptions) { // FIXME: move this to refs/remote as git would do? - this.remoteRefs = immutableRefCopy(options.history?.refs); + this.remoteRefs = immutableMapCopy(options.history?.refs); this.original = deepClone(obj); // store js ref, so obj can still be modified without going through repo.data this.data = obj as T; diff --git a/packages/ogre/src/utils.ts b/packages/ogre/src/utils.ts index 6be4067..b33714f 100644 --- a/packages/ogre/src/utils.ts +++ b/packages/ogre/src/utils.ts @@ -218,7 +218,7 @@ export const printChange = (chg: Operation) => { export const getLastRefPathElement = (thePath: string) => thePath.substring(thePath.lastIndexOf("/") + 1); -export const immutableRefCopy = ( +export const immutableMapCopy = ( map: Map | undefined, ) => { if (!map) {