[318755] [NLS] Java Facet Install Page not checking for duplicate folder names on add
diff --git a/plugins/org.eclipse.jst.common.project.facet.core/META-INF/MANIFEST.MF b/plugins/org.eclipse.jst.common.project.facet.core/META-INF/MANIFEST.MF
index 0bf4593..d507381 100644
--- a/plugins/org.eclipse.jst.common.project.facet.core/META-INF/MANIFEST.MF
+++ b/plugins/org.eclipse.jst.common.project.facet.core/META-INF/MANIFEST.MF
@@ -19,7 +19,8 @@
org.eclipse.wst.common.project.facet.core;bundle-version="[1.3.0,2.0.0)",
org.eclipse.jdt.launching;bundle-version="[3.4.0,4.0.0)",
org.eclipse.core.expressions;bundle-version="[3.4.0,4.0.0)",
- org.eclipse.core.net;bundle-version="[1.2.0,2.0.0)"
+ org.eclipse.core.net;bundle-version="[1.2.0,2.0.0)",
+ org.eclipse.core.filesystem;bundle-version="[1.3.0,2.0.0)"
Bundle-ActivationPolicy: lazy
Bundle-RequiredExecutionEnvironment: J2SE-1.5
Bundle-Activator: org.eclipse.jst.common.project.facet.core.internal.FacetCorePlugin
diff --git a/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/JavaFacetInstallConfig.java b/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/JavaFacetInstallConfig.java
index f32f554..c0f7bc9 100644
--- a/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/JavaFacetInstallConfig.java
+++ b/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/JavaFacetInstallConfig.java
@@ -7,24 +7,33 @@
*
* Contributors:
* Konstantin Komissarchik - initial implementation and ongoing maintenance
+ * shuff@us.ibm.com - several revisions of source paths validation logic
******************************************************************************/
package org.eclipse.jst.common.project.facet.core;
+import static org.eclipse.jst.common.project.facet.core.internal.FacetedProjectFrameworkJavaPlugin.PLUGIN_ID;
+
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
+import org.eclipse.core.filesystem.EFS;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IProjectDescription;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Path;
import org.eclipse.core.runtime.Platform;
+import org.eclipse.core.runtime.Status;
import org.eclipse.jst.common.project.facet.core.internal.FacetCorePlugin;
import org.eclipse.jst.common.project.facet.core.internal.JavaFacetUtil;
+import org.eclipse.osgi.util.NLS;
import org.eclipse.wst.common.project.facet.core.ActionConfig;
import org.eclipse.wst.common.project.facet.core.util.EventListenerRegistry;
import org.eclipse.wst.common.project.facet.core.util.IEventListener;
@@ -70,12 +79,20 @@
return this.installConfig;
}
}
+
+ /**
+ * Specifies if the local file system is case sensitive. Used by validation. This is a non-final
+ * instance field to allow unit tests to override the default behavior. Unit tests will access
+ * this field via reflection. Do not rename or otherwise modify without changing unit tests too.
+ */
+ private boolean caseSensitiveFs = EFS.getLocalFileSystem().isCaseSensitive();
+
private EventListenerRegistry<ChangeEvent.Type,ChangeEvent> listeners;
private List<IPath> sourceFolders;
private List<IPath> sourceFoldersReadOnly;
private IPath defaultOutputFolder;
-
+
public JavaFacetInstallConfig()
{
this.listeners = new EventListenerRegistry<ChangeEvent.Type,ChangeEvent>( ChangeEvent.Type.class );
@@ -119,6 +136,125 @@
return files;
}
+
+ @Override
+ public IStatus validate()
+ {
+ IStatus status = Status.OK_STATUS;
+
+ final List<IPath> localFolderList = new ArrayList<IPath>( this.sourceFolders );
+
+ while( ! localFolderList.isEmpty() )
+ {
+ final IPath folder = localFolderList.remove( 0 );
+ status = validateSourceFolder( localFolderList, folder );
+
+ if( ! status.isOK() )
+ {
+ break;
+ }
+ }
+
+ return status;
+ }
+
+ /**
+ * Validates the provided source folder against the existing configuration. Can be used to check
+ * if adding a given source folder to this configuration will introduce validation problems.
+ *
+ * @param candidateSourceFolder the source folder to validate
+ * @return status detailing the validity of the candidate source folder
+ * @throw {@link IllegalArgumentException} if candidateSourceFolder is null
+ */
+
+ public IStatus validateSourceFolder( final String candidateSourceFolder )
+ {
+ if( candidateSourceFolder == null )
+ {
+ throw new IllegalArgumentException();
+ }
+
+ return validateSourceFolder( this.sourceFolders, new Path( candidateSourceFolder.trim() ) );
+ }
+
+ private IStatus validateSourceFolder( final List<IPath> existingSourceFolders,
+ final IPath sourceFolder )
+ {
+ IStatus status = Status.OK_STATUS;
+
+ if( sourceFolder.segmentCount() == 0 )
+ {
+ status = new Status( IStatus.ERROR, PLUGIN_ID, Resources.mustSpecifySourceFolderMessage );
+ }
+ else
+ {
+ final String pjname = getFacetedProjectWorkingCopy().getProjectName();
+ final String fullPath = "/" + pjname + "/" + sourceFolder; //$NON-NLS-1$ //$NON-NLS-2$
+
+ status = ResourcesPlugin.getWorkspace().validatePath( fullPath, IResource.FOLDER );
+
+ if( status.isOK() )
+ {
+ for( IPath existingSourceFolder : existingSourceFolders )
+ {
+ status = validateSourceFolder( existingSourceFolder, sourceFolder );
+
+ if( ! status.isOK() )
+ {
+ break;
+ }
+ }
+ }
+ }
+
+ return status;
+ }
+
+ private IStatus validateSourceFolder( final IPath existingPath,
+ final IPath newPath )
+ {
+ final int existingPathLen = existingPath.segmentCount();
+ final int newPathLen = newPath.segmentCount();
+ final int minPathLen = Math.min( existingPathLen, newPathLen );
+
+ if( minPathLen == 0 )
+ {
+ // The check that handles this case better is done elsewhere.
+
+ return Status.OK_STATUS;
+ }
+
+ for( int i = 0; i < minPathLen; i++ )
+ {
+ if( comparePathSegments( existingPath.segment( i ), newPath.segment( i ) ) == false )
+ {
+ return Status.OK_STATUS;
+ }
+ }
+
+ final String message;
+
+ if( existingPathLen == newPathLen )
+ {
+ message = NLS.bind( Resources.nonUniqueSourceFolderMessage, newPath );
+ }
+ else if( existingPathLen > newPathLen )
+ {
+ message = NLS.bind( Resources.cannotNestSourceFoldersMessage, newPath, existingPath );
+ }
+ else
+ {
+ message = NLS.bind( Resources.cannotNestSourceFoldersMessage, existingPath, newPath );
+ }
+
+ return new Status( IStatus.ERROR, PLUGIN_ID, message );
+ }
+
+ private boolean comparePathSegments( final String a,
+ final String b )
+ {
+ return ( this.caseSensitiveFs && a.equals( b ) ) || ( ! this.caseSensitiveFs && a.equalsIgnoreCase( b ) );
+ }
public List<IPath> getSourceFolders()
{
@@ -155,6 +291,11 @@
public void addSourceFolder( final IPath path )
{
+ if( path == null )
+ {
+ throw new IllegalArgumentException();
+ }
+
final List<IPath> newSourceFolders = new ArrayList<IPath>( getSourceFolders() );
newSourceFolders.add( path );
setSourceFolders( newSourceFolders );
@@ -193,8 +334,6 @@
{
this.listeners.removeListener( listener );
}
-
-
private static boolean equal( final Object obj1,
final Object obj2 )
@@ -220,5 +359,20 @@
return value;
}
+
+ private static final class Resources
+
+ extends NLS
+
+ {
+ public static String nonUniqueSourceFolderMessage;
+ public static String cannotNestSourceFoldersMessage;
+ public static String mustSpecifySourceFolderMessage;
+
+ static
+ {
+ initializeMessages( JavaFacetInstallConfig.class.getName(), Resources.class );
+ }
+ }
}
diff --git a/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/JavaFacetInstallConfig.properties b/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/JavaFacetInstallConfig.properties
new file mode 100644
index 0000000..0bd73ae
--- /dev/null
+++ b/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/JavaFacetInstallConfig.properties
@@ -0,0 +1,3 @@
+nonUniqueSourceFolderMessage = Source folder "{0}" is already specified.
+cannotNestSourceFoldersMessage = Source folder "{1}" is contained by another source folder "{0}".
+mustSpecifySourceFolderMessage = Source folder must be specified.
\ No newline at end of file
diff --git a/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/internal/JavaFacetInstallDelegate.java b/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/internal/JavaFacetInstallDelegate.java
index f092977..52c39e6 100644
--- a/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/internal/JavaFacetInstallDelegate.java
+++ b/plugins/org.eclipse.jst.common.project.facet.core/src/org/eclipse/jst/common/project/facet/core/internal/JavaFacetInstallDelegate.java
@@ -18,10 +18,14 @@
import org.eclipse.core.resources.IFolder;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IProjectDescription;
+import org.eclipse.core.resources.IResource;
+import org.eclipse.core.resources.IResourceStatus;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IAdapterManager;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Path;
import org.eclipse.core.runtime.Platform;
import org.eclipse.jdt.core.IClasspathEntry;
import org.eclipse.jdt.core.IJavaProject;
@@ -65,9 +69,7 @@
for( IPath srcFolderPath : config.getSourceFolders() )
{
- final IFolder folder = project.getFolder( srcFolderPath );
- mkdirs( folder, false );
-
+ final IFolder folder = createFolder( project, srcFolderPath, false );
cp.add( JavaCore.newSourceEntry( folder.getFullPath() ) );
}
@@ -76,8 +78,7 @@
if( defOutputPath != null )
{
- defOutputFolder = project.getFolder( config.getDefaultOutputFolder() );
- mkdirs( defOutputFolder, true );
+ defOutputFolder = createFolder( project, config.getDefaultOutputFolder(), true );
}
// Add the java nature. This will automatically add the builder.
@@ -140,24 +141,55 @@
}
}
- private static void mkdirs( final IFolder folder,
- final boolean isDerived )
+ private static IFolder createFolder( final IProject project,
+ final IPath path,
+ final boolean isDerived )
throws CoreException
{
- if( ! folder.exists() )
+ IContainer current = project;
+
+ for( int i = 0, n = path.segmentCount(); i < n; i++ )
{
- final IContainer parent = folder.getParent();
+ final String name = path.segment( i );
+ IFolder folder = current.getFolder( new Path( name ) );
- if( parent instanceof IFolder )
+ if( ! folder.exists() )
{
- mkdirs( (IFolder) parent, isDerived );
+ try
+ {
+ folder.create( true, true, null );
+ folder.setDerived( isDerived, null );
+ }
+ catch( CoreException e )
+ {
+ // Eclipse Resources API is always case sensitive regardless of the underlying
+ // file system. In particular, IResource.exists() call will return false on
+ // case collision, but the create() call will fail.
+
+ final IStatus st = e.getStatus();
+
+ if( st.getCode() != IResourceStatus.CASE_VARIANT_EXISTS )
+ {
+ throw e;
+ }
+
+ for( IResource resource : current.members() )
+ {
+ if( resource instanceof IFolder && resource.getName().equalsIgnoreCase( name ) )
+ {
+ folder = (IFolder) resource;
+ break;
+ }
+ }
+ }
}
- folder.create( true, true, null );
- folder.setDerived( isDerived );
+ current = folder;
}
+
+ return (IFolder) current;
}
}
diff --git a/plugins/org.eclipse.jst.common.project.facet.ui/src/org/eclipse/jst/common/project/facet/ui/internal/JavaFacetInstallPage.java b/plugins/org.eclipse.jst.common.project.facet.ui/src/org/eclipse/jst/common/project/facet/ui/internal/JavaFacetInstallPage.java
index d12d788..d27bbef 100644
--- a/plugins/org.eclipse.jst.common.project.facet.ui/src/org/eclipse/jst/common/project/facet/ui/internal/JavaFacetInstallPage.java
+++ b/plugins/org.eclipse.jst.common.project.facet.ui/src/org/eclipse/jst/common/project/facet/ui/internal/JavaFacetInstallPage.java
@@ -25,9 +25,6 @@
import java.util.ArrayList;
import java.util.List;
-import org.eclipse.core.resources.IResource;
-import org.eclipse.core.resources.IWorkspace;
-import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Path;
@@ -61,7 +58,6 @@
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Text;
import org.eclipse.swt.widgets.Tree;
-import org.eclipse.wst.common.project.facet.core.IFacetedProjectWorkingCopy;
import org.eclipse.wst.common.project.facet.core.util.IEventListener;
import org.eclipse.wst.common.project.facet.core.util.internal.ObjectReference;
import org.eclipse.wst.common.project.facet.ui.AbstractFacetWizardPage;
@@ -72,49 +68,49 @@
public final class JavaFacetInstallPage
- extends AbstractFacetWizardPage
-
+ extends AbstractFacetWizardPage
+
{
- private static final String WIZARD_PAGE_NAME = "java.facet.install.page"; //$NON-NLS-1$
-
- private static final String IMG_KEY_SOURCE_FOLDER = "source.folder"; //$NON-NLS-1$
-
- private JavaFacetInstallConfig installConfig = null;
- private ImageRegistry imageRegistry = null;
- private Text defaultOutputFolderTextField = null;
- private TreeViewer sourceFoldersTreeViewer = null;
+ private static final String WIZARD_PAGE_NAME = "java.facet.install.page"; //$NON-NLS-1$
+
+ private static final String IMG_KEY_SOURCE_FOLDER = "source.folder"; //$NON-NLS-1$
+
+ private JavaFacetInstallConfig installConfig = null;
+ private ImageRegistry imageRegistry = null;
+ private Text defaultOutputFolderTextField = null;
+ private TreeViewer sourceFoldersTreeViewer = null;
private Tree sourceFoldersTree = null;
private Button addButton = null;
private Button editButton = null;
private Button removeButton = null;
-
- public JavaFacetInstallPage()
- {
- super( WIZARD_PAGE_NAME );
-
+
+ public JavaFacetInstallPage()
+ {
+ super( WIZARD_PAGE_NAME );
+
setTitle( Resources.pageTitle );
setDescription( Resources.pageDescription );
setImageDescriptor( getImageDescriptor( IMG_PATH_JAVA_WIZBAN ) );
this.imageRegistry = new ImageRegistry();
this.imageRegistry.put( IMG_KEY_SOURCE_FOLDER, getImageDescriptor( IMG_PATH_SOURCE_FOLDER ) );
- }
-
- public JavaFacetInstallConfig getConfig()
- {
- return this.installConfig;
- }
+ }
+
+ public JavaFacetInstallConfig getConfig()
+ {
+ return this.installConfig;
+ }
- public void setConfig( final Object config )
- {
- this.installConfig = (JavaFacetInstallConfig) config;
- }
+ public void setConfig( final Object config )
+ {
+ this.installConfig = (JavaFacetInstallConfig) config;
+ }
- public void createControl( final Composite parent)
- {
- final Composite composite = new Composite( parent, SWT.NONE );
- composite.setLayoutData( gdfill() );
- composite.setLayout( gl( 2 ) );
+ public void createControl( final Composite parent)
+ {
+ final Composite composite = new Composite( parent, SWT.NONE );
+ composite.setLayoutData( gdfill() );
+ composite.setLayout( gl( 2 ) );
composite.addDisposeListener
(
@@ -126,7 +122,7 @@
}
}
);
-
+
final Label sourceFoldersLabel = new Label( composite, SWT.NONE );
sourceFoldersLabel.setLayoutData( gdhspan( gd(), 2 ) );
sourceFoldersLabel.setText( Resources.sourceFoldersLabel );
@@ -138,7 +134,7 @@
this.sourceFoldersTree.getAccessible().addAccessibleListener
(
new AccessibleAdapter()
- {
+ {
public void getName( final AccessibleEvent event )
{
if( event.childID == -1 )
@@ -229,32 +225,32 @@
final Label defaultOutputFolderLabel = new Label( composite, SWT.NONE );
defaultOutputFolderLabel.setLayoutData( gdhspan( gd(), 2 ) );
- defaultOutputFolderLabel.setText( Resources.defaultOutputFolderLabel );
-
- this.defaultOutputFolderTextField = new Text( composite, SWT.BORDER );
- this.defaultOutputFolderTextField.setLayoutData( gdhspan( gdhfill(), 2 ) );
-
- bindUiToModel();
-
- setControl( composite );
- }
-
+ defaultOutputFolderLabel.setText( Resources.defaultOutputFolderLabel );
+
+ this.defaultOutputFolderTextField = new Text( composite, SWT.BORDER );
+ this.defaultOutputFolderTextField.setLayoutData( gdhspan( gdhfill(), 2 ) );
+
+ bindUiToModel();
+
+ setControl( composite );
+ }
+
private void bindUiToModel()
- {
- bindDefaultOutputFolder();
- }
-
- private void bindDefaultOutputFolder()
- {
- final JavaFacetInstallConfig installConfig = this.installConfig;
- final Text defaultOutputFolderTextField = this.defaultOutputFolderTextField;
-
- final ObjectReference<Boolean> updating = new ObjectReference<Boolean>( false );
-
- this.defaultOutputFolderTextField.addModifyListener
- (
- new ModifyListener()
- {
+ {
+ bindDefaultOutputFolder();
+ }
+
+ private void bindDefaultOutputFolder()
+ {
+ final JavaFacetInstallConfig installConfig = this.installConfig;
+ final Text defaultOutputFolderTextField = this.defaultOutputFolderTextField;
+
+ final ObjectReference<Boolean> updating = new ObjectReference<Boolean>( false );
+
+ this.defaultOutputFolderTextField.addModifyListener
+ (
+ new ModifyListener()
+ {
public void modifyText( final ModifyEvent e )
{
if( updating.get() )
@@ -274,11 +270,11 @@
updating.set( false );
}
}
- }
- );
-
- final IEventListener<JavaFacetInstallConfig.ChangeEvent> modelEventListener
- = new IEventListener<JavaFacetInstallConfig.ChangeEvent>()
+ }
+ );
+
+ final IEventListener<JavaFacetInstallConfig.ChangeEvent> modelEventListener
+ = new IEventListener<JavaFacetInstallConfig.ChangeEvent>()
{
public void handleEvent( final JavaFacetInstallConfig.ChangeEvent event )
{
@@ -302,14 +298,14 @@
};
this.installConfig.addListener
- (
- modelEventListener,
- JavaFacetInstallConfig.ChangeEvent.Type.DEFAULT_OUTPUT_FOLDER_CHANGED
- );
+ (
+ modelEventListener,
+ JavaFacetInstallConfig.ChangeEvent.Type.DEFAULT_OUTPUT_FOLDER_CHANGED
+ );
modelEventListener.handleEvent( null );
- }
-
+ }
+
private ImageRegistry getImageRegistry()
{
return this.imageRegistry;
@@ -321,19 +317,19 @@
return (IPath) sel.getFirstElement();
}
- private String convertToString( final IPath path )
- {
- return ( path == null ? "" : path.toOSString() ); //$NON-NLS-1$
- }
-
- private void updateButtonEnablement()
- {
- final boolean haveSelection = ! this.sourceFoldersTreeViewer.getSelection().isEmpty();
-
- this.editButton.setEnabled( haveSelection );
- this.removeButton.setEnabled( haveSelection );
- }
-
+ private String convertToString( final IPath path )
+ {
+ return ( path == null ? "" : path.toOSString() ); //$NON-NLS-1$
+ }
+
+ private void updateButtonEnablement()
+ {
+ final boolean haveSelection = ! this.sourceFoldersTreeViewer.getSelection().isEmpty();
+
+ this.editButton.setEnabled( haveSelection );
+ this.removeButton.setEnabled( haveSelection );
+ }
+
private void handleAddButtonPressed()
{
final InputDialog dialog
@@ -381,20 +377,15 @@
private IInputValidator createSourceFolderInputValidator()
{
- final IWorkspace ws = ResourcesPlugin.getWorkspace();
- final IFacetedProjectWorkingCopy fpjwc = this.installConfig.getFacetedProjectWorkingCopy();
- final String projectName = fpjwc.getProjectName();
-
final IInputValidator validator = new IInputValidator()
{
public String isValid( final String newText )
{
- final String fullPath = "/" + projectName + "/" + newText; //$NON-NLS-1$ //$NON-NLS-2$
- final IStatus result = ws.validatePath( fullPath, IResource.FOLDER );
+ final IStatus status = getConfig().validateSourceFolder( newText );
- if( result.getSeverity() == IStatus.ERROR )
+ if( ! status.isOK() )
{
- return result.getMessage();
+ return status.getMessage();
}
else
{
@@ -405,6 +396,7 @@
return validator;
}
+
private final class SourceFoldersContentProvider
implements ITreeContentProvider
@@ -436,7 +428,7 @@
final Object oldObject,
final Object newObject ) {}
}
-
+
private final class SourceFoldersLabelProvider
implements ILabelProvider