Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FISH-886 Deploy GAV from local repository + some code cleanup #5035

Merged
merged 2 commits into from
Dec 18, 2020
Merged

FISH-886 Deploy GAV from local repository + some code cleanup #5035

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36603b5
Deploy GAV from local repository
avpinchuk Dec 7, 2020
d36259c
Change copyright year
avpinchuk Dec 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 37 additions & 24 deletions nucleus/deployment/common/src/main/java/fish/payara/deployment/util/GAVConvertor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
*
* Copyright (c) 2016-2018 Payara Foundation and/or its affiliates. All rights reserved.
* Copyright (c) 2016-2020 Payara Foundation and/or its affiliates. All rights reserved.
*
* The contents of this file are subject to the terms of either the GNU
* General Public License Version 2 only ("GPL") or the Common Development
Expand Down Expand Up @@ -44,7 +44,10 @@
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.ProtocolException;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.AbstractMap;
import java.util.Base64;
import java.util.Collection;
Expand Down Expand Up @@ -77,7 +80,7 @@ public class GAVConvertor {
*/
public Map.Entry<String, URL> getArtefactMapEntry(String GAV, List<URL> repositoryURLs) throws MalformedURLException {
final Map<String, String> GAVMap = splitGAV(GAV);
Map.Entry<String, URL> artefactMapEntry = null;
Map.Entry<String, URL> artefactMapEntry;

final String relativeURLString = constructRelativeURLString(GAVMap);
final URL artefactURL = findArtefactURL(repositoryURLs, relativeURLString);
Expand All @@ -97,7 +100,7 @@ public Map.Entry<String, URL> getArtefactMapEntry(String GAV, List<URL> reposito
* the provided GAV
*/
public Map.Entry<String, URL> getArtefactMapEntry(String GAV, Collection<String> repositoryURLs) throws MalformedURLException {
List<URL> repoURLs = new LinkedList<URL>();
List<URL> repoURLs = new LinkedList<>();
for (String url: repositoryURLs) {
String convertedURL = TranslatedConfigView.expandValue(url);
if (!convertedURL.endsWith("/")) {
Expand All @@ -117,7 +120,7 @@ public Map.Entry<String, URL> getArtefactMapEntry(String GAV, Collection<String>
* the provided GAV as Strings
*/
private Map<String, String> splitGAV(String GAV) throws MalformedURLException {
final String[] splitGAV = GAV.split(",|:");
final String[] splitGAV = GAV.split("[,:]");
Copy link
Contributor

@svendiedrichsen svendiedrichsen Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't a valid GAV only split by :? Doesn't it suggest when you find a comma that this is really a list of GAV? And if you accept lists here you should really focus on splitting by comma first to have a chance to determine optional fields like packaging and classifier.

Copy link
Contributor Author

@avpinchuk avpinchuk Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to documentation https://docs.payara.fish/community/docs/5.2020.7/documentation/payara-micro/deploying/deploy-cmd-line.html#deploying-applications-from-a-maven-repository for each GAV needs its own --deployFromGAV option. Or I misunderstood the documentation. Here I just replaced logical operator with a simple class

Copy link
Contributor

@svendiedrichsen svendiedrichsen Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is true for the Payara doc, but at least uncommon to use a comma nonetheless. This needs to be decided by someone from Payara.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking back at the commit history around this, comma was the original delimiter as that's the "default" delimiter for Payara CLI (though we do use different delimiters all over the place), and we added in support for using a colon after to be more in line with the usual Maven notation.

So fish.payara,guppy,1.0 and fish.payara:guppy:1.0 are both valid, and if you want to deploy more than one artefact you have to specify the option multiple times.

final Map<String, String> GAVMap = new HashMap<>();
try {
GAVMap.put("groupId", splitGAV[0].replace('.', '/'));
Copy link
Contributor

@svendiedrichsen svendiedrichsen Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maven coordinates might also include packaging and classifier. If the packaging coordinate exists you don't have to iterate over the whole list of possible file extensions. You could simply use this. If the classifier coordinate exists you won't be able to find the artefact without it in the URL.
You can find some examples here: https://developpaper.com/coordinates-of-maven-basic-course/

Copy link
Contributor Author

@avpinchuk avpinchuk Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be implemented if we stick to some predefined format. In community doc defined artifactId, groupId and version only. Link in next answer

Copy link
Contributor

@svendiedrichsen svendiedrichsen Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, its true according to the Payara doc but shouldn't there be a possibility to use packaging and qualifier? You could imagine having a build of an deplyoable artefact which builts a jar and a war of the same name. According the current code it would only be possible to deploy the jar. As this comes first in the list. The same goes for the qualifier by which you have a built which produces two jars of the same name but one has i.e. the qualifier UBER to mark the uber-jar containing all the dependencies inside. You couldn't distinguish them without using a qualifier.
You could default those two GAV components to jar and empty and would not have to iterate over the list of file suffixes. But maybe I'm just over-engineering this thing for this usecase.
This needs to be decided by someone from Payara.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not allowing a user to specify packaging and qualifiers is simply a limitation / missing feature of the implementation at the moment - I'd say it's out of scope for this PR though. The use case you described makes sense, I just wouldn't bundle it into this PR to prevent scope creep, particularly since it isn't a trivial 1 line change.

Expand All @@ -134,17 +137,15 @@ private Map<String, String> splitGAV(String GAV) throws MalformedURLException {

/**
* Constructs the relative URL of the provided GAV as a String.
* @param GAV A map containing the target artefact's groupId, artifactId,
* @param GAVMap A map containing the target artefact's groupId, artifactId,
* and version number.
* @return A String representing the relative URL of the provided GAV.
* @throws MalformedURLException
*/
private String constructRelativeURLString(Map<String, String> GAVMap) throws MalformedURLException {
private String constructRelativeURLString(Map<String, String> GAVMap) {
final String artefactFileName = GAVMap.get("artefactId") + "-" + GAVMap.get("versionNumber");
final String relativeURLString = GAVMap.get("groupId") + "/" + GAVMap.get("artefactId") + "/"
+ GAVMap.get("versionNumber") + "/" + artefactFileName;

return relativeURLString;

return GAVMap.get("groupId") + "/" + GAVMap.get("artefactId") + "/"
+ GAVMap.get("versionNumber") + "/" + artefactFileName;
}

/**
Expand All @@ -155,7 +156,8 @@ private String constructRelativeURLString(Map<String, String> GAVMap) throws Mal
* @param relativeURLString A String representation of the relative
* artefact URL.
* @return A valid URL to download the target artefact from.
* @throws IOException
* @throws MalformedURLException Thrown if an artefact cannot be found for
* the provided GAV
*/
private URL findArtefactURL(List<URL> repositoryURLs, String relativeURLString) throws MalformedURLException {
final String[] archiveTypes = new String[]{".jar", ".war", ".ear", ".rar"};
Expand All @@ -171,18 +173,27 @@ private URL findArtefactURL(List<URL> repositoryURLs, String relativeURLString)
try {
artefactURL = new URL(repositoryURL, relativeURLString + archiveType);

HttpURLConnection httpConnection = (HttpURLConnection) artefactURL.openConnection();

String auth = artefactURL.getUserInfo();
if (auth != null) {
String encodedAuth = Base64.getEncoder().encodeToString(auth.getBytes());
httpConnection.setRequestProperty("Authorization", "Basic " + encodedAuth);
if ("file".equalsIgnoreCase(artefactURL.getProtocol())) {
if (Files.exists(Paths.get(artefactURL.toURI()))) {
validURLFound = true;
}
} else {
HttpURLConnection httpConnection = (HttpURLConnection) artefactURL.openConnection();
Copy link
Contributor

@svendiedrichsen svendiedrichsen Dec 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes some assumption about the type of connection but it might be a JarURLConnection. Like this one: jar:http://www.foo.com/bar/baz.jar.
You might want a separate if-block to handle these kind of archive URLs.
But if it is all about local URLs this is unnessessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only needed a local repository but jar scheme support also possible

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be decided by someone from Payara.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say that's outside the scope of this PR.
I'm not immediately sure of the need to handle them either tbh - I think that functionality is covered by the standard --deploy option unless you're talking about using a Jar Url as a "maven" repository in its own right, which sounds mega-niche!


String auth = artefactURL.getUserInfo();
if (auth != null) {
String encodedAuth = Base64.getEncoder().encodeToString(auth.getBytes());
httpConnection.setRequestProperty("Authorization", "Basic " + encodedAuth);
}

httpConnection.setRequestMethod("HEAD");

if (httpConnection.getResponseCode() == HttpURLConnection.HTTP_OK) {
validURLFound = true;
}
}

httpConnection.setRequestMethod("HEAD");

if (httpConnection.getResponseCode() == HttpURLConnection.HTTP_OK) {
validURLFound = true;
if (validURLFound) {
break;
} else {
logger.log(Level.FINE, "Artefact not found at URL: {0}", artefactURL.toString());
Expand All @@ -191,19 +202,21 @@ private URL findArtefactURL(List<URL> repositoryURLs, String relativeURLString)
String[] errorParameters = new String[]{repositoryURL.toString(), relativeURLString, archiveType};
logger.log(Level.WARNING, "Error creating URL from repository URL, {0}, relative URL, {1}, and archive"
+ " type, {2}", errorParameters);
} catch (URISyntaxException ex) {
logger.log(Level.WARNING, "Error creating URI from artefact URL, {0}", artefactURL.toString());
} catch (ProtocolException ex) {
logger.log(Level.WARNING,"Error setting request method to \"HEAD\"");
} catch (IOException ex) {
logger.log(Level.WARNING, "Error getting HTTP connection response code");
}
}

if (validURLFound == true) {
if (validURLFound) {
break;
}
}

if (validURLFound == false) {
if (!validURLFound) {
throw new MalformedURLException("No artefact can be found for relative URL: "+ relativeURLString);
}

Expand Down