Skip to content
This repository has been archived by the owner on May 10, 2022. It is now read-only.

Validate camel version when input during camel-project creation #15

Open
bfitzpat opened this issue Sep 4, 2018 · 13 comments
Open

Validate camel version when input during camel-project creation #15

bfitzpat opened this issue Sep 4, 2018 · 13 comments

Comments

@bfitzpat
Copy link
Contributor

bfitzpat commented Sep 4, 2018

We prompt the user for the Camel version to use for the project:

? Your Camel version (2.18.1)

But we don't validate it. So if the user enters an invalid number (9.9.9) it will just happily put that in the pom and it won't work correctly. As a result, we need to validate this value.

@bfitzpat
Copy link
Contributor Author

bfitzpat commented Sep 4, 2018

At @lhein 's suggestion, I tried this approach:

const download = require("mvn-artifact-download").default;
...
utils.validateCamelVersion = function(value) {
    var artifactString = 'org.apache.camel:camel-spring:' + value;
    var repositoryUrl = 'https://maven.repository.redhat.com/ga';
    download(artifactString, null, repositoryUrl).
        then(function() {
            return value;
        },
        function() {
            return chalk.red('Camel version \'' + value + '\' is not available in public repositories.');
        }
    );
}

It always passes and returns a jar file... but the camel-spring-VERSION.jar file is essentially a HTML page. Not exactly what I was looking for. Any suggestions? Based this loosely on https://github.com/camel-tooling/camel-lsp-client-vscode/blob/master/scripts/preinstall.js

@apupier
Copy link
Member

apupier commented Sep 6, 2018

I think that it depends on the implementation of the npm package.
I see several possibilities:

  • improve the mvn-artifact-download and then use this newer version
  • instead for returning the value directly, check for the content
  • try some other npm libraries
  • the maven repository is not returning a proper value

seems that in theory there is an error reported as indicated in this test https://github.com/laat/mvn-dl/blob/master/packages/mvn-artifact-download/test/test.js#L72 so the npm package is expecting a 404 reported by the maven repository when trying to access a non-existing artifact

@apupier
Copy link
Member

apupier commented Sep 6, 2018

I have a 404 when trying to access a non-existing version
image
strange that there isn't one in yoru case via the npm package

@apupier
Copy link
Member

apupier commented Sep 6, 2018

last note: you will need to search through all configured repositories in settings.xml of user and in the pom.xl of the generated project

@bfitzpat
Copy link
Contributor Author

bfitzpat commented Sep 6, 2018

Thanks for the feedback @apupier . A follow-up question... If we search through all possible repositories, should we go with the generated project pom.xml first and then fall back to the user's configured repositories?

@bfitzpat
Copy link
Contributor Author

bfitzpat commented Sep 6, 2018

Also @apupier , it seems that the download function is asynchronous (https://github.com/laat/mvn-dl/blob/e86525864de020582a9c06cf0dbe7d2e720a6dbe/packages/mvn-artifact-download/src/artifact-download.ts#L23) and we are calling it in a synchronous way, as it requires network time to ping the maven repo and see if the artifact exists, etc.

I have not found a great way to wrap that as a synchronous call yet.

I tried wrapping the call a little differently (as follows) and it just hangs...

    download(artifactString, null, repositoryUrl).
        then(function () {
            return value;
        }).catch(error => {
            return chalk.red('Camel version \'' + value + '\' is not available in public repositories.');
        }
    );

@apupier
Copy link
Member

apupier commented Sep 7, 2018

If we search through all possible repositories, should we go with the generated project pom.xml first and then fall back to the user's configured repositories?

I would start with default maven repositories, then project generated pom.xml then user defined.
But it feels like reinventing the wheel. Calling a resolution of artifact through Maven implementation might be better.

@bfitzpat
Copy link
Contributor Author

bfitzpat commented Sep 7, 2018

@apupier I'm not a fan of reinventing the wheel. It rolls just fine on its own. But I'm not sure what you're suggesting with "Calling a resolution of artifact through Maven implementation" -- can you expand on that?

@apupier
Copy link
Member

apupier commented Sep 7, 2018

what you're suggesting with "Calling a resolution of artifact through Maven implementation" -- can you expand on that?

Maven java implementation is providing API taking care correctly of all Maven repositories configured in the various places (including the ones that are secured)
I think that it is also possible to do it through command-line (and it will also take care of all configured Maven repositories)

@bfitzpat
Copy link
Contributor Author

bfitzpat commented Sep 7, 2018

@apupier We can probably run it at command line (or use the npm maven package to do so) - any ideas how we'd structure that call?

@apupier
Copy link
Member

apupier commented Sep 10, 2018

We can probably run it at command line (or use the npm maven package to do so) - any ideas how we'd structure that call?

there is at least https://maven.apache.org/plugins/maven-dependency-plugin/get-mojo.html. You need to check how it is working with repositories.

@bfitzpat
Copy link
Contributor Author

Due to the asynchronous wait, I'm thinking we need to do this validation not at the prompt, but in the writing stage when we start copying files. It can be the first thing we do there. It may be too late at that point, but there's a potentially long delay between trying to check for the artifact and returning a useful response.

@bfitzpat
Copy link
Contributor Author

I have added some of my early code attempts on this one to #27 for review. Maybe someone will have a better idea? Right now it definitely takes a while for this to run (see the mocha test I added) so I don't think it's a good fit for the prompt validation, but it may be a good check for when we create the project.

I'm open to ideas, but wanted to show some of my work so far on this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants