Add appropriate application name to GAV deployments / FISH-1015 #5071

avpinchuk · 2020-12-25T11:15:20Z

Description

This is a bug fix.

Remote artifact deployed with a temporary name app...tmp. But, e.g., for MDBs we need to specify resource adapter name activation property and we get an deployment error. This fix changes deploy name to artifactId-version.

When creating an uber jar a GAV artifacts are added with a context name, not a filename. When executes this uber jar an exception throws if context name length less than or equal 4. In other case we have an incomplete application name (context - last four chars). This fix writes GAV artifacts with their filenames and adds their contexts to the contexts.properties during uber jar creation.

Important Info

Blockers

Testing

New tests

Testing Performed

Testing Environment

Documentation

Notes for Reviewers

pzygielo · 2020-12-25T11:24:04Z

...ras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/impl/PayaraMicroImpl.java

-                                deploymentMapEntry.getKey(), "--force=true", "--loadOnly", "true");
+                        String artefactName= artefactURI.getPath().substring(artefactURI.getPath().lastIndexOf('/') + 1);
+                        // artefact name always has a valid extension
+                        String name = artefactName.substring(0, artefactName.length() - 4);


Could this be extracted to (tested) method?

Could this variable be renamed to something like nameWithExtension - and above comment removed?

pzygielo · 2020-12-25T11:26:50Z

...ras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/impl/PayaraMicroImpl.java

-                deploymentURLsMap.put(defaultContext, artefactMapEntry.getValue());
+                URL artefactURL = artefactMapEntry.getValue();
+                String artefactName = artefactURL.getPath().substring(artefactURL.getPath().lastIndexOf('/') + 1);
+


With above comment implemented - could new method be used here?

Same code also in UberJarCreator class. Maybe make sense move this to utility class? Or such class already exists?

lprimak · 2020-12-26T05:02:46Z

Can you put a problem example reproducer?
I’m not sure this solution is necessary or sideffect free.

Payara always has versioned deployment syntax wit a colon and not with a dash.
There should not be any other versioning syntax introduction

lprimak

See #5071 (comment)

avpinchuk · 2020-12-26T10:22:01Z

There some examples.

java -jar payara-micro.jar --deployFromGAV org.apache.actimemq,activemq-rar,5.16.0
RAR will be deployed with name app541699296244089487tmp or similar.
If we have an app with some MDB

@MessageDriven(activationConfig = {
...
@ActivationConfigProperty(propertyName="resourceAdapter", propertyValue="activemq-rar-5.16.0"),
...
}

and deploy it with second --deployFromGAV option we have an deployment error.

java -jar payara-micro.jar --deployFromGAV org.apache.actimemq,activemq-rar,5.16.0 --outputUberJar micro.jar
adds RAR to MICRO-INF/deploy as activemq-rar without extension (because artifactId is a deploymentURLsMap's keys and used as a filename).

java -jar micro.jar give us name activemq and context activemq-rar after deployment

PayaraMicroImpl, line 1543 and below

for (String entry : microInfEntries) {
                    File file = new File(entry);
                    String deployContext = file.getName();
                    String name = deployContext.substring(0, deployContext.length() - 4);

last line give us exception or empty name and other exception in case of filename length <= 4 and unconditionally truncate last four chars otherwise.

As opposed, java -jar payara-micro.jar --deploy app-1.0-SNAPSHOT.war
Everything is fine here: name and context is app-1.0-SNAPSHOT

avpinchuk · 2020-12-26T10:44:50Z

This does not change versioning scheme.

This patch try to make three things:

add name parameter to deploy command for GAV
writes GAV contexts to uberJar's contexts.properties when create it and deploys GAVs from uberJar with this contexts when execute
writes GAV to uberJar with actual filename not with atrifactId

lprimak · 2020-12-29T17:35:04Z

...ras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/impl/PayaraMicroImpl.java

-                                "true", "--contextroot",
-                                deploymentMapEntry.getKey(), "--force=true", "--loadOnly", "true");
+                        String artefactName = getFileName(artefactURI.getPath());
+                        String name = artefactName.substring(0, artefactName.length() - 4);


Yes, this will break in so many ways, I cannot count 👍

Thanks I understand. May be try to fix this by writing artifact with actual file name to a temporary directory instead of temp file?

There is reproducer https://github.com/avpinchuk/issue-gav
Install to your local repo and run micro (from current master)
java -jar payara-micro.jar --deployFromGAV org.apache.activemq:actevemq-rar:5.16.0 --deployFromGAV org.example:issue-gav:1.0 --additionalRepository file:///path/to/your/local/repo

lprimak · 2020-12-29T18:27:01Z

I am working on some suggested simple changes...

avpinchuk · 2020-12-29T18:31:59Z

I am working on some suggested simple changes...

Close this PR?

lprimak · 2020-12-29T18:41:08Z

No

I am working on some suggested simple changes...

Close this PR?

no

avpinchuk · 2020-12-29T18:59:24Z

I have a few more questions about Payara Micro boot time deployment. Where can they be discussed?

…isting java File() class

lprimak · 2020-12-30T16:10:07Z

I have a few more questions about Payara Micro boot time deployment. Where can they be discussed?

there is a discussion forum on GitHub for Payara.

Also, please take a look at the PR I made for your PR

avpinchuk · 2020-12-30T17:20:24Z

Also, please take a look at the PR I made for your PR

I have some questions. Take a look at code comments in your PR, please.

refactored primitive obsession when getting file name from path to ex…

lprimak · 2020-12-30T19:33:23Z

jenkins test

avpinchuk · 2020-12-30T19:35:23Z

No. Need fix.

lprimak · 2020-12-30T19:36:40Z

No. Need fix.

ok, just let us know when it's done

avpinchuk · 2020-12-30T19:39:29Z

Yes. Approx 1 hour

avpinchuk · 2020-12-30T20:09:00Z

I finished. Please take a look at my last commit. Thanks!

lprimak · 2020-12-30T21:02:10Z

jenkins test

Pandrex247

Just a little something I spotted.

Pandrex247 · 2021-01-07T12:39:57Z

...ras/payara-micro/payara-micro-core/src/main/java/fish/payara/micro/impl/PayaraMicroImpl.java

+        }
+    }
+
+    private static boolean hasJavaArchiveExtension(String filePath) {


I'm probably being blind, but is there a reason this is being made static?

Because it has no access to fields. Good enough reason for me :)

Fix GAV deployment

509f866

pzygielo reviewed Dec 25, 2020

View reviewed changes

lprimak self-requested a review December 26, 2020 05:03

lprimak suggested changes Dec 26, 2020

View reviewed changes

avpinchuk requested a review from lprimak December 26, 2020 10:25

avpinchuk added 2 commits December 26, 2020 14:44

Add utility method for getting filename

3f43830

Fix GAV write order

d18fc90

lprimak reviewed Dec 29, 2020

View reviewed changes

lprimak self-assigned this Dec 29, 2020

lprimak added PR: CLA CLA submitted on PR by the contributor Status: Accepted Confirmed defect or accepted improvement to implement, issue has been escalated to Platform Dev labels Dec 29, 2020

lprimak changed the title ~~Fix GAV deployment~~ Add appropriate application name to GAV deployments Dec 29, 2020

lprimak added 2 commits December 29, 2020 14:51

refactored primitive obsession when getting file name from path to ex…

1451ebd

…isting java File() class

factore out removeJavaArchiveExtension()

1d9a532

Merge pull request #1 from flowlogix/gav-branch

6501fee

refactored primitive obsession when getting file name from path to ex…

lprimak approved these changes Dec 30, 2020

View reviewed changes

lprimak added the PR: DO NOT MERGE Don't merge PR until further notice label Dec 30, 2020

Fix IllegalArgumentException for remote maven artifacts

361e8ec

avpinchuk requested a review from lprimak December 30, 2020 20:09

lprimak approved these changes Dec 30, 2020

View reviewed changes

lprimak removed the PR: DO NOT MERGE Don't merge PR until further notice label Dec 30, 2020

lprimak requested review from Cousjava and Pandrex247 January 5, 2021 17:48

Pandrex247 reviewed Jan 7, 2021

View reviewed changes

AlanRoth added Type: Community Contribution and removed PR: CLA CLA submitted on PR by the contributor labels Jan 18, 2021

AlanRoth changed the title ~~Add appropriate application name to GAV deployments~~ Add appropriate application name to GAV deployments / FISH-1015 Jan 18, 2021

AlanRoth unassigned lprimak Jan 18, 2021

AlanRoth added PR: CLA CLA submitted on PR by the contributor and removed Status: Accepted Confirmed defect or accepted improvement to implement, issue has been escalated to Platform Dev labels Jan 18, 2021

lprimak merged commit 2595404 into payara:master Jan 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add appropriate application name to GAV deployments / FISH-1015 #5071

Add appropriate application name to GAV deployments / FISH-1015 #5071

avpinchuk commented Dec 25, 2020

pzygielo Dec 25, 2020

pzygielo Dec 25, 2020

avpinchuk Dec 25, 2020

lprimak commented Dec 26, 2020 •

edited

Loading

lprimak left a comment •

edited

Loading

avpinchuk commented Dec 26, 2020

avpinchuk commented Dec 26, 2020 •

edited

Loading

lprimak Dec 29, 2020

avpinchuk Dec 29, 2020 •

edited

Loading

lprimak commented Dec 29, 2020

avpinchuk commented Dec 29, 2020

lprimak commented Dec 29, 2020

avpinchuk commented Dec 29, 2020

lprimak commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

lprimak commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

lprimak commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

lprimak commented Dec 30, 2020

Pandrex247 left a comment

Pandrex247 Jan 7, 2021

lprimak Jan 7, 2021

Add appropriate application name to GAV deployments / FISH-1015 #5071

Add appropriate application name to GAV deployments / FISH-1015 #5071

Conversation

avpinchuk commented Dec 25, 2020

Description

Important Info

Blockers

Testing

New tests

Testing Performed

Testing Environment

Documentation

Notes for Reviewers

pzygielo Dec 25, 2020

Choose a reason for hiding this comment

pzygielo Dec 25, 2020

Choose a reason for hiding this comment

avpinchuk Dec 25, 2020

Choose a reason for hiding this comment

lprimak commented Dec 26, 2020 • edited Loading

lprimak left a comment • edited Loading

Choose a reason for hiding this comment

avpinchuk commented Dec 26, 2020

avpinchuk commented Dec 26, 2020 • edited Loading

lprimak Dec 29, 2020

Choose a reason for hiding this comment

avpinchuk Dec 29, 2020 • edited Loading

Choose a reason for hiding this comment

lprimak commented Dec 29, 2020

avpinchuk commented Dec 29, 2020

lprimak commented Dec 29, 2020

avpinchuk commented Dec 29, 2020

lprimak commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

lprimak commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

lprimak commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

avpinchuk commented Dec 30, 2020

lprimak commented Dec 30, 2020

Pandrex247 left a comment

Choose a reason for hiding this comment

Pandrex247 Jan 7, 2021

Choose a reason for hiding this comment

lprimak Jan 7, 2021

Choose a reason for hiding this comment

lprimak commented Dec 26, 2020 •

edited

Loading

lprimak left a comment •

edited

Loading

avpinchuk commented Dec 26, 2020 •

edited

Loading

avpinchuk Dec 29, 2020 •

edited

Loading