Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: working undefined to obj patch #157

Merged
merged 1 commit into from
Mar 23, 2024
Merged

fix: working undefined to obj patch #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/ogre/src/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}
82 changes: 39 additions & 43 deletions packages/ogre/src/repository.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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" });
Expand Down
45 changes: 34 additions & 11 deletions packages/ogre/src/repository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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>([
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
};
Expand All @@ -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
Expand Down
Loading