Merge pull request #171 from dotindustries/fix/mutable_remote_refs

fix: mutable remote refs

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<ComplexObject>({}, {});
+    updateHeaderData(repo.data);
+    await repo.commit("header data", testAuthor);
+
+    const history = repo.getHistory();
+
+    const r2 = new Repository<ComplexObject>({}, { 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<string, Reference>(),
             commits: [],
-          } as History,
+          },
         }),
       {
         message: "unreachable: 'HEAD' is not present",

packages/ogre/src/repository.ts

@@ -21,8 +21,10 @@ import {
   createHeadRefValue,
   getLastRefPathElement,
   headValueRefPrefix,
+  immutableMapCopy,
   localHeadPathPrefix,
   mapPath,
+  mutableMapCopy,
   REFS_HEAD_KEY,
   REFS_MAIN_KEY,
   refsAtCommit,
@@ -92,9 +94,10 @@ export interface RepositoryObject<T extends { [k: string]: any }> {
   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<string, Reference> | undefined;
+  remote(): ReadonlyMap<string, Readonly<Reference>> | undefined;
 }
 
 /**
@@ -105,22 +108,23 @@ export class Repository<T extends { [k: PropertyKey]: any }>
 {
   constructor(obj: Partial<T>, options: RepositoryOptions<T>) {
     // FIXME: move this to refs/remote as git would do?
-    this.remoteRefs = 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;
     this.observer = observe(obj as T);
-    this.refs =
-      options.history?.refs ??
-      new Map<string, Reference>([
-        [
-          REFS_HEAD_KEY,
-          {
-            name: REFS_HEAD_KEY,
-            value: `ref: ${REFS_MAIN_KEY}`,
-          },
-        ],
-      ]);
+    this.refs = options.history?.refs
+      ? mutableMapCopy(options.history?.refs)
+      : new Map<string, Reference>([
+          [
+            REFS_HEAD_KEY,
+            {
+              name: REFS_HEAD_KEY,
+              value: `ref: ${REFS_MAIN_KEY}`,
+            },
+          ],
+        ]);
 
     this.commits = options.history?.commits ?? [];
 
@@ -138,14 +142,16 @@ export class Repository<T extends { [k: PropertyKey]: any }>
   data: T;
 
   // stores the remote state upon initialization
-  private readonly remoteRefs: Map<string, Reference> | undefined;
+  private readonly remoteRefs:
+    | ReadonlyMap<string, Readonly<Reference>>
+    | undefined;
 
   private observer: Observer<T>;
 
   private readonly refs: Map<string, Reference>;
   private readonly commits: Commit[];
 
-  remote(): Map<string, Reference> | undefined {
+  remote(): ReadonlyMap<string, Readonly<Reference>> | undefined {
     return this.remoteRefs;
   }
 

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 immutableMapCopy = <T extends object>(
+  map: Map<string, T> | undefined,
+) => {
+  if (!map) {
+    return undefined;
+  }
+  const m = new Map<string, Readonly<T>>();
+  for (const [key, value] of map) {
+    m.set(key, { ...value });
+  }
+  return m as ReadonlyMap<string, Readonly<T>>;
+};
+
+export const mutableMapCopy = <T extends object>(map: Map<string, T>) => {
+  const m = new Map<string, T>();
+  for (const [key, value] of map) {
+    m.set(key, { ...value });
+  }
+  return m;
+};