OHFJIRA-52 list of exposed webservices #52

kkovarik · 2017-05-13T23:42:33Z

ABOUT

Overview of exposed Webservices. See related JIRA task for more info: https://openhubframework.atlassian.net/browse/OHFJIRA-52

CHANGES

added new REST endpoint GET /api/ws with simple test. It does reuse WsdlRegistry functionality.

coveralls · 2017-05-13T23:58:19Z

Coverage increased (+0.4%) to 55.953% when pulling fa7bcc0 on feature/OHFJIRA-52-ws-overview into f0f307c on develop.

pjuza

Is there API documentation in Apiary? Also mockup diagram for GUI developers in Balsamiq would be nice.

pjuza · 2017-05-14T12:43:55Z

web-admin/src/main/java/org/openhubframework/openhub/admin/web/ws/rest/WsController.java

+ */
+@RestController
+@RequestMapping(value = WsController.REST_URI)
+public class WsController extends AbstractOhfController {


Maybe WsdlController would be better ...

And where to expose it, directly under /api/wsdl?

No problem with that, I do not have any strict opinion on this, my initial thinking was like - ok, what is wsdl? definition of service, resource itself desired by consumer is the webservice.

pjuza · 2017-05-14T12:45:02Z

web-admin/src/main/java/org/openhubframework/openhub/admin/web/ws/rpc/WsInfoRpc.java

+ * @author Karel Kovarik
+ */
+@XmlRootElement
+public class WsInfoRpc {


Same as controller - wouldn't be better to use "wsdl" prefix, isn't it more descriptive?

Possibly, as in previous comment, if it is the wsdl resource this endpoint should list, then yes, I will change it...

Yes, it's overview of input web service definitions (wsdl).
We will add overview of REST services in the next version ...

kkovarik · 2017-05-14T17:27:47Z

I apologize, the links to drafts of mockup and API documentation were mentioned only in the JIRA task:

Api draft: http://docs.openhubframework.apiary.io/#reference/0/openhub-web-services/gets-list-of-exposed-webservices
Mockup draft: https://openwise.mybalsamiq.com/projects/openhub/WS%20overview

pjuza · 2017-05-14T18:13:07Z

Please change URI: /api/ws -> /web/admin/api/ws

If you accept change from ws to wsdl then please change also names in API.

hanusto

I recommend to rename WS -> WSDL and whole endpoint should be in /api/services context. After that I agree with that.

hanusto · 2017-05-14T18:18:48Z

web-admin/src/main/java/org/openhubframework/openhub/admin/web/ws/rest/WsController.java

+    /**
+     * Base REST uri for the controller.
+     */
+    public static final String REST_URI = BASE_PATH + "/ws";


And WSDL for endpoint URL too. Maybe my recommendation is /api/services/wsdl

hanusto · 2017-05-14T18:51:20Z

web-admin/src/main/java/org/openhubframework/openhub/admin/web/ws/rest/WsController.java

+    /**
+     * Uri part on which wsdl is exposed.
+     */
+    private static final String WS_URI = "/ws/";


I dont know if the WS context can be configured or not. If yes, this should be the same.

Cannot, however changed it to the constant used to actually configure the context (RouteConstants.WS_URI_PREFIX).

hanusto · 2017-05-14T18:52:20Z

web-admin/src/main/java/org/openhubframework/openhub/admin/web/ws/rest/WsController.java

+     * @param request the input httpServletRequest.
+     * @return list of WsInfoRpc domain objects.
+     */
+    @GetMapping(produces = {"application/xml", "application/json"})


kkovarik · 2017-05-14T22:03:19Z

Thanks for the review, updated endpoint to /api/services/wsdl and refactored to WsdlController & WsdlInfoRpc etc. feel free to check again.

API documentation updated: http://docs.openhubframework.apiary.io/#reference/0/openhub-web-services, now I am using collection wrapping as it is introduced in PR#53: #53

Will wait until PR#53 is merged, then rebase this branch to use the common CollectionWrapper as well...

coveralls · 2017-05-14T22:13:45Z

Coverage increased (+0.3%) to 55.89% when pulling ee61348 on feature/OHFJIRA-52-ws-overview into f0f307c on develop.

coveralls · 2017-05-14T22:25:43Z

Coverage increased (+0.3%) to 55.89% when pulling ee61348 on feature/OHFJIRA-52-ws-overview into f0f307c on develop.

hanusto · 2017-05-15T05:05:14Z

.../src/main/java/org/openhubframework/openhub/admin/web/services/wsdl/rest/WsdlController.java

+    /**
+     * Get list of exposed web services.
+     * @param request the input httpServletRequest.
+     * @return list of WsInfoRpc domain objects wrapped in CollectionWrapper.


WsInfoRpc -> WsdlInfoRpc, or use {@link WsInfoRpc}. This was refactored automatically. The same for {@link CollectionWrapper}.

Thanks, missed that, fixed.

hanusto

Now it is perfect. Only one improvement introduced -> javadoc. PR #53 merged so you can rebase it with requested changes.

hanusto · 2017-05-15T18:46:29Z

PR #53 is merged, so you can rebase that.

coveralls · 2017-05-15T19:13:38Z

Coverage increased (+0.2%) to 56.315% when pulling 0c0759e on feature/OHFJIRA-52-ws-overview into 37fe88e on develop.

kkovarik · 2017-05-15T19:14:07Z

@hanusto thanks for fast merge, rebased on current develop.

kkovarik added the improvement label May 13, 2017

kkovarik added this to the v2.0 milestone May 13, 2017

kkovarik self-assigned this May 13, 2017

kkovarik requested review from hanusto and pjuza May 13, 2017 23:42

pjuza requested changes May 14, 2017

View reviewed changes

hanusto requested changes May 14, 2017

View reviewed changes

kkovarik force-pushed the feature/OHFJIRA-52-ws-overview branch 2 times, most recently from e18f4aa to a0e839a Compare May 14, 2017 21:58

kkovarik added task and removed improvement labels May 14, 2017

kkovarik force-pushed the feature/OHFJIRA-52-ws-overview branch from a0e839a to ee61348 Compare May 14, 2017 22:05

hanusto reviewed May 15, 2017

View reviewed changes

hanusto approved these changes May 15, 2017

View reviewed changes

pjuza approved these changes May 15, 2017

View reviewed changes

OHFJIRA-52 list of exposed webservices

0c0759e

kkovarik force-pushed the feature/OHFJIRA-52-ws-overview branch from ee61348 to 0c0759e Compare May 15, 2017 18:57

hanusto merged commit 40a8e64 into develop May 15, 2017

hanusto deleted the feature/OHFJIRA-52-ws-overview branch May 15, 2017 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OHFJIRA-52 list of exposed webservices #52

OHFJIRA-52 list of exposed webservices #52

kkovarik commented May 13, 2017

coveralls commented May 13, 2017

pjuza left a comment

pjuza May 14, 2017

kkovarik May 14, 2017

pjuza May 14, 2017

kkovarik May 14, 2017

pjuza May 14, 2017

kkovarik commented May 14, 2017

pjuza commented May 14, 2017

hanusto left a comment

hanusto May 14, 2017

kkovarik May 14, 2017

hanusto May 14, 2017

kkovarik May 14, 2017

hanusto May 14, 2017

kkovarik commented May 14, 2017

coveralls commented May 14, 2017

coveralls commented May 14, 2017

hanusto May 15, 2017

kkovarik May 15, 2017

hanusto left a comment •

edited

hanusto commented May 15, 2017

coveralls commented May 15, 2017

kkovarik commented May 15, 2017

OHFJIRA-52 list of exposed webservices #52

OHFJIRA-52 list of exposed webservices #52

Conversation

kkovarik commented May 13, 2017

ABOUT

CHANGES

coveralls commented May 13, 2017

pjuza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkovarik commented May 14, 2017

pjuza commented May 14, 2017

hanusto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkovarik commented May 14, 2017

coveralls commented May 14, 2017

coveralls commented May 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanusto left a comment • edited

Choose a reason for hiding this comment

hanusto commented May 15, 2017

coveralls commented May 15, 2017

kkovarik commented May 15, 2017

hanusto left a comment •

edited