Community
Participate
Working Groups
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
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?
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.
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.
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.
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>
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
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
@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.
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.
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.
Created attachment 225763 [details] a patch of api additions for servertools
Created attachment 225764 [details] a patch for jeetools to make use of the new api
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?
> 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.
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 :)
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)
+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.
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.
> 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.
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.
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.
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.
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.
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.
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.
Created attachment 227135 [details] v1.1 Server Tools Updated patch with javadoc update.
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.
New Gerrit change created: https://git.eclipse.org/r/109153
New Gerrit change created: https://git.eclipse.org/r/109154