fix: working undefined to obj patch (#157)

### TL;DR

Update History interface and RepositoryObject methods

### What changed?

- Updated History interface
- Updated RepositoryObject methods

### How to test?

No specific testing instructions provided

### Why make this change?

The changes were made to improve the handling of history and repository objects

---

packages/ogre/src/interfaces.ts

@@ -7,8 +7,7 @@ export interface Reference {
  value: string;
 }
 
-export interface History<T extends { [k: string]: any }> {
- original: T;
+export interface History {
  refs: Map<string, Reference>;
  commits: Commit[];
 }

packages/ogre/src/repository.test.ts

@@ -163,7 +163,7 @@ test("history", async (t) => {
  original: co,
  refs: new Map<string, Reference>(),
  commits: [],
- } as History<ComplexObject>,
+ } as History,
  }),
  {
  message: "unreachable: 'HEAD' is not present",
@@ -201,17 +201,29 @@ test("status", async (t) => {
  const [repo] = await getBaseline({ name: "base name" });
  repo.data.name = "changed name";
  const dirtyState = repo.status();
- t.equal(dirtyState.length, 1, "Status doesn't contain changes");
+ t.equal(
+ dirtyState.length,
+ 1,
+ `Status doesn't contain the expected # of changes: ${JSON.stringify(dirtyState)}`,
+ );
  });
- t.test("reading status doesn't clean observer", async (t) => {
+ t.test("reading status shouldn't clean observer", async (t) => {
  const [repo] = await getBaseline({ name: "base name" });
  repo.data.name = "changed name";
  const dirtyState = repo.status();
- t.equal(dirtyState.length, 1, "Status doesn't contain changes");
+ t.equal(
+ dirtyState.length,
+ 1,
+ `Status doesn't contain the expected # of changes: ${JSON.stringify(dirtyState)}`,
+ );
 
  const dirtyState2 = repo.status();
- t.equal(dirtyState2.length, 1, "Status doesn't contain changes");
- t.match(dirtyState2, dirtyState2, "different pending changes??");
+ t.equal(
+ dirtyState2.length,
+ 1,
+ `Status doesn't contain the expected # of changes: ${JSON.stringify(dirtyState)}`,
+ );
+ t.match(dirtyState2, dirtyState2, "why different pending changes??");
  });
  t.test("after commit no change", async (t) => {
  const [repo] = await getBaseline();
@@ -275,43 +287,27 @@ test("apply", async (t) => {
  t.match(dirtyState, patches, "It should have the right changes");
  });
 
- t.test(
- "patch for undefined props doesn't work as expected https://github.com/Starcounter-Jack/JSON-Patch/issues/280",
- async (t) => {
- const [repo] = await getBaseline();
- const cleanState = repo.status();
- t.match(cleanState, [], "Shouldn't have pending changes");
-
- const targetState: ComplexObject = {
- uuid: undefined,
- description: undefined,
- name: "a name",
- nested: [],
- };
- const patches = compare(repo.data, targetState);
- // this should record changes on the observer
- const err = repo.apply(patches);
- t.match(
- err,
- {
- name: "OPERATION_PATH_UNRESOLVABLE",
- operation: {
- op: "replace",
- path: "/name",
- value: "a name",
- },
- },
- "Failed to apply patch",
- );
- t.notMatch(
- repo.data,
- targetState,
- "The final state shoould not match up, because patch failed",
- );
- const dirtyState = repo.status();
- t.equal(dirtyState.length, 0, "Status doesn't contain changes");
- },
- );
+ t.test("patch for undefined props with workaround", async (t) => {
+ // solution for workaround from: https://github.com/Starcounter-Jack/JSON-Patch/issues/280
+ const [repo] = await getBaseline();
+ const cleanState = repo.status();
+ t.match(cleanState, [], "Shouldn't have pending changes");
+
+ const targetState: ComplexObject = {
+ uuid: undefined,
+ description: undefined,
+ name: "a name",
+ nested: [],
+ };
+ const patches = compare(repo.data, targetState);
+ // this should record changes on the observer
+ const err = repo.apply(patches);
+ t.equal(err, undefined, "Failed to apply patch");
+
+ t.match(repo.data, targetState, "The final state should match up");
+ const dirtyState = repo.status();
+ t.equal(dirtyState.length, 1, "Status should contain 1 change");
+ });
 
  t.test("multiple patches", async (t) => {
  const [repo] = await getBaseline({ name: "base name" });

packages/ogre/src/repository.ts

@@ -24,13 +24,13 @@ export const REFS_HEAD_KEY = "HEAD";
 export const REFS_MAIN_KEY = `${headsRefPathPrefix}main`;
 
 export interface RepositoryOptions<T extends { [k: string]: any }> {
- history?: History<T>;
+ history?: History;
 }
 
 export interface RepositoryObject<T extends { [k: string]: any }> {
  data: T;
 
- getHistory(): History<T>;
+ getHistory(): History;
 
  /**
  * Returns the diff between the current HEAD and provided shaish expression
@@ -65,7 +65,7 @@ export interface RepositoryObject<T extends { [k: string]: any }> {
 
  createBranch(name: string): string;
 
- merge(source: string | RepositoryObject<T> | History<T>): string;
+ merge(source: string | RepositoryObject<T> | History): string;
 
  /**
  * Branch returns the current branch name
@@ -88,10 +88,11 @@ export interface RepositoryObject<T extends { [k: string]: any }> {
 export class Repository<T extends { [k: PropertyKey]: any }>
  implements RepositoryObject<T>
 {
- constructor(obj: T, options: RepositoryOptions<T>) {
+ constructor(obj: Partial<T>, options: RepositoryOptions<T>) {
  this.original = deepClone(obj);
- this.data = obj;
- this.observer = observe(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>([
@@ -136,9 +137,32 @@ export class Repository<T extends { [k: PropertyKey]: any }>
  }
 
  apply(patch: Operation[]): JsonPatchError | undefined {
- const err = validate(patch, this.data);
+ const p = deepClone(patch) as Operation[];
+ const err = validate(p, this.data);
  if (err) {
- return err;
+ // credit goes to @NicBright
+ // https://github.com/Starcounter-Jack/JSON-Patch/issues/280#issuecomment-1980435509
+ if (err.name === "OPERATION_PATH_UNRESOLVABLE") {
+ if (err.operation.op === "replace") {
+ // can happen e.g. when states are like this:
+ // from.x = undefined
+ // to.x = 'something'
+ const op = p.find(
+ (o) => o.path === err.operation.path && o.op === err.operation.op,
+ );
+ if (!op) return err;
+ // try it once more with operation 'add' instead
+ op.op = "add";
+ return this.apply(p);
+ } else if (err.operation.op === "remove") {
+ // Can happen e.g. when states are like this:
+ // from.entity.reason = null;
+ // to.entity.reason = undefined;
+ // we don't do anything in this case because "to" is already in a good state!
+ }
+ } else {
+ return err;
+ }
  }
  applyPatch(this.data, patch);
  // const changed = patch.reduce(applyReducer, this.data);
@@ -347,9 +371,8 @@ export class Repository<T extends { [k: PropertyKey]: any }>
  return commitsList;
  }
 
- getHistory(): History<T> {
+ getHistory(): History {
  return {
- original: this.original,
  refs: new Map(this.refs),
  commits: this.collectCommits(),
  };
@@ -370,7 +393,7 @@ export class Repository<T extends { [k: PropertyKey]: any }>
 
  // endregion
 
- merge(source: string | RepositoryObject<T> | History<T>): string {
+ merge(source: string | RepositoryObject<T> | History): string {
  // inspiration
  // http://think-like-a-git.net
  // also check isomorphic-git