----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57582/#review169051 -----------------------------------------------------------
Fix it, then Ship it! Fix It and Ship It! Overall I like the patch, because then we will be able to generate any kind of client code from ambari-server source code, which would be great accomplishment. I have just one problem that i need to mention here. You are using Implicit params everywhere. That means, if anything change on the inputs (REST API), that would mean you will need to change the annotations. (maybe someone can miss that). In long term we shold use POJOs as inputs, then any field change would be generated for swagger documentation. I know that would be a really big change, because that would need to refactor the whoule rest api and resource provider layer. (some change that would be needed: upgrade to jersey2, because it supports to use @BeanParam annotations, thats needed if we want to mix @PathParam and @QueryParam, also validations should be defined on the POJOs, and we would need a custom entity-filter configuration for jersey - default is like: ?select=object.property, we are using ?fields=Object/property), but anyway, that would be the right way. ambari-web/api-docs/index.html Lines 1 (patched) <https://reviews.apache.org/r/57582/#comment241364> Instead of storing all of html + css files from swagger in the source code, I think its better idea to use swagger ui webjar: https://mvnrepository.com/artifact/org.webjars/swagger-ui (i think the exact swagger-ui version is not there that you used, but newer one can work) I used that for Log Search. (you can unpack that during the build or use resource files from the jar somehow else) If you will need any custom files you can override those files during the build. In Log Search i needed a specific index.html which supports basic authentication instead of api key, so i changed the index.html (https://github.com/apache/ambari/blob/trunk/ambari-logsearch/ambari-logsearch-portal/src/main/resources/swagger/swagger.html), but for every other file is used from the webjar, so i did not need to store swagger html+css in the source code. - Oliver Szabo On March 15, 2017, 7:21 p.m., Jaimin Jetly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57582/ > ----------------------------------------------------------- > > (Updated March 15, 2017, 7:21 p.m.) > > > Review request for Ambari, Jonathan Hurley, Jayush Luniya, Nate Cole, Oliver > Szabo, Robert Nettleton, Sumit Mohanty, Sid Wagle, and Yusaku Sako. > > > Bugs: AMBARI-20436 > https://issues.apache.org/jira/browse/AMBARI-20436 > > > Repository: ambari > > > Description > ------- > > A separate branch is created for this work: ambari-rest-api-explorer > > ## As part of this task, following changes are done: > 1. Users, Groups and Views API are integrated with swagger and exposed from > ambari rest api explorer ui (swagger ui) on a deployed cluster at path: > http://c6404.ambari.apache.org:8080/api-docs > 2. swagger-maven-plugin is used to generate swagger.json file on compile > time. This file is published in web resources directory. Note that this file > is generated build time and will be available on deployed ambari-server host > at web resources location but it is not yet decided to be committed and > maintained in Ambari source code > 3. swagger2markup-maven-plugin is used to generate asciidoc from swagger.json > file (that can be shown as markdown in github). More information about this > format an be found at http://asciidoc.org/ and http://asciidoctor.org/. This > generates files in docs/api/asciidoc/** location at build time. This > directory is currently intended to be committed and maintained in ambari > source code > 4. swagger-ui (version: v2.1.1-M2) compiled code with the different css skin > (adopted from [link|https://github.com/jensoleg/swagger-ui]) is committed to > ambari-web/api-docs directory with certain modification to make it work with > ambari api. Further ui polishing will be done in subsequent tasks. Also there > is a strong possibility to maintain the fork code of swagger-ui and compile > (minify and concanate) it during ambari compile time rather than directly > using swagger-ui dist files. Doing so will help developers when customization > done over swagger-ui will increase > 5. swagger-annotation expects application to define schema of request body > and response for each endpoint to be encapsulated in a class. While Ambari > follows this pattern for some of the endpoint, there are many others which > does not do so. For The ones which do not does so, new request and response > classes were defined. Going forward at the completion of this epic, either > each resource type or each resource provider should be coupled with a > resource response class and a resource request class. As part of this patch, > each resourceprovider worked upon introduces a new method "getResponse". At > completion of this epic ResourceProvider interface should also declare > methods like "getResponse" and "getRequest" that returns response schema and > request schema instances for the resource API endpoints > 6. Currently it seems that swagger has a limitation in supporting > "subresource locator methods". This issue is been reported to swagger > community and is being tracked at > [link|https://github.com/swagger-api/swagger-core/issues/2136]. As a result > of which currently as a temporary workaround, all subresources are converted > to root resources. Also all root resources on similar path are moved under > same subpackages (like userApi, groupApi and viewApi). > > > Diffs > ----- > > LICENSE.txt f05016f > ambari-server/docs/api/asciidoc/definitions.adoc PRE-CREATION > ambari-server/docs/api/asciidoc/overview.adoc PRE-CREATION > ambari-server/docs/api/asciidoc/paths.adoc PRE-CREATION > ambari-server/pom.xml f0c73e4 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ActiveWidgetLayoutService.java > 40cd6e0 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/GroupPrivilegeService.java > 9c1f1a3 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/GroupService.java > 8ec0097 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/MemberService.java > 28e53e6 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UserAuthorizationService.java > 26a7107 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UserPrivilegeService.java > f9c95e7 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UserService.java > 31f3a8c > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewDataMigrationService.java > 2a9aa64 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewExternalSubResourceService.java > 66ccae7 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewInstanceService.java > 53d4918 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewPermissionService.java > 8f7f4ef > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewPrivilegeService.java > 7393745 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewService.java > 17a9f34 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewSubResourceService.java > 76d28fe > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewVersionService.java > 3554da1 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/groupApi/GroupPrivilegeService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/userApi/ActiveWidgetLayoutService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/userApi/UserAuthorizationService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/userApi/UserPrivilegeService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/viewApi/ViewPermissionService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/viewApi/ViewPrivilegeService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ActiveWidgetLayoutRequest.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ActiveWidgetLayoutResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ApiModel.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/GroupPrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/GroupRequest.java > 1bc18cc > > ambari-server/src/main/java/org/apache/ambari/server/controller/GroupResponse.java > 0baccc7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/MemberRequest.java > 0245f36 > > ambari-server/src/main/java/org/apache/ambari/server/controller/MemberResponse.java > 3dc6558 > > ambari-server/src/main/java/org/apache/ambari/server/controller/PrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserAuthorizationResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserPrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserRequest.java > 282131a > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserResponse.java > b90f864 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewInstanceRequest.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewInstanceResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewPermissionResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewPrivilegeRequest.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewPrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewVersionResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java > d9a7997 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupPrivilegeResourceProvider.java > bf63794 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserAuthorizationResourceProvider.java > 8193a49 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProvider.java > b9b756b > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPermissionResourceProvider.java > 4f0a6f0 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewVersionResourceProvider.java > 4055f1a > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrincipalTypeEntity.java > e95223a > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/User.java > 18509d3 > ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java > 81c4734 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ActiveWidgetLayoutServiceTest.java > adae956 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/GroupPrivilegeServiceTest.java > 995dfaf > > ambari-server/src/test/java/org/apache/ambari/server/api/services/GroupServiceTest.java > b171ceb > > ambari-server/src/test/java/org/apache/ambari/server/api/services/MemberServiceTest.java > 1e0137e > > ambari-server/src/test/java/org/apache/ambari/server/api/services/UserAuthorizationServiceTest.java > 8a2f799 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/UserPrivilegeServiceTest.java > 7b0e06d > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewDataMigrationServiceTest.java > e1eb0de > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewExternalSubResourceServiceTest.java > 155f91e > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewPermissionServiceTest.java > 7f58fb2 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewSubResourceServiceTest.java > 9499466 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/GroupPrivilegeResourceProviderTest.java > ebe92e4 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProviderTest.java > 5092db5 > ambari-web/api-docs/css/api-explorer.css PRE-CREATION > ambari-web/api-docs/css/index.css PRE-CREATION > ambari-web/api-docs/css/print.css PRE-CREATION > ambari-web/api-docs/css/reset.css PRE-CREATION > ambari-web/api-docs/css/screen.css PRE-CREATION > ambari-web/api-docs/css/standalone.css PRE-CREATION > ambari-web/api-docs/css/typography.css PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.eot PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.svg PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.ttf PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.woff PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.woff2 PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.eot PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.svg PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.ttf PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.woff PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.woff2 PRE-CREATION > ambari-web/api-docs/images/Swagger_explorer.png PRE-CREATION > ambari-web/api-docs/images/Swagger_explorer_min.png PRE-CREATION > ambari-web/api-docs/images/explorer_icons.png PRE-CREATION > ambari-web/api-docs/images/favicon-16x16.png PRE-CREATION > ambari-web/api-docs/images/favicon-32x32.png PRE-CREATION > ambari-web/api-docs/images/favicon.ico PRE-CREATION > ambari-web/api-docs/images/json_editor_integration.png PRE-CREATION > ambari-web/api-docs/images/logo_small.png PRE-CREATION > ambari-web/api-docs/images/pet_store_api.png PRE-CREATION > ambari-web/api-docs/images/senodio.png PRE-CREATION > ambari-web/api-docs/images/throbber.gif PRE-CREATION > ambari-web/api-docs/images/wordnik_api.png PRE-CREATION > ambari-web/api-docs/index.html PRE-CREATION > ambari-web/api-docs/lib/backbone-min.js PRE-CREATION > ambari-web/api-docs/lib/bootstrap.min.js PRE-CREATION > ambari-web/api-docs/lib/handlebars-2.0.0.js PRE-CREATION > ambari-web/api-docs/lib/highlight.7.3.pack.js PRE-CREATION > ambari-web/api-docs/lib/jquery-1.8.0.min.js PRE-CREATION > ambari-web/api-docs/lib/jquery.ba-bbq.min.js PRE-CREATION > ambari-web/api-docs/lib/jquery.slideto.min.js PRE-CREATION > ambari-web/api-docs/lib/jquery.wiggle.min.js PRE-CREATION > ambari-web/api-docs/lib/jsoneditor.js PRE-CREATION > ambari-web/api-docs/lib/marked.js PRE-CREATION > ambari-web/api-docs/lib/swagger-oauth.js PRE-CREATION > ambari-web/api-docs/lib/underscore-min.js PRE-CREATION > ambari-web/api-docs/lib/underscore-min.map PRE-CREATION > ambari-web/api-docs/o2c.html PRE-CREATION > ambari-web/api-docs/swagger-ui.js PRE-CREATION > ambari-web/api-docs/swagger-ui.min.js PRE-CREATION > ambari-web/brunch-config.js 31eb1cb > > > Diff: https://reviews.apache.org/r/57582/diff/2/ > > > Testing > ------- > > Tested on dev environment for all integrated apis to be working from ui > interface > Verified that Rat check, style check and existing ambari-server unit tests > are passing with the patch > > > Thanks, > > Jaimin Jetly > >
