fixes #67, fixes #1: Migrate to Jersey 2 and Swagger #70

guusdk · 2021-12-18T11:13:04Z

This updates the Jersey dependency from 1.x to 2.x.

This commit requires OF-2352 / igniterealtime/Openfire#1945 which means that this PR will not work with 4.7.0 beta (but hopefully will with 4.7.0 non-beta).

Redor · 2021-12-18T15:27:02Z

wow, thats a good one. I have to take a bit of time to test it.

Fishbowler · 2022-01-01T22:48:14Z

Rebased.

Fishbowler · 2022-01-01T22:49:05Z

Testing on 4.7.0 master looks stable. Read and Write requests have identical behaviour and body content as master.

guusdk · 2022-01-07T16:22:32Z

I've now added conversion to Swagger. It is fully functional, but not all documentation has been ported yet (work in progress).

guusdk · 2022-01-07T19:14:57Z

I think I've added documentation for all endpoints. This is ready for review.

guusdk · 2022-01-07T19:17:57Z

Note that the Swagger documentation that's added in this PR can't be used 'offline'. We still need to replace the static documentation (in readme.html) with something that's generated during compilation/packaging. This effort is tracked as issue #71.

I'm not sure if I get around to doing that soon (the holidays have come to an end :) ). Is this PR acceptable as-is?

Fishbowler · 2022-01-09T18:20:35Z

This is great! I've even found a few new bugs using the swagger tool :)
Is there a way to set defaults for the XML Elements that are POST bodies? We could chip away at that in future PRs.
Also, any idea how to hide the /application.wadl entries in the default section at the bottom?

Fishbowler · 2022-01-09T18:22:19Z

All of the descriptions look accurate. I've added one tiny tweak to mirror the same constraint on usergroup updates.

guusdk · 2022-01-09T19:11:37Z

Is there a way to set defaults for the XML Elements that are POST bodies?

I don't recall that we can set defaults, but I think we can have more meaningful examples (with more valid-looking data than 'string').

Also, any idea how to hide the /application.wadl entries in the default section at the bottom?

I tried & failed. It should be easy - but disabling wadl support (which should prevent that documentation to be added didn't actually seem to disable wadl support. I'd be happy for that to be done, as I suspect that the endpoints won't be functional anyway.

GregDThomas · 2022-01-09T19:13:44Z

FWIW, the values in the page default to the examples you configure in the code.

guusdk · 2022-01-09T19:13:45Z

@Redor what do you think? For the 4.7.0 release of Openfire, we will need to publish a new version of this plugin. I'd love to include this PR.

Redor · 2022-01-09T19:38:50Z

It does look good, but didn't had the time to test it. (I will be tomorrow back from vacation). Will review it ASAP

Redor · 2022-01-12T21:15:22Z

src/java/org/jivesoftware/openfire/plugin/rest/AuthFilter.java

    @Override
-    public ContainerRequest filter(ContainerRequest containerRequest) throws WebApplicationException {
+    public void filter(ContainerRequestContext containerRequest) throws IOException {
+        if (containerRequest.getUriInfo().getRequestUri().getPath().equals("/plugins/restapi/v1/openapi.yaml")) {


Should we exclude it? In swagger UI we can set the secret key. I think we should make fewer "exceptions" in the authentication part, which is crucial.

My reasoning for it to include it is for non-authenticated users to make use of the Swagger UI, which needs this file.

Every request made from the Swagger UI obviously needs to be authenticated, but that can be achieved through the Swagger UI-provided functionality.

Redor · 2022-01-12T21:27:19Z

ServiceException seems not to work, to show specific Error and status codes. E.g. instead of a "bad request", I'm getting always 500. Probably the handling is slightly different in jersey 2 or something specific to that request.

Example:
POST http://localhost:9090/plugins/restapi/v1/users

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<user>
    <username>test3</username>
</user>

which should be Bad request (because password is missing), but 500 will be returned

Fishbowler · 2022-01-13T09:18:43Z

Another problem on a similar vein.
I've used the openapi.yaml to import as anew collection into Insomnia, which defaults to the JSON representation.
Something looks amiss with the collection-type properties.

POST http://localhost:9090/plugins/restapi/v1/users

{
  "username": "joseph",
  "name": "Joseph",
  "email": "[email protected]",
  "password": "secret",
  "properties": [
    {
      "key": "somekey",
      "value": "somevalue"
    }
  ]
}

Unrecognized field "properties" (class org.jivesoftware.openfire.plugin.rest.entity.UserEntity), not marked as ignorable (5 known properties: "name", "email", "property", "username", "password"])
at [Source: (org.glassfish.jersey.message.internal.ReaderInterceptorExecutor$UnCloseableInputStream); line: 6, column: 18] (through reference chain: org.jivesoftware.openfire.plugin.rest.entity.UserEntity["properties"])

Using XML doesn't encounter the same issue. Something to do with the JSON representation not happy with the XmlElementWrapper? https://github.com/guusdk/openfire-restAPI-plugin/blob/67_Migrate-to-Jersey-2/src/java/org/jivesoftware/openfire/plugin/rest/entity/UserEntity.java#L140

guusdk · 2022-01-13T09:50:25Z

ServiceException seems not to work, to show specific Error and status codes. E.g. instead of a "bad request", I'm getting always 500.

The RestExceptionMapper didn't automatically register. I have now modified the code to do that manually which fixes the problem.

This updates the Jersey dependency from 1.x to 2.x. This commit requires OF-2352 / igniterealtime/Openfire#1945 which means that this PR will _not_ work with 4.7.0 beta (but hopefully will with 4.7.0 non-beta).

This commit introduces an automatically generated Swagger-based REST client on /plugins/restapi/docs/index.html Note that this can't be used 'offline'. We still need to replace the static documentation (in readme.html) with something that's generated during compilation/packaging (issue igniterealtime#71)

By adding certain annotations, the documentation as generated by Swagger becomes enriched.

guusdk · 2022-01-13T11:06:02Z

Another problem on a similar vein.

Do you know if that happened before this PR? If so, I'd rather take on this issue in a separate effort.

Fishbowler · 2022-01-13T14:26:49Z

Do you know if that happened before this PR?

Tested. It's new :(

guusdk · 2022-01-13T15:20:51Z

Am I right to conclude that it affects POSTs, but not GETs?

guusdk · 2022-01-13T15:28:33Z

Also, the JSON example titled "REST API Version 1.3.0 and later - Payload Example 3 (available parameters):" in the version prior to my changes was already not working, right? If not, then I'm not going to reinstate that. I expect that this is a bug in the documentation: it should have shown only the first element of that 'users' array, instead of the entire array.

guusdk · 2022-01-13T15:53:53Z

I think I've now fixed this for all occurrences of XmlElementWrapper. I've compared JSON based retrieval and posts with version 1.6.0 of the plugin, and I think that with this update, we retain the existing behavior in 1.6.0.

Fishbowler · 2022-01-13T21:19:24Z

I'm seeing working JSON, and the lovely custom errors when I do the wrong thing. 👍 👍
Will spend a little while longer testing this time!

Fishbowler · 2022-01-13T23:59:29Z

I've tested everything except the MUC permissions endpoints and the legacy UserService, and pushed one additional commit to add another annotation.
Everything JSON is working - I've not tested much in XML, as it's likely safer.

Redor · 2022-01-16T15:38:08Z

Thy all for the great effort!

guusdk requested a review from Redor December 18, 2021 11:13

Fishbowler force-pushed the 67_Migrate-to-Jersey-2 branch from ac5050f to 2ce034b Compare January 1, 2022 22:47

guusdk marked this pull request as draft January 7, 2022 16:21

guusdk force-pushed the 67_Migrate-to-Jersey-2 branch 2 times, most recently from 38157e7 to 8a47fbc Compare January 7, 2022 19:13

guusdk marked this pull request as ready for review January 7, 2022 19:14

guusdk changed the title ~~fixes #67: Migrate to Jersey 2~~ fixes #67, fixes #1: Migrate to Jersey 2 and Swagger Jan 7, 2022

Redor reviewed Jan 12, 2022

View reviewed changes

guusdk force-pushed the 67_Migrate-to-Jersey-2 branch from 2dfe26f to af31ebe Compare January 13, 2022 09:43

guusdk added 4 commits January 13, 2022 10:55

fixes igniterealtime#67: Migrate to Jersey 2

6e0bc01

This updates the Jersey dependency from 1.x to 2.x. This commit requires OF-2352 / igniterealtime/Openfire#1945 which means that this PR will _not_ work with 4.7.0 beta (but hopefully will with 4.7.0 non-beta).

fixup

b4dd0d8

Add Swagger annotations to document endpoints

863bd66

By adding certain annotations, the documentation as generated by Swagger becomes enriched.

guusdk force-pushed the 67_Migrate-to-Jersey-2 branch from af31ebe to 863bd66 Compare January 13, 2022 09:56

Add JsonProperty matching XmlElementWrapper name

0d94ed4

Add JsonProperty annotation for a user's groupnames

a42b1a7

Redor merged commit 5ced36b into igniterealtime:master Jan 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes #67, fixes #1: Migrate to Jersey 2 and Swagger #70

fixes #67, fixes #1: Migrate to Jersey 2 and Swagger #70

guusdk commented Dec 18, 2021

Redor commented Dec 18, 2021

Fishbowler commented Jan 1, 2022

Fishbowler commented Jan 1, 2022

guusdk commented Jan 7, 2022

guusdk commented Jan 7, 2022

guusdk commented Jan 7, 2022

Fishbowler commented Jan 9, 2022

Fishbowler commented Jan 9, 2022

guusdk commented Jan 9, 2022

GregDThomas commented Jan 9, 2022

guusdk commented Jan 9, 2022

Redor commented Jan 9, 2022

Redor Jan 12, 2022

guusdk Jan 13, 2022

Redor commented Jan 12, 2022 •

edited

Loading

Fishbowler commented Jan 13, 2022

guusdk commented Jan 13, 2022

guusdk commented Jan 13, 2022

Fishbowler commented Jan 13, 2022

guusdk commented Jan 13, 2022

guusdk commented Jan 13, 2022 •

edited

Loading

guusdk commented Jan 13, 2022

Fishbowler commented Jan 13, 2022

Fishbowler commented Jan 13, 2022

Redor commented Jan 16, 2022

fixes #67, fixes #1: Migrate to Jersey 2 and Swagger #70

fixes #67, fixes #1: Migrate to Jersey 2 and Swagger #70

Conversation

guusdk commented Dec 18, 2021

Redor commented Dec 18, 2021

Fishbowler commented Jan 1, 2022

Fishbowler commented Jan 1, 2022

guusdk commented Jan 7, 2022

guusdk commented Jan 7, 2022

guusdk commented Jan 7, 2022

Fishbowler commented Jan 9, 2022

Fishbowler commented Jan 9, 2022

guusdk commented Jan 9, 2022

GregDThomas commented Jan 9, 2022

guusdk commented Jan 9, 2022

Redor commented Jan 9, 2022

Redor Jan 12, 2022

Choose a reason for hiding this comment

guusdk Jan 13, 2022

Choose a reason for hiding this comment

Redor commented Jan 12, 2022 • edited Loading

Fishbowler commented Jan 13, 2022

guusdk commented Jan 13, 2022

guusdk commented Jan 13, 2022

Fishbowler commented Jan 13, 2022

guusdk commented Jan 13, 2022

guusdk commented Jan 13, 2022 • edited Loading

guusdk commented Jan 13, 2022

Fishbowler commented Jan 13, 2022

Fishbowler commented Jan 13, 2022

Redor commented Jan 16, 2022

Redor commented Jan 12, 2022 •

edited

Loading

guusdk commented Jan 13, 2022 •

edited

Loading