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

Can find element with #32

Open
Truemedia opened this issue May 27, 2016 · 4 comments
Open

Can find element with #32

Truemedia opened this issue May 27, 2016 · 4 comments

Comments

@Truemedia
Copy link

Hi,
Sorry I left an issue before without a response due to dealing with other projects, but after playing around with this library more I can verify the problem.

I am trying to move one piece of JSON to another location in the same doc. While this has been working for the most part one particular operation seems to fail.

See the following:

[ { op: 'move',
    from: '/14',
    path: '/250/nested_classes/0/nested_classes/' } ]

I am performing this on a variable called doc.

If I do the following:
console.log(doc[250]['nested_classes'][0]['nested_classes']);

I get square brackets because that is an array, which is fine that's what I want and I just want to insert the moved data into that array so that it becomes the first item.

The data from this:
console.log(doc[14]);

I get:

{ class_name: 'CreditCard',
  sub_class: 'PaymentMethod',
  properties: {},
  nested_classes: [] }

I have used this same logic for an existing node in my doc and it works completely fine, but I have no idea why it does not work in this scenario when I can see both paths exist.

Is this some kind of hard to find bug in the library, or some issue with how my paths have been typed?

Really appreciate any help with this, thanks.

@Truemedia
Copy link
Author

Update,
It turns out that doing a copy and remove using the same logic works for achieving the desired outcome. While this solves my issue short term I think there is a bug in the replace operation.

Maybe it is worth treating the internal functionality to trigger a copy operation followed by a remove operation inside the replace function of the jsonpatch library rather than the current code:

JSONPointer.prototype.replace = function (doc, value, mutate) {
    // Special case for a pointer to the root
    if (0 === this.length) {
      return value;
    }
    return this._action(doc, function (node, lastSegment) {
        if (!Object.hasOwnProperty.call(node,lastSegment)) {
          throw new PatchApplyError('Replace operation must point to an existing value!');
        }
        if (isArray(node)) {
          node.splice(lastSegment, 1, value);
        } else {
          node[lastSegment] = value;
        }
      return node;
    }, mutate);
  };

@almost
Copy link
Member

almost commented May 28, 2016

It would be helpful to have test case I can just run. If you could include that I'll try it out and try to figure out the issue

@Truemedia
Copy link
Author

Truemedia commented May 28, 2016

Thanks I appreciate it. The repo I am using is https://github.com/Truemedia/slush-blueprints.

I need to finish a few commits before you will be able to run the command where this error resides but I will update this issue when I have (should be next day or so).

The command I run when this library is installed is:
slush blueprints:install

If you look for anything from this section of code:

/* Convert a KVP match to a JSONpatch */
    convert_match_to_patch: function(thing_match, parent_match)
    {
        var thing_path = thing_match.pointer,
            thing_path = thing_path.replace('/class_name', '');

        var parent_path = parent_match.pointer,
            parent_path = parent_path.replace('class_name', 'nested_classes/');

        // TODO: Use replace instead of copy and remove once JSONpatch library is fixed
        // var json_patch = [{ "op": "move", "from": thing_path, "path": parent_path }];

        schema.organized_things = jsonpatch.apply_patch(schema.organized_things, [{ "op": "copy", "from": thing_path, "path": parent_path }]);
        var json_patch = [{ "op": "remove", "path": thing_path }];

        return json_patch;
    },

This is where the error is triggered from if you uncomment the replace line, and comment out the copy and remove operations.

@Truemedia
Copy link
Author

Ok I have now updated the repo I mentioned. You will need to clone the master branch and use npm link for the local copy. If you have any issues let me know.

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