You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by kjduling <gi...@git.apache.org> on 2016/10/19 16:22:54 UTC

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

GitHub user kjduling opened a pull request:

    https://github.com/apache/incubator-geode/pull/265

    GEODE-2014: Upgrade Swagger libraries

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kjduling/incubator-geode feature/GEODE-2014

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-geode/pull/265.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #265
    
----
commit c2b15f22886431588e8a21a80e7ff13ea69a7ec2
Author: Kevin Duling <kd...@pivotal.io>
Date:   2016-10-18T22:54:55Z

    GEODE-2014: Upgrade Swagger libraries

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by metatype <gi...@git.apache.org>.
Github user metatype commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/265#discussion_r84111969
  
    --- Diff: gradle/dependency-versions.properties ---
    @@ -100,8 +100,8 @@ spring-tx.version = 4.3.2.RELEASE
     springframework.version = 4.3.2.RELEASE
     stephenc-findbugs.version = 1.3.9-1
     spymemcached.version = 2.9.0
    -swagger.version = 1.3.2
    -swagger-springmvc.version = 0.8.2
    +swagger.version=1.3.13
    --- End diff --
    
    Verified that the license is still ASL 2.0 so no changes to LICENSE.  Also does not include a NOTICE file so we do not need to propagate anything in our NOTICE.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/265#discussion_r84528366
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java ---
    @@ -0,0 +1,57 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +
    +import static org.hamcrest.CoreMatchers.*;
    +import static org.junit.Assert.*;
    +
    +import org.apache.http.HttpResponse;
    +import org.apache.http.client.methods.HttpGet;
    +import org.json.JSONObject;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +
    +import org.apache.geode.internal.i18n.LocalizedStrings;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +
    +@Category(IntegrationTest.class)
    +
    +public class SwaggerVerificationTest extends AbstractRestTest {
    +
    +  @Test
    +  public void isSwaggerRunning() throws Exception {
    +    // Check the UI
    +    HttpResponse response = doRequest(new HttpGet("/geode/swagger-ui.html"), "", "");
    +    assertThat(getCode(response), is(200));
    +
    +    // Check the JSON
    +    response = doRequest(new HttpGet("/geode/v2/api-docs"), "", "");
    --- End diff --
    
    "geode/v2/api-docs" is the endpoint for Swagger2 documentation.  Swagger was "geode/api-docs".  Geode's endpoints are all "geode/v1/*" and I've verified that if/when the REST API revision is bumped to "v2", there will be no conflict unless we create a new endpoint called "v2/api-docs".
    
    This is supposed to be able to be overridden with the "springfox.documentation.swagger.v2.path" env property, but it doesn't seem to be honored.  Or SwaggerConfig is being loaded too late in the lifecycle, which I doubt.  At any rate, I am adding the structure to use this, but leaving the values set to the default ones.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode issue #265: GEODE-2014: Upgrade Swagger libraries

Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on the issue:

    https://github.com/apache/incubator-geode/pull/265
  
    Added a test to see if Swagger is up and the splash page has the right Geode headers.
    Upgraded to the Swagger2 specification
    Removed the accidental checkin of the sleep.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/265#discussion_r84119234
  
    --- Diff: gradle/dependency-versions.properties ---
    @@ -100,8 +100,8 @@ spring-tx.version = 4.3.2.RELEASE
     springframework.version = 4.3.2.RELEASE
     stephenc-findbugs.version = 1.3.9-1
     spymemcached.version = 2.9.0
    -swagger.version = 1.3.2
    -swagger-springmvc.version = 0.8.2
    +swagger.version=1.3.13
    --- End diff --
    
    However, 1.3.13 is an old Swagger release. I don't think we should upgrade to an old version. If we're going to upgrade then let's upgrade to the latest version (1.5.10?).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/265#discussion_r84501406
  
    --- Diff: geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/FunctionAccessController.java ---
    @@ -93,8 +93,7 @@ protected String getRestApiVersion() {
           response = void.class
       )
       @ApiResponses({
    -      @ApiResponse(code = 200, message = "OK."),
    -      @ApiResponse( code = 401, message = "Invalid Username or Password." ),
    +      @ApiResponse(code = 200, message = "OK."), @ApiResponse(code = 401, message = "Invalid Username or Password."),
    --- End diff --
    
    use the old fomat?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by metatype <gi...@git.apache.org>.
Github user metatype commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/265#discussion_r84112416
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java ---
    @@ -94,7 +94,7 @@ public static void before() throws Exception {
       @Test
       public void testFunctions() throws Exception {
         String json = "{\"@type\":\"double\",\"@value\":210}";
    -
    +    Thread.sleep(500000);
    --- End diff --
    
    This seems like a really long pause for an integration test.  Is there another way to do this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by jinmeiliao <gi...@git.apache.org>.
Github user jinmeiliao commented on a diff in the pull request:

    https://github.com/apache/incubator-geode/pull/265#discussion_r84500905
  
    --- Diff: geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java ---
    @@ -0,0 +1,57 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.geode.rest.internal.web;
    +
    +
    +import static org.hamcrest.CoreMatchers.*;
    +import static org.junit.Assert.*;
    +
    +import org.apache.http.HttpResponse;
    +import org.apache.http.client.methods.HttpGet;
    +import org.json.JSONObject;
    +import org.junit.Test;
    +import org.junit.experimental.categories.Category;
    +
    +import org.apache.geode.internal.i18n.LocalizedStrings;
    +import org.apache.geode.test.junit.categories.IntegrationTest;
    +
    +@Category(IntegrationTest.class)
    +
    +public class SwaggerVerificationTest extends AbstractRestTest {
    +
    +  @Test
    +  public void isSwaggerRunning() throws Exception {
    +    // Check the UI
    +    HttpResponse response = doRequest(new HttpGet("/geode/swagger-ui.html"), "", "");
    +    assertThat(getCode(response), is(200));
    +
    +    // Check the JSON
    +    response = doRequest(new HttpGet("/geode/v2/api-docs"), "", "");
    --- End diff --
    
    Just curious, why this is geode/v2, but all the api is still geode/v1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by Kevin Duling <kd...@pivotal.io>.
I found the following in the LICENSE file and confirmed they are no longer
in the Geode project.  I'll update my PR for GEODE-2014.

reset.css
backbone-min.js
handlebars-1.0.0.js
highlight.7.3.pack.js
jquery-1.8.0.min.js
jquery.ba-bbq.min.js
jquery.slideto.min.js
jquery.wiggle.min.js
shred.bundle.js


On Tue, Oct 25, 2016 at 11:32 AM, Anthony Baker <ab...@pivotal.io> wrote:

> Looks like the only common (previous) dependency is the core jquery
> library which is used by pulse and the geode-site.
>
> > On Oct 25, 2016, at 11:27 AM, Kevin Duling <kd...@pivotal.io> wrote:
> >
> > None of these are related to pulse, that lives
> > in geode-web, not geode-web-api.
>

Re: [GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by Anthony Baker <ab...@pivotal.io>.
Looks like the only common (previous) dependency is the core jquery library which is used by pulse and the geode-site.

> On Oct 25, 2016, at 11:27 AM, Kevin Duling <kd...@pivotal.io> wrote:
> 
> None of these are related to pulse, that lives
> in geode-web, not geode-web-api.


Re: [GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by Kevin Duling <kd...@pivotal.io>.
All of those files are the old Swagger-UI, replaced by the following gradle
entry in geode-web-api.  There is no reason we should be bundling the
swagger ui github project in to Geode.  I'm not even sure this old UI would
work with the Swagger2 API.  None of these are related to pulse, that lives
in geode-web, not geode-web-api.

compile('io.springfox:springfox-swagger-ui:' + project.'springfox.version') {
  exclude module: 'slf4j-api'
}


On Tue, Oct 25, 2016 at 11:09 AM, Anthony Baker <ab...@pivotal.io> wrote:

> I missed these changes in the original pull request:
>
>  delete mode 100644 geode-web-api/src/main/webapp/docs/css/reset.css
>  delete mode 100644 geode-web-api/src/main/webapp/docs/css/screen.css
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/explorer_icons.png
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/logo_small.png
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/pet_store_api.png
>  delete mode 100755 geode-web-api/src/main/webapp/docs/images/throbber.gif
>  delete mode 100755 geode-web-api/src/main/webapp/
> docs/images/wordnik_api.png
>  delete mode 100644 geode-web-api/src/main/webapp/docs/lib/backbone-min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/handlebars-1.0.0.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/highlight.7.3.pack.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery-1.8.0.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery.ba-bbq.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery.slideto.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/jquery.wiggle.min.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/lib/shred.bundle.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/shred/content.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/swagger-oauth.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/lib/swagger.js
>  delete mode 100644 geode-web-api/src/main/webapp/
> docs/lib/underscore-min.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/o2c.html
>  delete mode 100644 geode-web-api/src/main/webapp/docs/swagger-ui.js
>  delete mode 100644 geode-web-api/src/main/webapp/docs/swagger-ui.min.js
>
> These changes affect the contents of LICENSE (both ./LICENSE and
> geode-assembly/src/main/dist/LICENSE) as well as gradle/rat.gradle.
>
> Can you update the above files to reflect these bundling changes?  Some of
> the above libraries may also be in use by Pulse.
>
> Thanks,
> Anthony
>
>
>
>
> > On Oct 24, 2016, at 12:03 PM, asfgit <gi...@git.apache.org> wrote:
> >
> > Github user asfgit closed the pull request at:
> >
> >    https://github.com/apache/incubator-geode/pull/265
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have
> your
> > reply appear on GitHub as well. If your project does not have this
> feature
> > enabled and wishes so, or if the feature is enabled but not working,
> please
> > contact infrastructure at infrastructure@apache.org or file a JIRA
> ticket
> > with INFRA.
> > ---
>
>

Re: [GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by Anthony Baker <ab...@pivotal.io>.
I missed these changes in the original pull request:

 delete mode 100644 geode-web-api/src/main/webapp/docs/css/reset.css
 delete mode 100644 geode-web-api/src/main/webapp/docs/css/screen.css
 delete mode 100755 geode-web-api/src/main/webapp/docs/images/explorer_icons.png
 delete mode 100755 geode-web-api/src/main/webapp/docs/images/logo_small.png
 delete mode 100755 geode-web-api/src/main/webapp/docs/images/pet_store_api.png
 delete mode 100755 geode-web-api/src/main/webapp/docs/images/throbber.gif
 delete mode 100755 geode-web-api/src/main/webapp/docs/images/wordnik_api.png
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/backbone-min.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/handlebars-1.0.0.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/highlight.7.3.pack.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/jquery-1.8.0.min.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/jquery.ba-bbq.min.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/jquery.slideto.min.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/jquery.wiggle.min.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/shred.bundle.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/shred/content.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/swagger-oauth.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/swagger.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/lib/underscore-min.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/o2c.html
 delete mode 100644 geode-web-api/src/main/webapp/docs/swagger-ui.js
 delete mode 100644 geode-web-api/src/main/webapp/docs/swagger-ui.min.js

These changes affect the contents of LICENSE (both ./LICENSE and geode-assembly/src/main/dist/LICENSE) as well as gradle/rat.gradle.

Can you update the above files to reflect these bundling changes?  Some of the above libraries may also be in use by Pulse.

Thanks,
Anthony




> On Oct 24, 2016, at 12:03 PM, asfgit <gi...@git.apache.org> wrote:
> 
> Github user asfgit closed the pull request at:
> 
>    https://github.com/apache/incubator-geode/pull/265
> 
> 
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---


[GitHub] incubator-geode pull request #265: GEODE-2014: Upgrade Swagger libraries

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-geode/pull/265


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---