Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 340918 - deploy-name of EAR projects is ignored during deployment
Summary: deploy-name of EAR projects is ignored during deployment
Status: RESOLVED FIXED
Alias: None
Product: WTP ServerTools
Classification: WebTools
Component: jst.server (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows 7
: P3 normal with 3 votes (vote)
Target Milestone: 3.5 M6   Edit
Assignee: Elson Yuen CLA
QA Contact: Elson Yuen CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 406932 400967 404078
  Show dependency tree
 
Reported: 2011-03-24 17:34 EDT by Fred Bricon CLA
Modified: 2017-10-11 16:39 EDT (History)
6 users (show)

See Also:


Attachments
Screencast showing deploy-name was used previously by Eclipse/WTP (18.79 MB, application/x-shockwave-flash)
2013-01-02 07:00 EST, Baptiste Mathus CLA
no flags Details
a patch of api additions for servertools (6.08 KB, patch)
2013-01-17 08:38 EST, Rob Stryker CLA
no flags Details | Diff
a patch for jeetools to make use of the new api (1.35 KB, patch)
2013-01-17 08:39 EST, Rob Stryker CLA
no flags Details | Diff
An alternate diff adding a property Map (14.90 KB, patch)
2013-02-01 11:37 EST, Rob Stryker CLA
no flags Details | Diff
v1.0 Server Tools (15.67 KB, patch)
2013-02-05 16:59 EST, Elson Yuen CLA
no flags Details | Diff
JEE patch for a sample of implementation on the new API (2.03 KB, patch)
2013-02-05 17:09 EST, Elson Yuen CLA
no flags Details | Diff
v1.1 Server Tools (16.35 KB, patch)
2013-02-15 09:54 EST, Elson Yuen CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fred Bricon CLA 2011-03-24 17:34:54 EDT
Build Identifier: 20110218-0911

Some maven users need to deploy EAR projects under another name than the project's name. While that feature is not supported in m2eclipse-wtp, I checked if WTP is capable of handling a custom deploy name. 

It turns out it isn't. Changing the ear's deploy-name in .settings/org.eclipse.wst.common.component is ignored when trying to deployed to a JBoss AS instance (havent tried other servers).

It affect the JBoss AS server adapters of WTP, as well as the JBoss Tools server adapters.

I believe it could easily be fixed for the Indigo release.


Reproducible: Always

Steps to Reproduce:
1. Create an EAR 5 project (with a webapp). Let's name it ear
2. In ear/.settings/org.eclipse.wst.common.component change <wb-module deploy-name="ear"> to <wb-module deploy-name="another-name">
3. Deploy the ear project to a JBoss AS 5 instance (or 6)
4. Check the AS deployment directory : ear.ear was deployed while  another-name.ear is expected
Comment 1 Angel Vera CLA 2011-03-25 10:53:58 EDT
The component file is owned by the JEE tooling, having said that the problem sounds like a deploy problem. But very likely we are just getting the module name from the module factory. 

Carl, do you have any insights on whether this is mine or yours?
Comment 2 Rob Stryker CLA 2012-02-15 03:05:30 EST
The issue here is both, and would need to be discussed. Can't just pass it off to one team or the other.

The jeetools/common teams have an attribute in teh component.xml which has no corollary in the servertools framework. Users and adopter products are changing this attribute and expecting the change to be shown in the deployment.

Having the deployers access this would require a dependency on common and the virtual component framework.  This is probably not what servertools team wants. On the other hand, removing it from component.xml would be removing API that adopter products which depend on all wtp might use,and would be a severe change that many adopters would fight against. 

Adding API to IModule (such as getDeployedName) might be hte easiest thing. The IVirtualComponent has a getDeployedName() but hte servertools api does not. So users are putting thing in the component.xml's deployedName and expecting it to be respected by servertools.
Comment 3 Chuck Bridgham CLA 2012-08-23 13:48:30 EDT
Can we consider this for 3.4.1?  This is a problem for any server as Rob says...
And using Maven, its very common to have a deploy-name  (archive name) that is different from the project name.   This server tools API should be using deploy-name.
Comment 4 Diego Santiviago CLA 2012-12-17 08:22:46 EST
Hello,

That's my environment:

JBoss AS
Eclipse
Maven 3.x
Seam 2.2.2

I have a seam file (components.xml and components.properties) for Seam configuration:

jndiPattern=my-ear/\#{ejbName}/local

my-ear is deploy-name, and my project-name in eclipse is 'my-project-name'.

I have the same issue.
Comment 5 Fred Bricon CLA 2012-12-17 08:57:51 EST
Diego, as a workaround, you can try playing with maven profiles and resource filtering to generate a proper components.properties : 

components.properties :
jndiPattern=${ear.deploy.name}/\#{ejbName}/local

pom.xml :
<properties>
 ...
 <!-- define default EAR deploy name (i.e. for all builds outside eclipse) -->
 <ear.deploy.name>my-ear</ear.deploy.name>
</properties>
...
<build>
 ...
 <resource>
 <!-- Enable resource filtering. See http://maven.apache.org/plugins/maven-resources-plugin/examples/filter.html -->        
  <directory>src/main/resources</directory>
  <filtering>true</filtering>
 </resource>
...
</build>
<profiles>
 <profile>
   <!-- the m2e profile is automatically enabled when running in eclipse/m2e -->
   <id>m2e</id>
   <activation>
     <property>
       <name>m2e.version</name>
     </property>
   </activation>
   <properties>
     <!-- Use WTP's EAR deploy name -->
     <ear.deploy.name>my-project-name</ear.deploy.name>
   </properties>
 </profile>
</profiles>
Comment 6 Baptiste Mathus CLA 2013-01-02 07:00:45 EST
Created attachment 225134 [details]
Screencast showing deploy-name was used previously by Eclipse/WTP

Actually, I'm currently bitten by this issue or some variant of it.
But the thing is: it was working for us previously fine with Eclipse 3.5. After migrating to Eclipse 4.2, it's not anymore.

Please note that in the same time, I also upgraded M2E so I cannot be perfectly sure where the "regression" lies.

I've done a small screencast using Wink showing that it was working fine previously, and it's not now anymore. The deploy-name attribute *was* used previously (see the video).

Cheers
Comment 7 Baptiste Mathus CLA 2013-01-02 07:03:51 EST
Please note that I'm having this issue using JBoss EAP 5 (working version with Eclipse 3.5) and JBoss EAP 6 (Eclipse Juno, not working anymore).

Thanks
Comment 8 Fred Bricon CLA 2013-01-02 12:31:19 EST
@Rob I confirm the Galileo WTP + JBT 3.1.1 combo uses the deploy-name, not only as shown in the UI (as per Baptiste screencast) but also for publishing. 
So this is actually a regression from previous behaviour.
Comment 9 Rob Stryker CLA 2013-01-17 04:43:27 EST
This is a regression of https://bugs.eclipse.org/bugs/show_bug.cgi?id=202541

But it's an (somewhat) intentional regression. Having the module names be set according to what the deploy-name is causes problems in servertools. Two web projects with the same deploy name were misidentified as the same module. This was a very bad situation. 

Perhaps not enough discussion went on for the bug, but you can see the patch WAS reviewed and discussed by several people. 

At this point, I believe the only true solution is for an API addition to servertools which would allow for a deploy-name in addition to a module name.  The module name would be unique, the project name, for example, while the deploy name would be different. 

This would require all server adapters to actually respect a deploy-name api addition. This would take a bit of work. 

There are also UI implications here. What should the add/remove wizard (or the server) indicate is the module's name? The official module name? Or it's deploy name? 

I believe more discussion needs to go on here before any true decision can be made, but we are running out of time for this major release. Does anyone favor a conference call of some type?  Would servertools consider allowing such api additions? 

I think it is very fair to say that the current api is not sufficient. Servertools gets confused if modules have the same "name", so the name must be unique, but servertools has no api to allow for the deploy name to be different than the module name. So this is a clear deficiency in api for servertools.
Comment 10 Rob Stryker CLA 2013-01-17 08:36:37 EST
Elson:

I was wondering if we could push this up in priority or at least have a discussion on it. This seems a pretty bad api deficiency, and since it's a bit of a regression for jee-tools, it'd be nice to discuss.
Comment 11 Rob Stryker CLA 2013-01-17 08:38:47 EST
Created attachment 225763 [details]
a patch of api additions for servertools
Comment 12 Rob Stryker CLA 2013-01-17 08:39:15 EST
Created attachment 225764 [details]
a patch for jeetools to make use of the new api
Comment 13 Elson Yuen CLA 2013-01-23 15:26:43 EST
I wasn't involved in the discussion on the original bug that causes the regression (https://bugs.eclipse.org/bugs/show_bug.cgi?id=202541) so I may not get the complete picture on why we are introducing the intentional regression mentioned in comment #9 in the first place.  Based on the history on that bug, the problem was introduced by changing both the module ID and the module name to the component name during the module creation in J2EEDeployableFactory.

The IModule API itself has already got mechanism for distinguishing the module id from the module name.  The module id is meant to be constant.  It is the unique identifier of the module but the module name is not.  Any look up on a module should be relying on the module ID instead of the name.  Instead of introducing a third identifier API for the deploy name, have we considered to set the module ID as the component name while leaving the module name as the deploy name as before?
Comment 14 Rob Stryker CLA 2013-01-30 05:19:26 EST
> have we considered to set the module ID as the component name while leaving the module name as the deploy name as before?

I'll run some tests and see if that solves the issues.
Comment 15 Rob Stryker CLA 2013-01-30 10:12:53 EST
Elson:

The problem with simply doing as you suggest, specifically, making the ID be equal to the component name, and the NAME equal to the deploy name (if it exists, else the component name) is that the UI will be confusing. 

Take for instance the following example:
  1) Create dynamic web project Web1
  2) Create dynamic web project Web2
  3) Set the deploy-name of Web2 to the value "Web1"
  4) Right-click a server, select "Add and Remove...", and look at the dialog

You will note there are now two projects listed as "Web1". A user hoping to deploy one specific one now has no idea which one to move over to the right half of the dialog. 

Changing the "Add and Remove..." dialog to use the id instead of the name is not exactly a good solution, as the id may be a nonesense string, a number, or any number of other items. 

Clearly, to be usable, the dialog must present module names that are unique. While I'm sure servertools cannot ensure this for all modules of all types, the hope would be that different modules would display some different icon for their module type. So I won't press this issue for a web project and a (custom project module type) having an identical name.

However it's clear that two ear projects, or two web projects,  or two projects of ANY matching type, may suffer from this problem. 

It's obvious the id's cannot match. This would cause big errors in the framework. ANd we all accept that. 

However, for these UI purposes, there are severe deficiencies in suggesting that a module factory create two modules of identical type with the same UI-visible name. This will become very confusing for users. 

Now, if the addition of a "deploy-name" doesn't suit you, as it's too specific, and not all modules would have a "deploy name", I would be just as happy if we were to add a HashMap of key/value pairs, and leave it up to the server adapters and the projects to respect those key/value pairs, if you see this as a more robust solution. 

Looking forward to your thoughts :)
Comment 16 Fred Bricon CLA 2013-02-01 09:54:19 EST
Ok so setting module.name = deploy-name really seems the way to fix the deployement aspect of this issue.

Now I agree a Map added to the module could be useful to disambiguate potential duplicates in the UI, while providing future extendability on the modules.

So we'd have a module.properties.displayName = deploy-name. 

IMHO, the UI should by default show for web1 :
module.properties.displayName (would be webapp-1.0.0-SNAPSHOT.war for instance)

if web1 and web2 have the same deploy-name, we could display :
webapp-1.0.0-SNAPSHOT.war (web1) 
webapp-1.0.0-SNAPSHOT.war (web2)

either the displayName disambiguated directly during the string construction, or in the UI, by using something like 
module.properties.displayName (module.properties.projectName)
Comment 17 Chuck Bridgham CLA 2013-02-01 10:37:43 EST
+1 on this suggestion Fred.  Just build a display string base on deploy name and project name  - this should be enough, and is easiest to implement.
Comment 18 Rob Stryker CLA 2013-02-01 10:46:31 EST
In the past, servertools has expressed to me that a "deploy name" in general doesn't belong in servertools, at least not as part of IModule, as not all modules are to be deployed, or if they are deployed, have names that matter. 

Other issues is we don't really want the UI doing the concatenation itself. Servertools doesn't really know what type of module is being given to it, and an IModule only officially has the two values of name and id.  Some modules may use anything as abstract as even a number, or a nonsense string, as their id, and the id's are never suitable for display purposes ever. 

So I believe it should be up to the module factory to set a display name and a deploy name in this hashmap of properties. A module factory would know better than anyone if there'd be conflicting names from the modules within its purvue, and so it should be their responsibility. 

We really want to keep as much changes as possible out of servertools, and the majority of the responsibility should be in the consumers of the servertools api, such as jeetools. But servertools does need to provide a proper api so that this information can be received. 

I have a patch forthcoming in an hour or two. Then we can see what the proposed changes look like and the extent of them.
Comment 19 Rob Stryker CLA 2013-02-01 11:09:54 EST
> Just build a display string base on deploy name and project name 

I'd also like to note that servertools core or UI should not be doing this. That's too much responsibility for servertools for things they might not know about, and also for many module types, a backing project is optional or even nonexistant. So something that simple is not quite useful enough. 

The module factories should be able to set the display name through new API, so that the responsibility is not in the hands of servertools. 

Now I know, you're all thinking "but the getName() *IS* the display name", but that's not quite true. the getName() is a display name, that's true enough, however almost all server adapters use the getName() value as the name of the output of a publish event,  getName() + ".war". 

So again, getName() is used for too many different things, and its conflicting with each other.
Comment 20 Rob Stryker CLA 2013-02-01 11:37:23 EST
Created attachment 226461 [details]
An alternate diff adding a property Map

I'm attaching an alternate diff, where instead of providing a "deploy name", we're providing a properties map and a new interface called IModuleWithProperties.  There are also two pre-provided constants in IModuleProperties, one for deploy name, one for display name. 

Factories can use any, all, or none of these if they wish. Factory providers should be aware not to go changing anything drastically, but they can set these extra parameters at will. 

After this patch, module.getName() remains in its ambiguous state. It can be used for display or publishes if none of the override property keys are set. Some server adapters may not react to these override keys very fast, which is why we'll want to keep our getName() fairly consistant and return a value that is correct in 99% of the times. Eventually, though, these server adapters should consider that modules and their factories may be providing a different deploy name and that they might want to respect that. 

You might consider this patch to be 'overkill', in that we don't need a deploy name. The display name is good enough to work around the situation where one deploy name conflicts with another module's actual name. This may be true, but I still feel the deploy-name key is a valuable addition. 

In this attached patch, the deploy-name value is without suffix. I'd consider modifying it to have the suffix on it, as I always thought it was kinda strange that the servers had to guess the suffix of a module based on its module type. It'd make more sense if the module factory, which knows its own type, attached its own suffix. 

But, to minimize differences in behavior and to open discussion, I didn't assume the deploy-name has a suffix for this patch.
Comment 21 Elson Yuen CLA 2013-02-05 16:57:19 EST
Rob, thanks for submitting the patches.  I take a look at the existing JEE code.  It looks like it has tight coupling on the assumption that the component name is the module name, e.g. in the module artifact adapter.  I agree with Rob that we should introduce something new for the deploy name (just to make sure we don't drastically break the existing tools).

After reviewing the patches, I like the version that introduce the property map more since it will provide some flexibility for us to introduce new property in the future without breaking existing API.  I am keeping the new module interface IModule2 as an interface instead of an abstract class to prevent multiple inheritance problem for existing adopters if they decide to switch to this new interface.
Comment 22 Elson Yuen CLA 2013-02-05 16:59:07 EST
Created attachment 226594 [details]
v1.0 Server Tools

I have made the following changes based on the patch on comment 20 for the patch on Server tools side:
1. Combined IModuleWithProperties and IModuleProperties.
2. Renamed IModuleWithProperties to IModule2 to follow similar naming convention as other Eclipse APIs when enhancing existing APIs.
3. Other code cleanup and making the code more robust.
Comment 23 Elson Yuen CLA 2013-02-05 17:09:47 EST
Created attachment 226595 [details]
JEE patch for a sample of implementation on the new API

In order to completely resolve this problem, we need changes on both the server tools side and the JEE side.  I have created this new patch on a potential implementation on the JEE side to make use of the new API.  Note that there are multiple places in the JEE code for creating modules.  All of them should be updated.  This patch only have one place updated to provide a sample implementation on that. 

In the sample implementation, the display name is only set to show in the form of "componentName(deployName)" only if the component name and deploy name are different.  I do it in this way since in most of the cases, the module and deploy name are the same.  So we'll only show the componentName in normal cases to avoid clustering the model name labels on the UI.  Also, it will leave the existing UI unchanged if the two names are the same.

Chuck/Rob, can you review this one and provide the complete implementation on the JEE side? Thanks.
Comment 24 Rob Stryker CLA 2013-02-06 23:01:49 EST
Hi Elson:

Patch looks good. I'm only slightly concerned about combining the constants interface and the IModule2 together. My concern here is that if the two are in one interface which is intended to be implemented, if and when we decide to add a new constant, it could conflict with a subclass or adopter's fields. 

This is admittedly a low risk since we  use all CAPS when defining our static fields anyway, but, its still technically a possibility to cause API breakage when we add a field. 

Anyway, I don't think it's severe enough to put a hold on the patch, so, I +1 it.
Comment 25 Elson Yuen CLA 2013-02-07 11:59:34 EST
I find putting under the same class is easier for users to know what are the standard properties supported by the framework itself.  I added the prefix PROP_ to reduce the chance of getting conflict.  But I see your concern on potential API breakage.  I'll add a comment to the javadoc to mention that the framework reserve the property variables name begins with "PROP_" as a the prefix if you are ok with that approach.
Comment 26 Elson Yuen CLA 2013-02-15 09:54:36 EST
Created attachment 227135 [details]
v1.1 Server Tools

Updated patch with javadoc update.
Comment 27 Elson Yuen CLA 2013-02-15 16:27:18 EST
Server tools side of the fix dropped to 3.5.  In order to complete this fix, the Java EE side also needs to be fixed as well.  I have opened bug 400967 with a sample implementation on that.
Comment 28 Eclipse Genie CLA 2017-10-11 16:39:36 EDT
New Gerrit change created: https://git.eclipse.org/r/109153
Comment 29 Eclipse Genie CLA 2017-10-11 16:39:43 EDT
New Gerrit change created: https://git.eclipse.org/r/109154