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

[Bug]: json.path.put fails if there are no results #4123

Closed
Pmofmalasia opened this issue Jun 11, 2023 · 9 comments · Fixed by #4280
Closed

[Bug]: json.path.put fails if there are no results #4123

Pmofmalasia opened this issue Jun 11, 2023 · 9 comments · Fixed by #4280
Labels

Comments

@Pmofmalasia
Copy link

Describe the Bug

When using json.path.put in 1.14.0-alpha1, if the path given has no results then it fails with the error: Error with function "json.path.put": null.

In prior releases, the function did not fail and moved on without putting anything in the json.

To Reproduce

Paste the following code into the chat:

[h:testArray = json.append("",json.set("","Test",1),json.set("","Test",3))]
[h:broadcast(testArray)]
[h:broadcast(json.path.put(testArray,"[*][?(@.Test == 1)]","Success",1))]
[h:broadcast(json.path.put(testArray,"[*][?(@.Test == 2)]","Success",0))]

In 1.14.0-alpha1, the output will be:

[{"Test":1},{"Test":3}]
[{"Test":1,"Success":1},{"Test":3}]
Error with function "json.path.put": null
Error trace : chat

Expected Behaviour

In earlier releases, the output of the above would be:

[{"Test":1},{"Test":3}]
[{"Test":1,"Success":1},{"Test":3}]
[{"Test":1,"Success":1},{"Test":3}]

The success key remaining even though testArray was not actually set to the result of the json.path.put function in the 3rd line brings up another issue (#4122), but ignoring that for now, the code fully executed despite not having any valid targets in line 4.

Screenshots

No response

MapTool Info

1.14.0-alpha1

Desktop

Windows 10

Additional Context

No response

@kwvanderlinde
Copy link
Collaborator

kwvanderlinde commented Jun 12, 2023

This was a breaking change in json-path 2.7.0:

It's not clear whether this is desired behavior (i.e., they could just be more strict now about existing expectations) or if this was some collateral damage. We may need to push for some updates there, otherwise I don't think there's much we can do about it.

@kwvanderlinde kwvanderlinde changed the title [Bug]: [Bug]: json.path.put fails if there are no results Jun 16, 2023
@Pmofmalasia
Copy link
Author

Was just revisiting this issue, and apparently was not paying close enough attention to the links last time. It looks like issues/changes you've posted are from much older versions, with the change happening in 2019. This issue is affecting code that works in the 1.13.2 release, but not 1.14.0-alpha1 - not sure how that change would be the cause?

@kwvanderlinde
Copy link
Collaborator

My mistake. I had the right issue numbers, but they naturally linked to ths maptool repo instead of the json-path repo 😬 I've updated my comment with the correct links.

@Pmofmalasia
Copy link
Author

Pmofmalasia commented Aug 1, 2023

Been thinking about this one as I plan to make edits in order to test 1.14.0-alpha3. My planned solution on the MTScript side was just to make a wrapper function that first runs json.path.read() on the path given, then only runs json.path.put() if the resulting array is not empty. Would something like this be doable in MapTool itself so compatibility can be maintained without editing?

@Pmofmalasia
Copy link
Author

Pmofmalasia commented Aug 15, 2023

I've been able to work around this issue using a UDF as follows:


[h:JSONToRead = arg(0)]
[h:JSONPath = arg(1)]

[h,if(json.type(JSONPath) == "ARRAY"),CODE:{
	[h:JSONOnlyNode = json.get(JSONPath,0)]
	[h,if(isNumber(JSONOnlyNode) || JSONOnlyNode == "*"):
		JSONPath = "\$["+JSONOnlyNode+"]";
		JSONPath = "\$['"+JSONOnlyNode+"']"
	]
}]

[h:validEndpoints = json.path.read(JSONToRead,JSONPath)]

[h,if(json.isEmpty(validEndpoints)):
	finalJSON = JSONToRead;
	finalJSON = json.path.put(JSONToRead,JSONPath,arg(2),arg(3))
]

[h:return(0,finalJSON)]

However, I've also noticed that this issue is present for json.path.set() and json.path.delete() as well, unsurprisingly. Adjusting the above UDF as needed for those specific functions works as well.

While I was able to work around this easily with find and replace, users not using addons will probably have a more difficult time correcting all of their json.path functions. It would be preferable to fix this natively if possible.

@aliasmask
Copy link

I haven't use .put yet, but does it work if you add the "SUPPRESS_EXCEPTIONS" option? I pretty much always use that.

@Pmofmalasia
Copy link
Author

json.path.put(), .set(), and .delete() don't accept config options - I attempted it just in case it wasn't documented on the wiki, but I get the error Error with function "json.path.put": Function "json.path.put" requires exactly 4 parameters; 5 were provided. if I attempt it.

json.path.read() is not affected by this change, and it's the only one that accepts config options, so I'm not sure if it would help or not. Perhaps that could be something to attempt, if possible.

@aliasmask
Copy link

That makes sense since it's altering the data structure. Sorry for the dumb question.

@kwvanderlinde
Copy link
Collaborator

Tested. json.path.put(), json.path.set(), and json.path.delete() all now work even when the path doesn't match anything. The result is the unmodified input.

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

Successfully merging a pull request may close this issue.

3 participants