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.

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

[PAYARA-3662] fixing glassfish-web.xml contextroot overwrite #3831

Merged
merged 5 commits into from Mar 19, 2019

Conversation

Projects
None yet
5 participants
@cubastanley
Copy link
Contributor

commented Mar 13, 2019

Unfortunately git has tracked the changes in formatting that ended up coming into existence, the only physical changes I've made to the code are I the deployAll() method from line 1530 onwards

@@ -14,7 +14,6 @@
* limitations under the License.

This comment has been minimized.

Copy link
@cubastanley

cubastanley Mar 13, 2019

Author Contributor

Changes to this file can be ignored

@rdebusscher
Copy link
Contributor

left a comment

@cubastanley With reproducer, context root from glassfish-web.xml is still ignored.

if (war.getName().startsWith("ROOT.")) {
deployer.deploy(war, "--availabilityenabled=true", "--force=true", "--contextroot=/", "--loadOnly", "true");
} else {
deployer.deploy(war, "--availabilityenabled=true", "--force=true", "--loadOnly", "true", "--contextroot", deployContext);
if(deployContext == null) {

This comment has been minimized.

Copy link
@rdebusscher

rdebusscher Mar 13, 2019

Contributor

deployContext is never null since it is set earlier from the war name.

This comment has been minimized.

Copy link
@cubastanley

cubastanley Mar 13, 2019

Author Contributor

@smillidge regarding this, what's the purpose of the deployContext variable? It seems in these cases it would be more beneficial to just use contextRoot directly?

@rdebusscher

This comment has been minimized.

Copy link
Contributor

commented Mar 13, 2019

Difficult to find out what exactly changed due to the many code reformats

@Pandrex247
Copy link
Member

left a comment

Formatting quibbles

@@ -1587,7 +1595,11 @@ private void deployAll() throws GlassFishException {
if (entry.getName().startsWith("ROOT.")) {
deployer.deploy(entry, "--availabilityenabled=true", "--force=true", "--contextroot=/", "--loadOnly", "true");
} else {
deployer.deploy(entry, "--availabilityenabled=true", "--force=true", "--loadOnly", "true", "--contextroot",deployContext);
if(deployContext == null) {

This comment has been minimized.

Copy link
@Pandrex247

Pandrex247 Mar 13, 2019

Member

Should be a space between if and (

if (war.getName().startsWith("ROOT.")) {
deployer.deploy(war, "--availabilityenabled=true", "--force=true", "--contextroot=/", "--loadOnly", "true");
} else {
deployer.deploy(war, "--availabilityenabled=true", "--force=true", "--loadOnly", "true", "--contextroot", deployContext);
if(deployContext == null) {

This comment has been minimized.

Copy link
@Pandrex247

Pandrex247 Mar 13, 2019

Member

Should be a space between if and (

@mulderbaba mulderbaba added this to the 5.192 milestone Mar 13, 2019

@cubastanley cubastanley requested a review from Pandrex247 Mar 14, 2019

@rdebusscher
Copy link
Contributor

left a comment

Only a small improvement for me required.

for (File entry : deploymentDirEntries) {

boolean hasDefinedContextRoot = (contextRoot != null && contextRoot.isEmpty() == false);

This comment has been minimized.

Copy link
@rdebusscher

rdebusscher Mar 14, 2019

Contributor

contextRoot.isEmpty() == false -> !contextRoot.isEmpty()

if (war.exists() && war.canRead()) {
for (File war : deployments) {

boolean hasDefinedContextRoot = (contextRoot != null && contextRoot.isEmpty() == false);

This comment has been minimized.

Copy link
@rdebusscher

rdebusscher Mar 14, 2019

Contributor

contextRoot.isEmpty() == false -> !contextRoot.isEmpty()

@cubastanley cubastanley requested a review from rdebusscher Mar 14, 2019

@Pandrex247 Pandrex247 changed the title Payara-3662 fixing glassfish-web.xml contextroot overwrite [PAYARA-3662] fixing glassfish-web.xml contextroot overwrite Mar 14, 2019

@cubastanley

This comment has been minimized.

Copy link
Contributor Author

commented Mar 18, 2019

Jenkins test please

@Pandrex247 Pandrex247 merged commit 6c638c6 into payara:master Mar 19, 2019

59 checks passed

Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/javaee-api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.