search

jboss-fuse / fuse-apicurito-generator

Fuse Apicurito Generator
5 stars 14 forks source link

issue12 - making uuid more standard across Ids in template #18

Closed bfitzpat closed 5 years ago

bfitzpat commented 5 years ago

-- adding new test

Signed-off-by: Brian Fitzpatrick bfitzpat@redhat.com

bfitzpat commented 5 years ago

Looks like CI is having some issues. Anybody know what this is? @apupier @EricWittmann @lhein ? Error: Could not find or load main class org.apache.maven.surefire.booter.ForkedBooter

https://circleci.com/gh/jboss-fuse/fuse-apicurito-generator/26?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

apupier commented 5 years ago

CI failing with

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Error: Could not find or load main class org.apache.maven.surefire.booter.ForkedBooter

Results :

Tests run: 0, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  05:29 min
[INFO] Finished at: 2018-11-02T14:20:49Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.12.4:test (default-test) on project fuse-apicurito-generator: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:2.12.4:test failed: The forked VM terminated without saying properly goodbye. VM crash or System.exit called ? -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.

which seems to be a know issue of Maven Surefire with SpringBoot and latest Ubuntu/Debian version https://issues.apache.org/jira/browse/SUREFIRE-1588

bfitzpat commented 5 years ago

which seems to be a know issue of Maven Surefire with SpringBoot and latest Ubuntu/Debian version https://issues.apache.org/jira/browse/SUREFIRE-1588

So @apupier do we need to override the maven-surefire-plugin version somewhere in the pom to be 3.0.0-M1? That seems dangerous to rely on an unreleased version. And I can't find a reference to the surefire plug-in explicitly in the pom (https://github.com/jboss-fuse/fuse-apicurito-generator/blob/master/pom.xml) so it must be coming from somewhere else in the dependency tree... Any ideas?

EricWittmann commented 5 years ago

Sorry I'm late to the party on this. Taking a quick look now to see if I can help at all...

bfitzpat commented 5 years ago

Sorry I'm late to the party on this. Taking a quick look now to see if I can help at all...

Thanks @EricWittmann !

EricWittmann commented 5 years ago

So it turns out I can't build the project locally because there are fuse artifacts that cannot be located. I don't know which maven repository they might be in, and I haven't tried doing the build with the VPN enabled. I'm assuming that one or more of the maven repos configured in pom.xml are RH internal repos. Anyone know if that's the case?

bfitzpat commented 5 years ago

I'm able to build not being on VPN and have no settings.xml in my .m2, but I'm not sure I want to blow away my maven cache to confirm that. :) It builds here, which doesn't help you. @apupier may have a different situation

apupier commented 5 years ago

@apupier may have a different situation

I reported few days ago that the master branch is not building https://github.com/jboss-fuse/fuse-apicurito-generator/issues/16

EricWittmann commented 5 years ago

Yep - that's exactly the error I get when I try to build.

EricWittmann commented 5 years ago

I haven't tried building this project in awhile - and it looks like @chirino pushed a change to the fuse version that is perhaps unavailable in a public maven repo?

apupier commented 5 years ago

the versions used are staging versions so they are disappearing after a certain amount of time/build.

(when there is only XX-fuse-XXX it is a staging version)

EricWittmann commented 5 years ago

In that case should we have a branch? Seems like master should be stable and buildable.

bfitzpat commented 5 years ago

Rebased on the build change in the master branch and also updated the camel version to match what's used in the pom version we updated to.

bfitzpat commented 5 years ago

@apupier do you mind taking another look and approving this one since we're back to a green build again?

EricWittmann commented 5 years ago

GREEEEEEN!

bfitzpat commented 5 years ago

Thanks for the help @EricWittmann !! That build issue was driving me crazy. :)

bfitzpat commented 5 years ago

@apupier - Let me try and clarify a few things in response to your feedback.

First, about the git commit messages. I'm finding that VSCode does not provide great git commit integration for updating previous messages. Committing via VSCode is not as fully-functioned as egit support in Eclipse. That said, git commit labels are less important to me than the actual code, so I'll focus the rest of my response on that. If I can find a simple way of updating the git messages to be more correct, I will try to do so.

Second, onto the code itself.

There were two issues. One had to do with the way I was managing IDs in the template with https://github.com/jboss-fuse/fuse-apicurito-generator/pull/10. Simply put, I missed a few. The revised template in this PR corrects that problem.

If you look at the project I attached to the issue here - https://github.com/jboss-fuse/fuse-apicurito-generator/issues/12#issuecomment-435495969 - it includes a context file generated with the fixes from @tsedmik 's swagger file https://github.com/tsedmik/fuse-apicurito-generator-tests/blob/master/src/test/resources/openapi-spec.json to create the project I reference there. It built and ran at the time, though it is likely out of date now with the fuse and camel versions in the pom.xml.

Thus, I addressed the issue as reported by Tomas in https://github.com/jboss-fuse/fuse-apicurito-generator/issues/12. The test is a manual one at this point on my side. Perhaps the QE team can add a test (if it doesn't already exist) to run the generated project in https://github.com/tsedmik/fuse-apicurito-generator-tests, but I think that goes beyond the scope of what I am capable of doing in the fuse-apicurito-generator project tests at the moment.

As to the UUID method of generating unique IDs, it's the same approach as I used in the Eclipse Camel model code here - https://github.com/jbosstools/jbosstools-fuse/blob/7e6c11f3a75f0d5d436ae75057bc38a56f09871c/core/plugins/org.fusesource.ide.camel.model.service.core/src/org/fusesource/ide/camel/model/service/core/model/AbstractRestCamelModelElement.java#L58 - so I'm not sure I understand the issue you have at this point.

Honestly I'm not sure I know of a way to auto-generate meaningful IDs in any logical way -- and even if I could, that goes beyond the scope of what this issue was attempting to address. Perhaps @lhein may have some ideas here.

The second issue was fixed by @EricWittmann 's fix for https://github.com/jboss-fuse/fuse-apicurito-generator/pull/20. Now that that is addressed, we have a stable, green build again for the moment and can continue marching forward.

Thanks for the feedback.

bfitzpat commented 5 years ago

Fixed the commit messages. Fixed the assert issue.

apupier commented 5 years ago

First, about the git commit messages. I'm finding that VSCode does not provide great git commit integration for updating previous messages. Committing via VSCode is not as fully-functioned as egit support in Eclipse.

I can only recommend to report issues/enhancement requests to VS Code and switch to Eclipse for committing until they have addressed them.

That said, git commit labels are less important to me than the actual code,

commit labels are very important when debugging some codes to understand why code modifications occurred. I was pointing not only commit message but commit by itself. There is one commit which is introducing non-compiling code. It also forbids to use git bisect to search when a bug was introduced in a repository.

so I'll focus the rest of my response on that. If I can find a simple way of updating the git messages to be more correct, I will try to do so.

Fixed the commit messages.

a new merge commit has been introduced and the reference to the issue in the commit message is still wrong. see https://github.com/apupier/fuse-apicurito-generator/tree/bfitzpat-issue12-adjusting-id-generation for a cleaned history You can force push this version on your branch.

apupier commented 5 years ago

it includes a context file generated with the fixes from @tsedmik 's swagger file https://github.com/tsedmik/fuse-apicurito-generator-tests/blob/master/src/test/resources/openapi-spec.json to create the project I reference there. It built and ran at the time, though it is likely out of date now with the fuse and camel versions in the pom.xml.

I don't understand why it is out of date. The json file is related to the Camel/Fuse version? it is the input of the transformation we it can be used with all versions of Camel/Fuse, isn't it?

apupier commented 5 years ago

Thus, I addressed the issue as reported by Tomas in #12. The test is a manual one at this point on my side. Perhaps the QE team can add a test (if it doesn't already exist) to run the generated project in https://github.com/tsedmik/fuse-apicurito-generator-tests, but I think that goes beyond the scope of what I am capable of doing in the fuse-apicurito-generator project tests at the moment.

ok. will need to find time to test manually also or find someone else to validate the issue then because i'm not able to validate myself without this kind of test giving my limited understanding of this code project.

apupier commented 5 years ago

As to the UUID method of generating unique IDs, it's the same approach as I used in the Eclipse Camel model code here - https://github.com/jbosstools/jbosstools-fuse/blob/7e6c11f3a75f0d5d436ae75057bc38a56f09871c/core/plugins/org.fusesource.ide.camel.model.service.core/src/org/fusesource/ide/camel/model/service/core/model/AbstractRestCamelModelElement.java#L58 - so I'm not sure I understand the issue you have at this point.

Honestly I'm not sure I know of a way to auto-generate meaningful IDs in any logical way -- and even if I could, that goes beyond the scope of what this issue was attempting to address. Perhaps @lhein may have some ideas here.

A part of the elements of the generated Camel file is in fact always the same if I understand it well. For these elements, using a meaningful naming instead of a random UUID seems better. For instance the route2 and elements inside it.

lhein commented 5 years ago

For how to generate endpoint id's manually see https://github.com/apache/camel/blob/master/camel-core/src/main/java/org/apache/camel/util/EndpointHelper.java#L425 or create your own logic similar.

bfitzpat commented 5 years ago

I don't understand why it is out of date. The json file is related to the Camel/Fuse version? it is the input of the transformation we it can be used with all versions of Camel/Fuse, isn't it?

Yes. Camel/Fuse version in the pom used in the zipped project. The fuse version is located here in the pom https://github.com/jboss-fuse/fuse-apicurito-generator/blob/2338747b8d760d069c6161bdaa3f25ba9c7385c8/src/main/resources/camel-project-template/pom.xml#L27 - that should be the only thing different.

bfitzpat commented 5 years ago

will need to find time to test manually also or find someone else to validate the issue then because i'm not able to validate myself without this kind of test giving my limited understanding of this code project.

I will write up a series of steps for a manual test.

bfitzpat commented 5 years ago

You can force push this version on your branch.

Thanks for the cleaned history and help getting that resolved.

apupier commented 5 years ago

test failing:

Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.093 sec <<< FAILURE!
testForRestDSLOutputFromSimpleOpenAPIFile(com.redhat.fuse.apicurio.jaxrs.tests.GenerateFuseProjectResourceTest)  Time elapsed: 0.002 sec  <<< FAILURE!
java.lang.AssertionError
    at org.junit.Assert.fail(Assert.java:86)
    at org.junit.Assert.assertTrue(Assert.java:41)
    at org.junit.Assert.assertTrue(Assert.java:52)
    at com.redhat.fuse.apicurio.jaxrs.tests.GenerateFuseProjectResourceTest.getRestDSLXMLFromSwagger(GenerateFuseProjectResourceTest.java:52)
    at com.redhat.fuse.apicurio.jaxrs.tests.GenerateFuseProjectResourceTest.testForRestDSLOutputFromSimpleOpenAPIFile(GenerateFuseProjectResourceTest.java:101)

did I mess the clean of the history?

EDIT: effectively, I missed a commit. fix in progress... fixed!

bfitzpat commented 5 years ago

did I mess the clean of the history? EDIT: effectively, I missed a commit. fix in progress... fixed!

Thanks Aurelien - I was just starting to dive back into this!

bfitzpat commented 5 years ago

Looking into the Id question and will get back to you by EOD.

bfitzpat commented 5 years ago

I have updated the PR locally (and will push my changes momentarily) to make the generated camel context file look as follows. I believe this is more in line with what you were looking for @apupier 

<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
       xsi:schemaLocation="
       http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
       http://camel.apache.org/schema/spring       http://camel.apache.org/schema/spring/camel-spring.xsd">

    <camelContext id="context-2e648a59-9971-4ca0-92c0-7f53ea291b01" xmlns="http://camel.apache.org/schema/spring">

        <onException>
            <exception>java.lang.Exception</exception>
            <handled><constant>true</constant></handled>
            <setHeader headerName="Exchange.HTTP_RESPONSE_CODE">
                <constant>500</constant>
            </setHeader>
            <setBody>
                <simple>${exception.message}</simple>
            </setBody>
        </onException>

        <restConfiguration component="servlet" apiContextPath="/openapi.json"/>

        <rest id="rest-for-openapi-document" path="/" enableCORS="true">
            <get id="openapi.json" produces="application/json" uri="openapi.json">
                <description>Gets the OpenAPI document for this service</description>
                <route id="route-for-openapi-document">
                    <setHeader id="setHeader-for-openapi-document" headerName="Exchange.CONTENT_TYPE">
                        <constant>application/vnd.oai.openapi+json</constant>
                    </setHeader>
                    <setBody id="setBody-for-openapi-document">
                        <simple>resource:classpath:openapi.json</simple>
                    </setBody>
                </route>
            </get>
        </rest>

        <rest id="rest-7131cd90-475e-45ca-944c-8050b158d050" path="" bindingMode="json" enableCORS="true">
            <get uri="/test">
                <to uri="direct:rest1"/>
            </get>
        </rest>

        <route id="route-f4af4516-709d-4135-a6b0-0112317e6b2e">
            <from id="from-1e0a8c8c-5092-4fe6-b6a5-d3875b66a57e" uri="direct:rest1"/>
            <to id="to-9781ce2f-7c7b-459d-afc5-3cc95b234da9" uri="direct:501"/>
        </route>

        <route id="route-for-unimplemented-operations">
            <from id="from-for-unimplemented-operations-route" uri="direct:501"/>
            <log id="log-for-unimplemented-operations-route" message="API operation not yet implemented: ${headers.CamelHttpMethod} ${headers.CamelHttpPath}"/>
            <setHeader id="setHeader-for-unimplemented-operations-route" headerName="Exchange.HTTP_RESPONSE_CODE">
                <constant>501</constant>
            </setHeader>
            <setBody id="setBody-for-unimplemented-operations-route">
                <simple>API operation not implemented: ${headers.CamelHttpMethod} ${headers.CamelHttpPath}</simple>
            </setBody>
        </route>

    </camelContext>
</beans>
bfitzpat commented 5 years ago

This zip includes the generated project with the additional repository details necessary to build and run. camel-project-test-swagger.zip

bfitzpat commented 5 years ago

To test manually:

  1. Clone https://github.com/apicurio/apicurito locally
  2. Make sure node and yarn are installed
  3. Run mvn clean package in the fuse-apicurito-generator directory
  4. Run java -jar target/fuse-apicurito-generator-*-SNAPSHOT.jar in the fuse-apicurito-generator directory
  5. In a separate console, go back to the apicurito/ui directory and run yarn install; yarn start
  6. Go to a web browser and hit http://localhost:4200/
  7. Instead of creating a new API, import the OpenAPI file located here - https://github.com/tsedmik/fuse-apicurito-generator-tests/blob/master/src/test/resources/openapi-spec.json (copy locally, browse to it, and import it)
  8. Export the project (it is generated as a zip archive)
  9. Unzip the generated project locally
  10. Update the repository list in the project pom to include the jboss-ea repository as is in the zip attached in an earlier comment
  11. build and run the project locally (make sure to shut down the fuse-apicurito-generator jar first, as it uses the same port as the local springboot instance
apupier commented 5 years ago

test seems to be covered by side test project https://github.com/jboss-fuse/fuse-apicurito-generator/issues/15#issuecomment-438276027