This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 55414 - Delete web service action doesn't delete entry in sun-web.xml
Summary: Delete web service action doesn't delete entry in sun-web.xml
Status: CLOSED FIXED
Alias: None
Product: serverplugins
Classification: Unclassified
Component: Sun Appserver 8 (show other bugs)
Version: 4.x
Hardware: PC Windows XP
: P3 blocker (vote)
Assignee: _ ludo
URL:
Keywords:
: 55492 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-02-23 13:17 UTC by L Martinek
Modified: 2006-03-24 13:16 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
proposed diff (2.44 KB, patch)
2005-03-02 05:11 UTC, Pavel Buzek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description L Martinek 2005-02-23 13:17:15 UTC
[NB 4.1 Beta build 200502222100, JDK 1.5.0_02]

1. Create new web service in web application. 
2. In project view, select corresponding web
service node under Web Services node and invoke
Delete action. 
3. Now look at disk at sun-web.xml. Web service's
servlet entry isn't deleted.
4. Invoke Edit action on sun-web.xml. Web
service's servlet entry is still there. Notice
that file is marked as modifed. Close it and
discard changes.
5. Invoke Open action on sun-web.xml. Now there is
no ws entry. Close it and confirm to save changes.
Now entry is sun-web.xml finally disappeared.
Comment 1 Lukas Jungmann 2005-02-23 14:08:37 UTC
this is similar to issue #55181, isn't it?
Comment 2 _ pcw 2005-02-23 20:52:27 UTC
Is this a plugin issue, or is it a j2eeserver (e.g. jsr88 toolside)
issue?  In order to delete the servlet entry for the service in
sun-web.xml, the DConfigBean tree is created (if it doesn't already
exist) and then the DConfigBean is deleted when the corresponding
servlet entry in web.xml is deleted and the DConfigBean tree is marked
as modified.  If the user selects <Open> notice the tree is correct. 
If the user selects <Edit>, then since the DConfigBean tree is
modified, j2eeserver probably should save it before bringing up
sun-web.xml in the text editor (this aspect of this bug I feel is an
independent problem - if the DConfigBean tree is modified, it should
always be flushed before opening the editor.)

Of course, if the plugin does not save the DConfigBean tree on a
change like this, the user could open the editor and then discard the
unsaved changes, which is probably not good.  However, as far as I
know, the only mechanism the plugin has to save itself is to invoke
LifecycleManager.saveAll() after deleting the servlet which will save
all modified files in the IDE.  Furthermore, this will happen on all
delete servlet (or delete ejb, in the case of bug #55181) requests,
not just the ones related to this case, which is probably not desirable.

On the flip side, in the <delete> operation code, the webservice
module has no idea that this file has been modified in the first place
since it does not know any particulars of server specific DD's so
again, we're back to LifecycleManager.saveAll(), but at least at this
end, it will only happen when the user deletes a service via this
mechanism.  I just confirmed this is what I implemented in the <delete
web service client> code, which otherwise would have the same problem.
 Not the best solution, but probably better than this.
Comment 3 _ pcw 2005-02-23 20:54:59 UTC
Anyway, I will fix this and 55181 by having the web service <delete>
action invoke LifecycleManager.saveAll(), similar to what is done for
web service clients.

It would be much better if there were a j2eeserver API that allowed
the caller of DD API's to invoke a "saveConfiguration()" that would
save all modified configuration files.
Comment 4 _ pcw 2005-02-23 22:08:28 UTC
Fixed
Comment 5 Pavel Buzek 2005-02-24 13:21:17 UTC
Please consider a different fix.
I would rather make a change in j2eeserver then have an automatic
saveAll(). IMO this is a potential data lost.

I have seen discussion about auto save after refactoring - adding Jano
to CC.
Comment 6 L Martinek 2005-02-24 14:12:10 UTC
I found out another similar bug related to saveAll, but it's older, it
isn't caused by this fix. See issue #55492.
Comment 7 _ pcw 2005-02-24 17:26:26 UTC
Yes that bug is a related issue.  This topic (saveAll) was discussed
last summer and I thought the resolution was "not desirable, but only
solution available at the moment."

Anyway, in J2eeModuleProvider.ConfigSupport, there is a method
"saveConfiguration" that is commented out.  There is an implementation
of this method in SaveConfigurationImpl that is used (so I would guess
it does work).  If this were exposed, then wizards (or delete
operations) that manipulate the DD files can use it instead of
saveAll() to force save operations.
Comment 8 Pavel Buzek 2005-02-24 17:47:07 UTC
Petr, can you verify this it solves your problem?
If it does I agree with exposing it in API.
Comment 9 _ ludo 2005-03-02 04:34:05 UTC
so what is the status on this one?
Fixed? not Fixed? new API available? 
Other?
I am lost. Please, comment asap... Not having synced DD is a P2, not a
P3 imo.
Comment 10 Pavel Buzek 2005-03-02 04:50:08 UTC
*** Issue 55492 has been marked as a duplicate of this issue. ***
Comment 11 Pavel Buzek 2005-03-02 05:11:18 UTC
Ludo, it was not clear to me if Peter tested that this method actually
helps him (he said he would guess:-). That's why I asked him to check
before I add this to API. Otherwise I see no problem.

I will attach a diff and please test it. 
I will commit the change when you let me know it works.
Please also take 55492 back to plugin.

Also notifying devrev about an API change (I won't wait with commit,
knowing that this is blocking an important P2).
Comment 12 Pavel Buzek 2005-03-02 05:11:47 UTC
Created attachment 20604 [details]
proposed diff
Comment 13 _ pcw 2005-03-02 18:58:27 UTC
Sorry for the delay.  I got a comment from Nam in email that there
should be no need for saveConfiguration() to be exposed.  If I
understood him correctly, the act of saving a j2ee deployment
descriptor file (e.g. web.xml or ejb-jar.xml - which consumers of
ddapi can do by calling write()) should cause j2eeserver to also save
any open server specific configurations bound to that file, in this
case sun-web.xml or ejb-jar.xml.

That says to me that I need to remove the saveAll() calls we do have,
test the code again to see if it still fails, and file appropriate
bugs against j2eeserver. 
Comment 14 Pavel Buzek 2005-03-03 16:53:03 UTC
Please remove the save all code ASAP, it has very strange consequences
and should not have been done in the first place. I am getting dialogs
about files modified externally, etc. The fix is worse then the
original problem.

The DDFilesListener in j2eeserver added by Nam is an ungly hack, too.
Is it trying to do some king of heuristic based on the assumption that
the deployment descriptor is in the root of project?? Apparently it
does not work that way...

What;s the problem with save method?
Comment 15 _ ludo 2005-03-16 05:51:28 UTC
Pcw: can you comment on this bug asap, or move it to someone else?
Comment 16 _ ludo 2005-03-16 05:53:07 UTC
Pcw: can you comment on this bug asap, or move it to someone else?
Comment 17 _ pcw 2005-03-17 07:43:27 UTC
I have not yet taken any action on this bug.

We need to grep for LifecycleManager.saveAll() in web\project, j2ee\*, and
websvc\* and comment out those calls (maybe get a bit of background on the
circumstances of each so we know what to test as well).

In cases where the code had previously updated a deployment descriptor, we need
to call write() on the DDRoot, if this is not already being done in these places
(It likely won't be.)
Comment 18 _ pcw 2005-03-17 07:44:13 UTC
Oh, one more thing - there are some places in the appserver plugin that do this
too.  I guess those should also be removed for the same reasons, correct?
Comment 19 Petr Jiricka 2005-03-21 15:38:49 UTC
So could someone from the plugin team take this over? Rajeshwar or Nitya?

Comment 20 _ pcw 2005-03-21 17:51:49 UTC
I just stripped all <saveAll> calls from web/project, j2ee/ejbjarproject, and
websvc/core.  All locations where this was previously used are calling write on
a DD root of some type (web or ejb) after modifying said DD and are currently
marked with a FIXME comment from me to check that the server specific changes
were also saved (in case someone is debugging something like that).

I'll commit these pending update and commit validation on my source tree.  I
will also check the plugin and take care of that codebase as well.
Comment 21 _ pcw 2005-03-21 20:32:36 UTC
Changes to j2ee, web, and websvc code integrated.  Plugin changes pending.
Comment 22 _ pcw 2005-03-21 20:50:23 UTC
No plugin integration seems to be necessary as all calls there were already
commented out.
Comment 23 Pavel Buzek 2005-03-29 14:05:22 UTC
I can still see one call to saveAll in plugin code. Not sure if it is being
called... Please verify this:

appsrv-plugin\libsrc\org\netbeans\modules\j2ee\sun\ide\j2ee\ConfigurationSupportImpl.java

line 363
Comment 24 Pavel Buzek 2005-03-29 14:06:55 UTC
*** Issue 55492 has been marked as a duplicate of this issue. ***
Comment 25 _ ludo 2005-03-29 15:16:23 UTC
This is in a private function, which is never called, so I will remove this.
Comment 26 _ ludo 2005-03-29 15:16:59 UTC
fixed
Comment 27 L Martinek 2005-04-04 13:04:01 UTC
verified