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

Add max, min, and sum functions for dynamic badges #6071

Open
1 of 3 tasks
YashTotale opened this issue Jan 17, 2021 · 6 comments
Open
1 of 3 tasks

Add max, min, and sum functions for dynamic badges #6071

YashTotale opened this issue Jan 17, 2021 · 6 comments
Labels
needs-upstream-help Not actionable without help from a service provider question Support questions, usage questions, unconfirmed bugs, discussions, ideas

Comments

@YashTotale
Copy link
Contributor

YashTotale commented Jan 17, 2021

Are you experiencing an issue with...

🪲 Description

.max(), .sum(), .min() functions in dynamic badge queries don't work.

Support for .max(), .sum(), .min() would be a great help and improve the flexibility of dynamic badges.

Examples:$.numbers.sum(), $.numbers.min(), $.numbers.max(), $.sum($.numbers.max())

🔗 Example badges

Doesn't work when .sum() is present: Failing Badge

Works when .sum() is not present: Working Badge

💡 Possible Solution

I think this is a problem with the upstream jsonpath library, and I have filed an issue in that repo as well. I thought informing the maintainers of this project as well would be a good idea.

Thanks!

@YashTotale YashTotale added the question Support questions, usage questions, unconfirmed bugs, discussions, ideas label Jan 17, 2021
@chris48s chris48s added good first issue New contributors, join in! needs-upstream-help Not actionable without help from a service provider and removed good first issue New contributors, join in! labels Jan 17, 2021
@calebcartwright
Copy link
Member

Could we clarify the scope of what, if anything, is being requested of Shields here? For example, I would assume this is specifically targeted at the json/yaml badges, but the title is a little ambiguous.

My sense is that this is largely just a tracking issue to get clarity on whether the functions are supported upstream and what the corresponding query syntax would be, but that there's no action to be taken for Shields.

@YashTotale
Copy link
Contributor Author

YashTotale commented Jan 17, 2021

Yes, this is not an issue with Shields, but with the package jsonpath that Shields relies on for dynamic json/yaml badges. I was hoping that it could serve as both a tracking issue and a point of reference for people who are running into the same problem.

Additionally, this issue could encourage the maintainers of jsonpath to add these methods if people who view this issue upvote the issue I opened in the jsonpath repo.

@Cimbali
Copy link
Contributor

Cimbali commented Jan 18, 2022

I think this could be fixed easier by switching to a different JSONPath library, such as jsonpath-plus which seems more fully-featured and possibly more maintained.

This JSONPath comparator website seems to indicate that no javascipt implementation has a .sum(), however you can re-create a min() / max() and other useful selectors (e.g. .first()) with jsonpath-plus, as it is slightly less buggy with the () script evaluation.

For example I tried creating a query for the latest successful run of a github workflow:

$.workflow_runs[(@.findIndex(run => run.conclusion == "success"))].updated_at

That could be adapted to find the smallest, instead of first success value. For example, smallest .workflow_runs.*.run_number:

$.workflow_runs[(@.findIndex(run => run.run_number == Math.min(...@.map(run => run.run_number))))].run_number

Rather tedious to write, but that is inherent to how JSONPath works.

Note that with jsonpath:

  1. With no arrow-functions and the need for everything to be evaluated statically, this is how querying the last successful run should be written:
$.workflow_runs[(@.findIndex(function(run) { return (run || {"conclusion": "failure"}).conclusion == "success" }))].updated_at
  1. None of this will not work until Replace @ and $ with valid javascript names before evaluation  dchester/jsonpath#167 is merged

@calebcartwright
Copy link
Member

Thanks for sharing, although just swapping out the library isn't as easy as it may seem/sound (refs #6987, #6786)

@Cimbali
Copy link
Contributor

Cimbali commented Jan 18, 2022

You’re right, I’ve found out the “spec” is pretty poorly defined and pretty much every implementation has its own variants. Here are the 36 / 246 features listed on the comparison page where both jsonpath and jsonpath-plus differ:

                                                                     jsonpath jsonpath-plus                              Example
Feature                                                                                                                         
Array slice with range of 0                                                 ✓             ✗                              $[0:0] 
Array slice with step and leading zeros                                     ✗             ✓                      $[010:024:010] 
Bracket notation with empty path                                            ✓             ✗                                 $[] 
Bracket notation with number on object                                      ✓             ✗                                $[0] 
Bracket notation with quoted array slice literal                            ✓             ✗                              $[':'] 
Bracket notation with quoted closing bracket literal                        ✓             ✗                              $[']'] 
Bracket notation with quoted current object literal                         ✓             e                              $['@'] 
Bracket notation with quoted root literal                                   ✓             ✗                              $['$'] 
Bracket notation with quoted string and unescaped single quote              ✓             ✗                   $['single'quote'] 
Bracket notation with quoted union literal                                  ✓             ✗                              $[','] 
Bracket notation with quoted wildcard literal ⁴                             ✓             ✗                              $['*'] 
Bracket notation with quoted wildcard literal on object without key         ✓             ✗                              $['*'] 
Bracket notation with two literals separated by dot                         ✓             ✗                     $['two'.'some'] 
Bracket notation with two literals separated by dot without quotes          ✓             ✗                         $[two.some] 
Bracket notation without quotes                                             ✓             ✗                              $[key] 
Dot bracket notation without quotes                                         ✓             ✗                             $.[key] 
Dot notation after union with keys                                          ✓             ✗                $['one','three'].key 
Dot notation with dash                                                      ✗             ✓                          $.key-dash 
Dot notation with empty path                                                ✓             ✗                                  $. 
Dot notation with non ASCII key                                             ✗             ✓                                $.屬性 
Dot notation with wildcard after recursive descent on scalar ⁴              e             ✓                                $..* 
Dot notation without dot                                                    ✓             ✗                                  $a 
Filter expression with empty expression                                     ✓             ✗                              $[?()] 
Filter expression with equals on array of numbers                           ✓             e                         $[?(@==42)] 
Filter expression with parent axis operator                                 ✓             ✗  $[*].bookmarks[?(@.page == 45)]^^^ 
Filter expression without parens                                            ✓             ✗                       $[?@.key==42] 
Parens notation                                                             ✓             ✗                         $(key,more) 
Root on scalar                                                              e             ✓                                   $ 
Root on scalar true                                                         e             ✓                                   $ 
Union with duplication from array                                           ✗             ✓                              $[0,0] 
Union with keys                                                             ✓             ✗                  $['key','another'] 
Union with keys on object without key                                       ✓             ✗                  $['missing','key'] 
Union with keys after array slice                                           ✓             ✗                       $[:]['c','d'] 
Union with keys after bracket notation                                      ✓             ✗                       $[0]['c','d'] 
Union with keys after dot notation with wildcard                            ✓             ✗                        $.*['c','d'] 
Union with wildcard and number                                              ✓             ✗                              $[*,1] 

However it seems the differences in the () script evaluation I found are not even mentioned. I think the only option to change libraries would be to deprecate the current json endpoint and create a new one. We might as well PR to jsonpath some more and hope it gets merged.

@calebcartwright
Copy link
Member

The biggest concern for us, given the context of where we are today, is not breaking our existing users. As such I think for the very foreseeable future using a Custom Endpoint will remain the best option for those needing more transformation power on dynamic values

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-upstream-help Not actionable without help from a service provider question Support questions, usage questions, unconfirmed bugs, discussions, ideas
Projects
None yet
Development

No branches or pull requests

4 participants