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.

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

move fails with "missing value" if from is "" #192

Closed
lucastheisen opened this issue Jan 3, 2024 · 4 comments
Closed

move fails with "missing value" if from is "" #192

lucastheisen opened this issue Jan 3, 2024 · 4 comments

Comments

@lucastheisen
Copy link

According to the JSON Pointer RFC 6901:

The following JSON strings evaluate to the accompanying values:

""           // the whole document

But using that as a from results in a missing value error.

Adding this test will demonstrate the bug:

func TestMoveFromRoot(t *testing.T) {
	expected := `{"baz":{"foo":"bar"}}`
	res, err := applyPatch(`{"foo":"bar"}`, `[{"op":"move","from":"","path":"/baz"}]`)
	if err != nil {
		t.Errorf("unexpected error: %+v", err)
	} else if res != expected {
		t.Errorf("expected:\n%s\ngot:\n%s", expected, res)
	}
}
@lucastheisen
Copy link
Author

Well, i just saw this in the RFC for JSON Patch:

The "from" location MUST NOT be a proper prefix of the "path"
location; i.e., a location cannot be moved into one of its children.

So my use case is invalid (as is the test per-se)... But the lack of support for "" JSON Pointer may still be something you want to address. You can close this issue out if you would like, or modify it to deal with the JSON Pointer support.

@evanphx
Copy link
Owner

evanphx commented Jan 11, 2024

@lucastheisen Gotcha gotcha. Trying some other json patch implementations, they largely fail to handle when from is "", so I think we're in a bit of uncharted waters here.

evanphx added a commit that referenced this issue Jan 11, 2024
@evanphx
Copy link
Owner

evanphx commented Jan 11, 2024

Could you have a look at #193 and give me your thoughts? Thanks!

evanphx added a commit that referenced this issue Jan 12, 2024
Handle from="" more properly. Fixes #192
@lucastheisen
Copy link
Author

@evanphx , that merge looks great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants