Added JSONPath syntactic validation #561

andresceballosm · 2024-07-30T20:39:10Z

Type of PR:

Bugfix
Feature
Documentation
Other

Required reviews:

1
2
3

What this does:

Added JSONPath syntactic validation

Related to https://github.com/nucypher/taco-web/pull/550/files#r1696888086

Issues fixed/closed:

Fixes Syntactic validation of JSONPath queries while creating conditions #559

Why it's needed:

Explain how this PR fits in the greater context of the NuCypher Network. E.g.,
if this PR address a nucypher/productdev issue, let reviewers know!

Notes for reviewers:

This depends on the PR #550
With the integrated PR #550 my approach is to use it in the following way

export const JsonApiConditionSchema = z.object({
  conditionType: z.literal(JsonApiConditionType).default(JsonApiConditionType),
  endpoint: z.string().url(),
  parameters: z.record(z.string(), z.unknown()).optional(),
  query: jsonPathSchema.optional(),
  returnValueTest: returnValueTestSchema, // Update to allow multiple return values after expanding supported methods
});

netlify · 2024-07-30T20:39:25Z

✅ Deploy Preview for taco-demo ready!

Name	Link
🔨 Latest commit	`eb78af3`
🔍 Latest deploy log	https://app.netlify.com/sites/taco-demo/deploys/66a94f7087382a0008a2a1e6
😎 Deploy Preview	https://deploy-preview-561--taco-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify · 2024-07-30T20:39:26Z

✅ Deploy Preview for taco-nft-demo ready!

Name	Link
🔨 Latest commit	`eb78af3`
🔍 Latest deploy log	https://app.netlify.com/sites/taco-nft-demo/deploys/66a94f706e363800087b3176
😎 Deploy Preview	https://deploy-preview-561--taco-nft-demo.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

theref

Very nice, thank you for this. I thought the jsonpath package was deprecated now though? and has been replaced by jsonpath-plus

piotr-roslaniec · 2024-07-31T07:15:53Z

packages/taco/package.json

@@ -43,11 +43,13 @@
    "@nucypher/nucypher-core": "*",
    "@nucypher/shared": "workspace:*",
    "ethers": "^5.7.2",
+    "jsonpath": "^1.1.1",


As @theref noted, this package seems to be no longer maintained. jsonpath-plus is in maintenance mode:

Please note: This project is not currently being actively maintained. We may accept well-documented PRs or some simple updates, but are not looking to make fixes or add new features ourselves.

We may want to switch to jsonpath-plus or https://www.npmjs.com/package/nimma

I see the same message on the JSONPath-plus GitHub (https://github.com/JSONPath-Plus/JSONPath)

This messaging also appears to be reiterated in this comment on May 15, 2024 JSONPath-Plus/JSONPath#173 (comment)

As mentioned on the main page of the README, I am not actively maintaining this project, though I am accepting PRs. Feel free to submit a well-documented PR if you find an issue and your question is not answered by the README.

In my opinion, since there appears to be no actively maintained jsonpath package, if the original jsonpath package is functioning as expected (demonstrated in this PR) I'm in favor of merging it ~~as-is~~ if we can get CI to pass.

After all, the only functionality we actually need is the syntactic check and not the evaluation. If jsonpath supports that but jsonpath-plus does not I don't think we have another way to implement this validator without building our own regular expressions.

Due to various issues with jsonpath and jsonpath-plus not functioning as expected, I decided to implement our own JSONPath validator.

derekpierre · 2024-07-31T17:10:27Z

re: CI failures

From @piotr-roslaniec :

"""
Pretty sure this is an issue with compiling jsonpath:

"""

derekpierre · 2024-08-01T14:24:22Z

@andresceballosm can you try using https://www.npmjs.com/package/@astronautlabs/jsonpath (https://github.com/astronautlabs/jsonpath) instead of jsonpath? I did a cursory check locally and it seemed to do the trick.

andresceballosm · 2024-08-01T15:52:42Z

@andresceballosm can you try using https://www.npmjs.com/package/@astronautlabs/jsonpath (https://github.com/astronautlabs/jsonpath) instead of jsonpath? I did a cursory check locally and it seemed to do the trick.

Hey @derekpierre works great! thanks

derekpierre · 2024-08-01T18:07:37Z

packages/taco/test/conditions/base/json-api.test.ts

+    const invalidPath = '$.store.book[?(@.price < ]';
+    const result = jsonPathSchema.safeParse(invalidPath);
+    expect(result.success).toBe(false);
+    if (!result.success) {


Are these if statements necessary? If not, let's remove them. We know result.success is false from the expect in the line above. Same comment for other tests below.

derekpierre · 2024-08-01T19:41:12Z

packages/taco/test/conditions/base/json-api.test.ts

-    if (!result.success) {
-      expect(result.error.errors[0].message).toBe(
-        'Invalid JSONPath expression',
-      );
-    }


Sorry for not being more clear. I meant to keep the expect test but remove the if i.e. keep only

expect(result.error.errors[0].message).toBe( 'Invalid JSONPath expression', );

derekpierre

🎸 - nice work!

derekpierre changed the base branch from main to epic-offchain July 30, 2024 20:52

theref approved these changes Jul 31, 2024

View reviewed changes

piotr-roslaniec reviewed Jul 31, 2024

View reviewed changes

Added JSONPath syntactic validation

4383192

andresceballosm force-pushed the json-path-validation branch from eb78af3 to 4383192 Compare July 31, 2024 14:12

Updated pnpm-lock.yamls and fix linter errors

abb82b7

Removed jsonpath library

f161bc2

Added Library @astronautlabs/jsonpath

c56d7bd

derekpierre reviewed Aug 1, 2024

View reviewed changes

Updated unit test

e42e69e

derekpierre reviewed Aug 1, 2024

View reviewed changes

Update in unit tests

e2d162e

derekpierre approved these changes Aug 1, 2024

View reviewed changes

derekpierre merged commit d59110a into nucypher:epic-offchain Aug 2, 2024
4 checks passed

derekpierre mentioned this pull request Aug 27, 2024

Syntactic validation of JSONPath queries while creating conditions #559

Closed

derekpierre added a commit that referenced this pull request Oct 4, 2024

Added JSONPath syntactic validation (#561)

21f16bf

derekpierre mentioned this pull request Oct 4, 2024

[EPIC] v0.6.x #592

Open

23 tasks

derekpierre added a commit that referenced this pull request Oct 10, 2024

Added JSONPath syntactic validation (#561)

bd6f2b5

derekpierre added a commit that referenced this pull request Oct 10, 2024

Added JSONPath syntactic validation (#561)

b3d2bc1

derekpierre added a commit that referenced this pull request Oct 15, 2024

Added JSONPath syntactic validation (#561)

6576c03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added JSONPath syntactic validation #561

Added JSONPath syntactic validation #561

andresceballosm commented Jul 30, 2024 •

edited by derekpierre

Loading

netlify bot commented Jul 30, 2024 •

edited

Loading

netlify bot commented Jul 30, 2024 •

edited

Loading

theref left a comment

piotr-roslaniec Jul 31, 2024

KPrasch Jul 31, 2024

KPrasch Jul 31, 2024 •

edited

Loading

andresceballosm Jul 31, 2024

derekpierre commented Jul 31, 2024 •

edited

Loading

derekpierre commented Aug 1, 2024

andresceballosm commented Aug 1, 2024

derekpierre Aug 1, 2024

derekpierre Aug 1, 2024

derekpierre left a comment

Added JSONPath syntactic validation #561

Added JSONPath syntactic validation #561

Conversation

andresceballosm commented Jul 30, 2024 • edited by derekpierre Loading

netlify bot commented Jul 30, 2024 • edited Loading

✅ Deploy Preview for taco-demo ready!

netlify bot commented Jul 30, 2024 • edited Loading

✅ Deploy Preview for taco-nft-demo ready!

theref left a comment

Choose a reason for hiding this comment

piotr-roslaniec Jul 31, 2024

Choose a reason for hiding this comment

KPrasch Jul 31, 2024

Choose a reason for hiding this comment

KPrasch Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

andresceballosm Jul 31, 2024

Choose a reason for hiding this comment

derekpierre commented Jul 31, 2024 • edited Loading

derekpierre commented Aug 1, 2024

andresceballosm commented Aug 1, 2024

derekpierre Aug 1, 2024

Choose a reason for hiding this comment

derekpierre Aug 1, 2024

Choose a reason for hiding this comment

derekpierre left a comment

Choose a reason for hiding this comment

andresceballosm commented Jul 30, 2024 •

edited by derekpierre

Loading

netlify bot commented Jul 30, 2024 •

edited

Loading

netlify bot commented Jul 30, 2024 •

edited

Loading

KPrasch Jul 31, 2024 •

edited

Loading

derekpierre commented Jul 31, 2024 •

edited

Loading