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

Jq extra functions missing #68

Closed
Jiehong opened this issue Jan 9, 2023 · 3 comments · Fixed by #69
Closed

Jq extra functions missing #68

Jiehong opened this issue Jan 9, 2023 · 3 comments · Fixed by #69

Comments

@Jiehong
Copy link
Contributor

Jiehong commented Jan 9, 2023

Hello there,

The following dependency could also be added for some extra functions:

<dependency>
      <groupId>net.thisptr</groupId>
      <artifactId>jackson-jq-extra</artifactId>
      <version>${jackson-jq.version}</version>
    </dependency>

(Source: https://github.com/eiiches/jackson-jq#using-jackson-jqextras-module)

So that those functions are correctly registered for reflection as well.

WDYT?

@ricardozanini
Copy link
Member

ricardozanini commented Jan 9, 2023

That's a good idea. Are you willing to send a PR and verify if they are correctly registered?
Asking because, IIRC, I tried this before, and for some reason, it failed. I am trying to remember what the reason was.

@Jiehong
Copy link
Contributor Author

Jiehong commented Jan 9, 2023

I just tried something, and adding this as a dependency seems to work, even without registering for reflection.

Wondered why, and it seems that this extension finds all BuiltinFunctions (https://github.com/quarkiverse/quarkus-jackson-jq/blob/main/deployment/src/main/java/io/quarkiverse/jackson/jq/deployment/JacksonJqProcessor.java#L59), and the jq extra functions are all annotation (eg: https://github.com/eiiches/jackson-jq/blob/develop/1.x/jackson-jq-extra/src/main/java/net/thisptr/jackson/jq/extra/functions/HostnameFunction.java#L21), and so it works just fine.

I can create a PR to simply add the extra as a dependency in this extension then, and that should be enough.

@ricardozanini
Copy link
Member

If only adding the extras dependency work, we can add a new module -extras (runtime AND deployment) here. So users pick their favorite.

Not sure if adding the extras in the main module is a good idea since some use cases might not leverage from it. They are kept in the extras by the underlying library for a reason. I think it's a reasonable approach to do the same here.

Jiehong added a commit to Jiehong/quarkus-jackson-jq that referenced this issue Jan 9, 2023
Jiehong added a commit to Jiehong/quarkus-jackson-jq that referenced this issue Jan 9, 2023
ricardozanini added a commit that referenced this issue Jan 12, 2023
feature: provide jackson jq extra as an independent module (#68)
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

Successfully merging a pull request may close this issue.

2 participants