Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 315093

Summary: deleting project imported from another workspace raise exception
Product: z_Archived Reporter: Bozier jerome <jerome.bozier>
Component: TPTPAssignee: Bozier jerome <jerome.bozier>
Status: CLOSED FIXED QA Contact: Kathy Chan <kathy>
Severity: normal    
Priority: P2 CC: jptoomey, paulslau
Version: unspecifiedFlags: jptoomey: review+
paulslau: review+
paulslau: review+
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
Whiteboard: adopter
Attachments:
Description Flags
patch
none
Patch V2
jerome.bozier: review?
patch V3 none

Description Bozier jerome CLA 2010-05-31 11:03:07 EDT
. import a project from another workspace
. try to delete it (do not select "delete project contens" else you will kill your other workspace....)

=> theses exceptions are raised :
java.lang.Throwable: error while putting container into trashbox : src
	at org.eclipse.hyades.test.ui.UiPlugin.logError(UiPlugin.java:343)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.DeleteContainerChange.reorgFolder(DeleteContainerChange.java:61)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.ReorgContainerChange.perform(ReorgContainerChange.java:116)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.DeleteContainerCompositeChange.perform(DeleteContainerCompositeChange.java:88)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.CompositeListenerChange.perform(CompositeListenerChange.java:39)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.ReorgContainerCompositeChange.performChilds(ReorgContainerCompositeChange.java:65)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.DeleteContainerCompositeChange.perform(DeleteContainerCompositeChange.java:93)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.CompositeListenerChange.perform(CompositeListenerChange.java:39)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringTransactionRootChange.access$0(RefactoringTransactionRootChange.java:1)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringTransactionRootChange$SaveResourceSetRunnable.run(RefactoringTransactionRootChange.java:42)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1957)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringTransactionRootChange.perform(RefactoringTransactionRootChange.java:63)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.ltk.core.refactoring.PerformChangeOperation$1.run(PerformChangeOperation.java:258)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975)
	at org.eclipse.ltk.core.refactoring.PerformChangeOperation.executeChange(PerformChangeOperation.java:306)
	at org.eclipse.ltk.internal.ui.refactoring.UIPerformChangeOperation.executeChange(UIPerformChangeOperation.java:92)
	at org.eclipse.ltk.core.refactoring.PerformChangeOperation.run(PerformChangeOperation.java:218)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975)
	at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87)
	at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)

____________________________________________

java.lang.Exception: invalid source
	at org.eclipse.hyades.test.ui.internal.util.ZipUtils.zip(ZipUtils.java:118)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.TrashBox.trashResource(TrashBox.java:122)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.DeleteContainerChange.reorgFolder(DeleteContainerChange.java:57)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.ReorgContainerChange.perform(ReorgContainerChange.java:116)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.DeleteContainerCompositeChange.perform(DeleteContainerCompositeChange.java:88)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.CompositeListenerChange.perform(CompositeListenerChange.java:39)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.ReorgContainerCompositeChange.performChilds(ReorgContainerCompositeChange.java:65)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.DeleteContainerCompositeChange.perform(DeleteContainerCompositeChange.java:93)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.CompositeListenerChange.perform(CompositeListenerChange.java:39)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringTransactionRootChange.access$0(RefactoringTransactionRootChange.java:1)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringTransactionRootChange$SaveResourceSetRunnable.run(RefactoringTransactionRootChange.java:42)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1957)
	at org.eclipse.hyades.test.ui.internal.navigator.refactoring.RefactoringTransactionRootChange.perform(RefactoringTransactionRootChange.java:63)
	at org.eclipse.ltk.core.refactoring.CompositeChange.perform(CompositeChange.java:278)
	at org.eclipse.ltk.core.refactoring.PerformChangeOperation$1.run(PerformChangeOperation.java:258)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975)
	at org.eclipse.ltk.core.refactoring.PerformChangeOperation.executeChange(PerformChangeOperation.java:306)
	at org.eclipse.ltk.internal.ui.refactoring.UIPerformChangeOperation.executeChange(UIPerformChangeOperation.java:92)
	at org.eclipse.ltk.core.refactoring.PerformChangeOperation.run(PerformChangeOperation.java:218)
	at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975)
	at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87)
	at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)

(the second stack trigger the first one i think)

theses exception probably comes from a misuse of workspace root location to calculate the physical file location on disk

this is a regression coming from 282145
Comment 1 Kathy Chan CLA 2010-05-31 13:48:52 EDT
Jerome,

You indicated that this is a regression coming from bug 282145 (which was fixed in release 4.6.2).  So was this scenario working in 4.6.1?  How common is the problem?

Adding Joe to the CC list to assess impact to consumer.

I'm tentatively targeting this to 4.7.1 unless this turns out to be a serious problem affecting many consumers.
Comment 2 Joe Toomey CLA 2010-05-31 14:52:34 EDT
Although this is a regression, the symptoms seem to be very minor.  No dialog box pops up for the exception (it just gets logged in the error log), and the project appears to be deleted successfully.

In light of this, I don't expect RPT will request a fix for this in 4.7.0, and agree with the target of 4.7.1
Comment 3 Bozier jerome CLA 2010-05-31 15:34:27 EDT
I discovered this bug while working on 228262 (and also 315078)
I only entered them to not forget to fix them
This configuration (deleting project that was imported in another workspace) was not covered by 282145 manual test : I did not thought of it when I wrote it because it is not a classic way of using eclipse. It is dangerous and I don't expect users to work like this (I erased 1 week of work like this by making and error...)
In conclusion, I don't think it is vital to fix it for 4.7.1 
Updating estimated time to fix it
Comment 4 Bozier jerome CLA 2010-06-01 08:47:55 EDT
Created attachment 170630 [details]
patch

this patch fix 2 problems :
. for resource location, it use resource.getLocation() to have its physical location on disk instead of building it with current workspace location + full path (that does not work on resource from project that are not in default location)
. fix an exception that sometime occur on project import, inside resource listener (listener expect an IFile instead of a IProject, and cast was a bit brutal)
Comment 5 Bozier jerome CLA 2010-06-01 08:48:21 EDT
updating worked hours
Comment 6 Bozier jerome CLA 2010-06-01 10:25:30 EDT
manual test + allTest added under CVS (will update wiki when patch will be under CVS)
Comment 7 Bozier jerome CLA 2010-06-01 10:30:58 EDT
Joe, could you review this patch please ?

i made some test this morning (without the patch) and found that you can lose some source code if patch is not there

if you import a project from another workspace that contains package and java source, when you it them without clicking on "delete project content on disk", the package and the java file may be physically erased (it's how i lost 1 week of coding some day ago). the exception raised during the delete seems to mix up the checkbox values in the refactoring tree

it is why i change my mind and really think it's needed to push it under 4.7
Comment 8 Joe Toomey CLA 2010-06-01 16:27:11 EDT
Jerome,

A couple of comments:

1) I reviewed your changes, and they seem fine.

2) I am not about to reproduce the condition where files are accidentally deleted.  We have a couple of developers on our team who had seen this issue before, but it was believed to have been fixed by https://bugs.eclipse.org/bugs/show_bug.cgi?id=284071 , and it had not been seen recently.  I tried to reproduce this defect based on your description using the adopting product, and I was unable to reproduce it. (i.e. From the test navigator, I imported an existing project without copying it into the workspace and then deleted it without checking the "delete files on sile system" checkbox.)  I see the two exceptions in the error log, but the project still exists on the file system.

I need to understand exactly when files will be deleted erroneously.  If we really delete users files erroneously, then I agree we should try to fix this in 4.7.0, but otherwise I'd prefer to wait for 4.7.1.  Can you please clarify exactly how to reproduce the data loss, and perhaps also explain how "the exception raised during the delete seems to mix up the checkbox values in the refactoring tree".  

Thanks!
Comment 9 Bozier jerome CLA 2010-06-02 11:40:40 EDT
after some testing, it can not reproduce the problem of unwanted deleted resource anymore (with or without the patch)

my thinking is that debug launching + some hot code replacing leaded to an unstable configuration.

in conclusion, i finally agree with Joe to defer it to 4.7.1
Comment 10 Bozier jerome CLA 2010-06-04 05:07:43 EDT
Created attachment 171075 [details]
Patch V2

this patch replace previous one

previous patch was fixing IFile/IFolder deleting when they where located on a project form another workspace

this version add the same kind of fix for physical delete of a IProject imported from another workspace
it also add some code cleaning (remove of unnecessary field + removing of compilation warning by typing the used maps)
Comment 11 Bozier jerome CLA 2010-06-04 05:08:05 EDT
updating worked hours
Comment 12 Paul Slauenwhite CLA 2010-06-21 12:28:32 EDT
Requires adding /org.eclipse.hyades.test.ui.navigator.tests/manual/Test.UI.TestNavigator_bugzilla_315093.testsuite to /org.eclipse.hyades.test.ui.navigator.tests/manual/AllTests.testsuite and http://wiki.eclipse.org/TPTP_Test_Tools_Project_Tests#Manual.
Comment 13 Paul Slauenwhite CLA 2010-06-22 13:56:47 EDT
Of interested but not formally requested by a consuming product.  As such, a candidate defect for TPTP 4.7.0.1.
Comment 14 Paul Slauenwhite CLA 2010-06-23 09:38:10 EDT
Patch reviewed and approved.
Comment 15 Bozier jerome CLA 2010-06-28 08:23:51 EDT
. patch pushed under CVS head
. allTest updated
. wiki updated

closing
Comment 16 Paul Slauenwhite CLA 2010-06-28 08:36:40 EDT
Requires verification before closing.
Comment 17 Bozier jerome CLA 2010-08-24 09:14:41 EDT
ok, test failed on latest build. 

here is the problem :
inside DeleteLogicalProjectChange : perform

		try {		
			try {
				TrashBox.instance().trashProject(project, false);
			} catch (Exception e) {
				UiPlugin.logError("error while putting project into trashbox : "+project.getName());
				UiPlugin.logError(e);
			}
			project.delete(false, true, new SubProgressMonitor(pm, 1));
		} catch (CoreException e) {
			UiPlugin.logError(e);
		}
		Change undo = null;
		try {
			undo = TrashBox.instance().createUntrashChange(project);
		} catch (Exception e) {
			UiPlugin.logError("error while creating undo delete for project : "+project.getName());
			UiPlugin.logError(e);
		}
		return undo;

problem is that undo is calculated after the "project.delete". so, when we make a "getLocation" inside createUntrashChange, the result is null...

new patch incomming
Comment 18 Bozier jerome CLA 2010-08-24 11:03:32 EDT
Created attachment 177326 [details]
patch V3

this patch complete previous one

now, undo change is calculated before the physical delete, so that we are able to retrieve physical location (it is too late if done after the delete)
Comment 19 Bozier jerome CLA 2010-08-24 11:04:59 EDT
Paul, could you review this small patch please ?

many thanks in advance
(re-opening the bug until this final fix is under CVS)
Comment 20 Bozier jerome CLA 2010-08-25 03:44:29 EDT
fix delivered under CVS after approval
Comment 21 Kathy Chan CLA 2011-02-11 13:46:10 EST
This defect had been resolved as FIXED for more than 1 month.  Please verify with the latest TPTP 4.7.2 driver.  If this defect is still left unverified by February 25, we'll close it on the originator's behalf.

TPTP 4.7.2 driver can be downloaded from:

http://www.eclipse.org/tptp/home/downloads/?ver=4.7.2
Comment 22 Bozier jerome CLA 2011-04-04 09:38:42 EDT
closing