[204484] Adding JSF facet to an imported EAR breaks web.xml

[226709] Removal of facet from Web 2.5 app fails to update web.xml

4 files changed, 202 insertions, 164 deletions

diff --git a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFFacetUninstallDelegate.java b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFFacetUninstallDelegate.java
index f4592df61..a1ae42618 100644
--- a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFFacetUninstallDelegate.java
+++ b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFFacetUninstallDelegate.java
@@ -16,6 +16,7 @@ import java.util.List;
 
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Path;
@@ -121,20 +122,21 @@ public final class JSFFacetUninstallDelegate implements IDelegate {
 		IModelProvider provider = ModelProviderManager.getModelProvider(project);
 		Object webAppObj = provider.getModelObject();
 		if (webAppObj != null) {
+			IPath ddPath = new Path("WEB-INF").append("web.xml"); //$NON-NLS-1$ //$NON-NLS-2$
 			if (isJavaEEWebApp(webAppObj)){		
 				WebApp webApp = (WebApp)webAppObj;
 				Servlet servlet = JSFUtils12.findJSFServlet(webApp);
 				if (servlet == null)
 					return;
 
-				provider.modify(new RemoveJSFFromJavaEEWebAppOperation(webApp, servlet), new Path("WEB-INF").append("web.xml")); //$NON-NLS-1$ //$NON-NLS-2$
+				provider.modify(new RemoveJSFFromJavaEEWebAppOperation(project), ddPath);
 			} else {//2.3 or 2.4 web app
 				org.eclipse.jst.j2ee.webapplication.WebApp webApp = (org.eclipse.jst.j2ee.webapplication.WebApp)webAppObj;
 				org.eclipse.jst.j2ee.webapplication.Servlet servlet = JSFUtils11.findJSFServlet(webApp);
 				if (servlet == null)
 					return;
 
-				provider.modify(new RemoveJSFFromJ2EEWebAppOperation(webApp, servlet), new Path("WEB-INF").append("web.xml")); //$NON-NLS-1$ //$NON-NLS-2$				
+				provider.modify(new RemoveJSFFromJ2EEWebAppOperation(project), ddPath);			
 			}
 		}
 
@@ -149,21 +151,22 @@ public final class JSFFacetUninstallDelegate implements IDelegate {
 	}
 
 	static class RemoveJSFFromJavaEEWebAppOperation implements Runnable {
-		private WebApp _webApp;
-		private Servlet _servlet;
+		private IProject _project;
 		
-		RemoveJSFFromJavaEEWebAppOperation(final WebApp webApp, final Servlet servlet){
-			this._webApp = webApp;
-			this._servlet = servlet;
+		RemoveJSFFromJavaEEWebAppOperation(final IProject project){
+			this._project = project;			
 		}
 		
 		public void run() {
+			WebApp webApp = (WebApp) ModelProviderManager.getModelProvider(_project).getModelObject();
+			Servlet servlet = JSFUtils12.findJSFServlet(webApp);
+			
 			// remove faces url mappings
-			JSFUtils12.removeURLMappings(_webApp, _servlet);
+			JSFUtils12.removeURLMappings(webApp, servlet);
 			// remove context params
-			removeJSFContextParams(_webApp, _servlet);
+			removeJSFContextParams(webApp, servlet);
 			// remove servlet
-			removeJSFServlet(_webApp, _servlet);
+			removeJSFServlet(webApp, servlet);
 			
 		}
 		private void removeJSFContextParams(final WebApp webApp, final Servlet servlet) {
@@ -177,28 +180,29 @@ public final class JSFFacetUninstallDelegate implements IDelegate {
 			}
 		}
 
-		private void removeJSFServlet(final WebApp webApp, final Servlet servlet) {
+		private void removeJSFServlet(final WebApp webApp, final Servlet servlet) {		
 			webApp.getServlets().remove(servlet);
 		}
 		
 	}
 	
 	static class RemoveJSFFromJ2EEWebAppOperation implements Runnable {
-		private org.eclipse.jst.j2ee.webapplication.WebApp _webApp;
-		private org.eclipse.jst.j2ee.webapplication.Servlet _servlet;
+		private IProject _project;		
 		
-		RemoveJSFFromJ2EEWebAppOperation(final org.eclipse.jst.j2ee.webapplication.WebApp webApp, final org.eclipse.jst.j2ee.webapplication.Servlet servlet){
-			this._webApp = webApp;
-			this._servlet = servlet;
+		RemoveJSFFromJ2EEWebAppOperation(final IProject project){
+			this._project = project;
 		}
 		
 		public void run() {
+			org.eclipse.jst.j2ee.webapplication.WebApp webApp = (org.eclipse.jst.j2ee.webapplication.WebApp) ModelProviderManager.getModelProvider(_project).getModelObject();
+			org.eclipse.jst.j2ee.webapplication.Servlet servlet = JSFUtils11.findJSFServlet(webApp);
+
 			// remove faces url mappings
-			JSFUtils11.removeURLMappings(_webApp, _servlet);
+			JSFUtils11.removeURLMappings(webApp, servlet);
 			// remove context params
-			removeJSFContextParams(_webApp, _servlet);
+			removeJSFContextParams(webApp, servlet);
 			// remove servlet
-			removeJSFServlet(_webApp, _servlet);
+			removeJSFServlet(webApp, servlet);
 			
 		}
 		private void removeJSFContextParams(final org.eclipse.jst.j2ee.webapplication.WebApp webApp, final org.eclipse.jst.j2ee.webapplication.Servlet servlet) {
diff --git a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils.java b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils.java
index 749d7f427..a348f0d3f 100644
--- a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils.java
+++ b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils.java
@@ -150,9 +150,9 @@ public abstract class JSFUtils {
 	 */
 	protected static String getDisplayName(IDataModel config) {
 		String displayName = config.getStringProperty(IJSFFacetInstallDataModelProperties.SERVLET_NAME);
-		if (displayName.equals("")) //$NON-NLS-1$
+		if (displayName == null || displayName.trim().equals("")) //$NON-NLS-1$
 			displayName = JSF_DEFAULT_SERVLET_NAME;
-		return displayName;
+		return displayName.trim();
 	}
 	
 	/**
@@ -161,9 +161,9 @@ public abstract class JSFUtils {
 	 */
 	protected static String getServletClassname(IDataModel config) {
 		String className = config.getStringProperty(IJSFFacetInstallDataModelProperties.SERVLET_CLASSNAME);
-		if (className.equals("")) //$NON-NLS-1$
+		if (className == null || className.trim().equals("")) //$NON-NLS-1$
 			className = JSF_SERVLET_CLASS;
-		return className;
+		return className.trim();
 	}
 	
 	/**
@@ -217,107 +217,95 @@ public abstract class JSFUtils {
 	 * @param prefPrefixMapping
 	 * @return the result
 	 */
-	public static MappingSearchResult searchServletMappings(final  List<String> mappings, String prefExtMapping, String prefPrefixMapping)
-	{
-	    String firstExtFound = null;
-	    String firstPrefixFound = null;
-	    boolean foundExtMapping = false;
-	    boolean foundPrefixMapping = false;
-	    
-	    // if the caller has no preferredMapping, then
-        // set it to something guaranteed to be non-null
-        // and which is guaranteed not to match anything
-        // that pass isExtensionMapping
-	    if (prefExtMapping == null)
-	    {
-	        prefExtMapping = "NOTANEXTENSIONMAPPING";
-	    }
-	    
-	    // similarly, guarantee that if the caller has no
-	    // preferred prefix mapping, that we set a non-null
-	    // comp mapping
-	    if (prefPrefixMapping == null)
-	    {
-	        prefPrefixMapping = "NOTAPREFIXMAPPING";
-	    }
+	public static MappingSearchResult searchServletMappings(
+			final List<String> mappings, String prefExtMapping,
+			String prefPrefixMapping) {
+		String firstExtFound = null;
+		String firstPrefixFound = null;
+		boolean foundExtMapping = false;
+		boolean foundPrefixMapping = false;
 
-        SEARCH_LOOP:for (String mapping : mappings)
-        {
-            if (isExtensionMapping(mapping))
-            {
-                // can assum that mapping is non-null since
-                // it is an ext mapping
-                if (prefExtMapping.equals(mapping.trim()))
-                {
-                    firstExtFound = prefExtMapping;
-                    continue;
-                }
+		// if the caller has no preferredMapping, then
+		// set it to something guaranteed to be non-null
+		// and which is guaranteed not to match anything
+		// that pass isExtensionMapping
+		if (prefExtMapping == null) {
+			prefExtMapping = "NOTANEXTENSIONMAPPING";
+		}
 
-                if (firstExtFound == null)
-                {
-                    firstExtFound = mapping.trim();
-                }
-            }
-            else if (isPrefixMapping(mapping))
-            {
-                if (prefPrefixMapping.equals(mapping.trim()))
-                {
-                    firstPrefixFound = prefPrefixMapping;
-                    continue;
-                }
-                
-                if (firstPrefixFound == null)
-                {
-                    firstPrefixFound = mapping.trim();
-                }
-            }
-            
-            if (foundExtMapping && foundPrefixMapping)
-            {
-                break SEARCH_LOOP;
-            }
-        }
-	    
-	    return new MappingSearchResult(firstExtFound, firstPrefixFound);
+		// similarly, guarantee that if the caller has no
+		// preferred prefix mapping, that we set a non-null
+		// comp mapping
+		if (prefPrefixMapping == null) {
+			prefPrefixMapping = "NOTAPREFIXMAPPING";
+		}
+
+		SEARCH_LOOP: for (String mapping : mappings) {
+			if (isExtensionMapping(mapping)) {
+				// can assum that mapping is non-null since
+				// it is an ext mapping
+				if (prefExtMapping.equals(mapping.trim())) {
+					firstExtFound = prefExtMapping;
+					continue;
+				}
+
+				if (firstExtFound == null) {
+					firstExtFound = mapping.trim();
+				}
+			} else if (isPrefixMapping(mapping)) {
+				if (prefPrefixMapping.equals(mapping.trim())) {
+					firstPrefixFound = prefPrefixMapping;
+					continue;
+				}
+
+				if (firstPrefixFound == null) {
+					firstPrefixFound = mapping.trim();
+				}
+			}
+
+			if (foundExtMapping && foundPrefixMapping) {
+				break SEARCH_LOOP;
+			}
+		}
+
+		return new MappingSearchResult(firstExtFound, firstPrefixFound);
 	}
 	
     /**
-     * The result of a servlet mapping search
-     *
-     */
-    public static class MappingSearchResult
-    {
-        private final String  _extensionMapping; // may be null;
-        private final String  _prefixMapping;    // may be null
-        
-        MappingSearchResult(final String extensionMapping, final String prefixMapping)
-        {
-            _extensionMapping = extensionMapping;
-            _prefixMapping = prefixMapping;
-        }
+	 * The result of a servlet mapping search
+	 * 
+	 */
+	public static class MappingSearchResult {
+		private final String _extensionMapping; // may be null;
+		private final String _prefixMapping; // may be null
 
-        /**
-         * @return true if the search yielded a valid result
-         */
-        public boolean isResult()
-        {
-            return _extensionMapping != null || _prefixMapping != null;
-        }
+		MappingSearchResult(final String extensionMapping,
+				final String prefixMapping) {
+			_extensionMapping = extensionMapping;
+			_prefixMapping = prefixMapping;
+		}
 
-        /**
-         * @return the first extension mapping matching search criteria or null
-         * if none
-         */
-        public final String getExtensionMapping() {
-            return _extensionMapping;
-        }
+		/**
+		 * @return true if the search yielded a valid result
+		 */
+		public boolean isResult() {
+			return _extensionMapping != null || _prefixMapping != null;
+		}
 
-        /**
-         * @return the first prefix mapping matching search criteria or null
-         * if none
-         */
-        public final String getPrefixMapping() {
-            return _prefixMapping;
-        }
+		/**
+		 * @return the first extension mapping matching search criteria or null
+		 * if none
+		 */
+		public final String getExtensionMapping() {
+			return _extensionMapping;
+		}
+
+		/**
+		 * @return the first prefix mapping matching search criteria or null
+		 * if none
+		 */
+		public final String getPrefixMapping() {
+			return _prefixMapping;
+		}
     }
 }
diff --git a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils11.java b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils11.java
index 70ec7af18..8fa212fcd 100644
--- a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils11.java
+++ b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils11.java
@@ -16,6 +16,7 @@ import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
@@ -49,8 +50,9 @@ public class JSFUtils11 extends JSFUtils {
 	 * Convenience method for getting writeable WebApp model
 	 * @param project
 	 * @return WebArtifactEdit
+	 * @deprecated - must use IModelProviders
 	 */
-	public static WebArtifactEdit getWebArtifactEditForWrite(IProject project) {
+	public static WebArtifactEdit getWebArtifactEditForWrite(final IProject project) {
 		return WebArtifactEdit.getWebArtifactEditForWrite(project);
 	}
 
@@ -58,8 +60,9 @@ public class JSFUtils11 extends JSFUtils {
 	 * Convenience method for getting read-only WebApp model
 	 * @param project
 	 * @return WebArtifactEdit
+	 * @deprecated - must use IModelProviders
 	 */
-	public static WebArtifactEdit getWebArtifactEditForRead(IProject project) {
+	public static WebArtifactEdit getWebArtifactEditForRead(final IProject project) {
 		return WebArtifactEdit.getWebArtifactEditForRead(project);
 	}
 
@@ -67,7 +70,7 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param webApp as Object
 	 * @return Servlet - the JSF Servlet for the specified WebApp or null if not present
 	 */
-	public static Servlet findJSFServlet(Object webApp) {
+	public static Servlet findJSFServlet(final Object webApp) {
 		Iterator it = null;
 		if (webApp == null)
 			return null;
@@ -83,12 +86,14 @@ public class JSFUtils11 extends JSFUtils {
 			if (servlet != null && servlet.getWebType() != null) {
 				
 				if(	servlet.getWebType().isServletType()) {
-					if (((ServletType) servlet.getWebType()).getClassName().equals(
+					if (((ServletType) servlet.getWebType()).getClassName() != null && 
+							((ServletType) servlet.getWebType()).getClassName().trim().equals(
 							JSF_SERVLET_CLASS)) {
 						return servlet;
 					}
 				} else if (servlet.getWebType().isJspType()) {
-					if (((JSPType) servlet.getWebType()).getJspFile().equals(
+					if (((JSPType) servlet.getWebType()).getJspFile() != null && 
+							((JSPType) servlet.getWebType()).getJspFile().trim().equals(
 							JSF_SERVLET_CLASS)) {
 						return servlet;
 					}
@@ -106,7 +111,7 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param jsfVersion
 	 * @param configPath
 	 */
-	public static void createConfigFile(String jsfVersion, IPath configPath) {
+	public static void createConfigFile(final String jsfVersion, final IPath configPath) {
 		FileOutputStream os = null;
 		PrintWriter pw = null;
 		final String QUOTE = new String(new char[] { '"' });
@@ -164,8 +169,8 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param servlet
 	 * @return Servlet servlet - if passed servlet was null, will return created servlet
 	 */
-	public static Servlet createOrUpdateServletRef(WebApp webApp,
-			IDataModel config, Servlet servlet) {
+	public static Servlet createOrUpdateServletRef(final WebApp webApp,
+			final IDataModel config, Servlet servlet) {
 		
 		String displayName 	= getDisplayName(config);
 		String className 	= getServletClassname(config);
@@ -185,17 +190,42 @@ public class JSFUtils11 extends JSFUtils {
 			webApp.getServlets().add(servlet);			
 		} else {
 			// update
+			updateServletMappings(webApp, servlet, servlet.getServletName().trim(), displayName);
 			servlet.setServletName(displayName);
 			servlet.setLoadOnStartup(Integer.valueOf(1));
 		}
 		return servlet;
 	}
 
+	private static void updateServletMappings(final WebApp webApp, final Servlet servlet,
+			final String servletName, final String newServletName) {
+		List<ServletMapping> mappings = findServletMappings(webApp, servlet, servletName);
+		for (ServletMapping map : mappings){
+			map.setName(newServletName);
+		}
+		
+	}
+
+	
+	private static List<ServletMapping> findServletMappings(final WebApp webApp, final Servlet servlet, final String servletName) {			
+		List<ServletMapping> mappings = new ArrayList<ServletMapping>();
+		List<ServletMapping> allMappings = webApp.getServletMappings();			
+		for (int i=allMappings.size()-1;i>=0;--i){
+			ServletMapping mapping = allMappings.get(i);
+			if (mapping != null && 
+					mapping.getServlet() != null && 
+					mapping.getServlet().getServletName() != null &&
+					mapping.getServlet().getServletName().trim().equals(servletName))
+				mappings.add(mapping);
+		}		
+		return mappings;		
+	}
+
 	/**
 	 * @param webApp as Object
 	 * @return true if webApp instanceof org.eclipse.jst.javaee.web.WebApp
 	 */
-	public static boolean isWebApp25(Object webApp) {
+	public static boolean isWebApp25(final Object webApp) {
 		if (webApp instanceof org.eclipse.jst.javaee.web.WebApp)
 			return true;
 		return false;
@@ -208,8 +238,8 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param urlMappingList - list of string values to  be used in url-pattern for servlet-mapping
 	 * @param servlet
 	 */
-	public static void setUpURLMappings(WebApp webApp, List urlMappingList,
-			Servlet servlet) {
+	public static void setUpURLMappings(final WebApp webApp, final List urlMappingList,
+			final Servlet servlet) {
 		// Add mappings
 		Iterator it = urlMappingList.iterator();
 		while (it.hasNext()) {
@@ -237,8 +267,9 @@ public class JSFUtils11 extends JSFUtils {
 				if (mapping != null && 
 						mapping.getServlet() != null && 
 						mapping.getServlet().getServletName() != null &&
-						mapping.getServlet().getServletName().equals(servletName) &&
-						mapping.getUrlPattern().equals(pattern)) {
+						mapping.getServlet().getServletName().trim().equals(servletName) &&
+						mapping.getUrlPattern() != null && 
+						mapping.getUrlPattern().trim().equals(pattern)) {
 					return true;
 				}
 			}
@@ -251,7 +282,7 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param webApp
 	 * @param servlet
 	 */
-	public static void removeURLMappings(WebApp webApp, Servlet servlet) {
+	public static void removeURLMappings(final WebApp webApp, final Servlet servlet) {
 		List mappings = webApp.getServletMappings();
 		String servletName = servlet.getServletName();
 		if (servletName != null) {
@@ -260,7 +291,7 @@ public class JSFUtils11 extends JSFUtils {
 				if (mapping != null && 
 						mapping.getServlet() != null && 
 						mapping.getServlet().getServletName() != null &&
-						mapping.getServlet().getServletName()
+						mapping.getServlet().getServletName().trim()
 							.equals(servletName)) {
 					mappings.remove(mapping);
 				}
@@ -273,8 +304,8 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param webApp
 	 * @param config
 	 */
-	public static void setupConfigFileContextParamForV2_3(WebApp webApp,
-			IDataModel config) {
+	public static void setupConfigFileContextParamForV2_3(final WebApp webApp,
+			final IDataModel config) {
 		// if not default name and location, then add context param
 		ContextParam cp = null;
 		ContextParam foundCP = null;
@@ -314,8 +345,8 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param webApp
 	 * @param config
 	 */
-	public static void setupConfigFileContextParamForV2_4(WebApp webApp,
-			IDataModel config) {
+	public static void setupConfigFileContextParamForV2_4(final WebApp webApp,
+			final IDataModel config) {
 		// if not default name and location, then add context param
 		ParamValue foundCP = null;
 		ParamValue cp = null;
@@ -327,7 +358,7 @@ public class JSFUtils11 extends JSFUtils {
 				cp = (ParamValue) it.next();
 				if (cp != null && 
 						cp.getName() != null &&
-						cp.getName().equals(JSF_CONFIG_CONTEXT_PARAM)) {
+						cp.getName().trim().equals(JSF_CONFIG_CONTEXT_PARAM)) {
 					foundCP = cp;
 					found = true;
 				}
@@ -355,7 +386,7 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param webAppObj as Object
 	 * @return true if webApp instanceof org.eclipse.jst.j2ee.web.WebApp and versionID == 24
 	 */
-	public static boolean isWebApp24(Object webAppObj) {
+	public static boolean isWebApp24(final Object webAppObj) {
 		if (webAppObj instanceof WebApp &&
 				((WebApp)webAppObj).getVersionID() == 24)
 			return true;
@@ -366,7 +397,7 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param webAppObj as Object
 	 * @return true if webApp instanceof org.eclipse.jst.j2ee.web.WebApp and versionID == 23
 	 */
-	public static boolean isWebApp23(Object webAppObj) {
+	public static boolean isWebApp23(final Object webAppObj) {
 		if (webAppObj instanceof WebApp &&
 				((WebApp)webAppObj).getVersionID() == 23)
 			return true;
@@ -377,13 +408,13 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param webApp
 	 * @return the default file extension from the context param.  Default is "jsp" if no context param.
 	 */
-	public static String getDefaultSuffix(WebApp webApp) {
+	public static String getDefaultSuffix(final WebApp webApp) {
 		String defaultSuffix = "jsp"; //$NON-NLS-1$
 		for (Iterator it = webApp.getContexts().iterator();it.hasNext();) {		
 			ContextParam cp = (ContextParam) it.next();		
 			if (cp != null &&
 					cp.getParamName() != null && 
-					cp.getParamName().equals(JSF_DEFAULT_SUFFIX_CONTEXT_PARAM)){				
+					cp.getParamName().trim().equals(JSF_DEFAULT_SUFFIX_CONTEXT_PARAM)){				
 				String defSuffix = cp.getParamValue();
 				if (defSuffix.startsWith(".")) //$NON-NLS-1$
 					defSuffix = defSuffix.substring(1);
@@ -398,7 +429,7 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param map
 	 * @return prefix mapping.  may return null.
 	 */
-	public static String getPrefixMapping(ServletMapping map) {
+	public static String getPrefixMapping(final ServletMapping map) {
 		IPath extPath = new Path(map.getUrlPattern());
 		if (extPath != null){
 			String ext = extPath.getFileExtension();
@@ -420,7 +451,7 @@ public class JSFUtils11 extends JSFUtils {
 	 * @param map
 	 * @return extension from map.  Will return null if file extension not found in url patterns.
 	 */
-	public static String getFileExtensionFromMap(ServletMapping map) {
+	public static String getFileExtensionFromMap(final ServletMapping map) {
 		IPath extPath = new Path(map.getUrlPattern());
 		if (extPath != null){
 			String ext = extPath.getFileExtension();
diff --git a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils12.java b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils12.java
index ab14bc69b..71a2b2bff 100644
--- a/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils12.java
+++ b/jsf/plugins/org.eclipse.jst.jsf.core/src/org/eclipse/jst/jsf/core/internal/project/facet/JSFUtils12.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005 Oracle Corporation.
+ * Copyright (c) 2005, 2008 Oracle Corporation.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -46,14 +46,14 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param webApp
 	 * @return Servlet - the JSF Servlet for the specified WebApp or null if not present
 	 */
-	public static Servlet findJSFServlet(WebApp webApp) {
+	public static Servlet findJSFServlet(final WebApp webApp) {
 
 		Iterator it = webApp.getServlets().iterator();
 		
 		while (it.hasNext()) {
             Servlet servlet = (Servlet) it.next();
 			if (servlet.getServletClass() != null &&
-					servlet.getServletClass().equals (JSF_SERVLET_CLASS)) {
+					servlet.getServletClass().trim().equals (JSF_SERVLET_CLASS)) {
 				return servlet;
 			}
 		}
@@ -68,7 +68,7 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param jsfVersion
 	 * @param configPath
 	 */
-	public static void createConfigFile(String jsfVersion, IPath configPath) {
+	public static void createConfigFile(final String jsfVersion, final IPath configPath) {
 		FileOutputStream os = null;
 		PrintWriter pw = null;
 		final String QUOTE = new String(new char[] { '"' });
@@ -129,8 +129,8 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param servlet
 	 * @return Servlet servlet - if passed servlet was null, will return created servlet
 	 */
-	public static Servlet createOrUpdateServletRef(WebApp webApp,
-			IDataModel config, org.eclipse.jst.javaee.web.Servlet servlet) {
+	public static Servlet createOrUpdateServletRef(final WebApp webApp,
+			final IDataModel config, org.eclipse.jst.javaee.web.Servlet servlet) {
 		
 		String displayName = getDisplayName(config);		
 		String className = getServletClassname(config);
@@ -146,18 +146,28 @@ public class JSFUtils12 extends JSFUtils {
 			webApp.getServlets().add(servlet);
 
 		} else {
-			// update
+			updateServletMappings(webApp, servlet, displayName);
 			servlet.setServletName(displayName);
 			servlet.setLoadOnStartup(Integer.valueOf(1));
 		}
 		return servlet;
 	}
 
+	private static void updateServletMappings(final WebApp webApp, final Servlet servlet,
+			final String displayName) {
+		// update mappings for new name
+		ServletMapping mapping = findServletMapping(webApp, servlet);
+		if (mapping != null) {
+			mapping.setServletName(displayName);
+		}
+		
+	}
+
 	/**
 	 * @param webAppObj as Object
 	 * @return true if webApp instanceof org.eclipse.jst.javaee.web.WebApp and WebAppVersionType._25
 	 */
-	public static boolean isWebApp25(Object webAppObj) {
+	public static boolean isWebApp25(final Object webAppObj) {
 		if (webAppObj instanceof WebApp &&
 				((WebApp)webAppObj).getVersion() == WebAppVersionType._25_LITERAL) 
 			return true;
@@ -171,8 +181,8 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param urlMappingList - list of string values to  be used in url-pattern for servlet-mapping
 	 * @param servlet
 	 */
-	public static void setUpURLMappings(WebApp webApp, List urlMappingList,
-			Servlet servlet) {
+	public static void setUpURLMappings(final WebApp webApp, final List urlMappingList,
+			final Servlet servlet) {
 		
 		if (urlMappingList.size() > 0) {
 			ServletMapping mapping = findServletMapping(webApp, servlet);
@@ -197,7 +207,9 @@ public class JSFUtils12 extends JSFUtils {
 	private static ServletMapping findServletMapping(final WebApp webApp, final Servlet servlet) {
 		for (Iterator it=webApp.getServletMappings().iterator();it.hasNext();){
 			ServletMapping mapping = (ServletMapping)it.next();
-			if (mapping.getServletName().equals(servlet.getServletName()))
+			if (mapping.getServletName() != null && 
+					servlet.getServletName() != null &&
+					mapping.getServletName().trim().equals(servlet.getServletName().trim()))
 				return mapping;
 		}
 		return null;
@@ -206,7 +218,9 @@ public class JSFUtils12 extends JSFUtils {
 	private static boolean doesServletMappingPatternExist(final WebApp webApp, final ServletMapping mapping,
 			final String pattern) {	
 		for (Iterator it=mapping.getUrlPatterns().iterator();it.hasNext();){
-			if(pattern.equals(((UrlPatternType)it.next()).getValue()))
+			String patternTypeValue = ((UrlPatternType)it.next()).getValue();
+			if( patternTypeValue != null && 
+					pattern.equals(patternTypeValue.trim()))
 				return true;
 		}
 		return false;
@@ -217,15 +231,16 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param webApp
 	 * @param servlet
 	 */
-	public static void removeURLMappings(WebApp webApp, Servlet servlet) {
+	public static void removeURLMappings(final WebApp webApp, final Servlet servlet) {
 		List mappings = webApp.getServletMappings();					
 		String servletName = servlet.getServletName();
 		if (servletName != null) {
+			servletName = servletName.trim();
 			for (int i=mappings.size()-1;i>=0;--i){
 				ServletMapping mapping = (ServletMapping)mappings.get(i);
 				if (mapping != null && 
 						mapping.getServletName() != null && 
-						mapping.getServletName()
+						mapping.getServletName().trim()
 						.equals(servletName)) {
 					mappings.remove(mapping);
 				}
@@ -238,8 +253,8 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param webApp
 	 * @param config
 	 */
-	public static void setupConfigFileContextParamForV2_5(org.eclipse.jst.javaee.web.WebApp webApp,
-			IDataModel config) {
+	public static void setupConfigFileContextParamForV2_5(final org.eclipse.jst.javaee.web.WebApp webApp,
+			final IDataModel config) {
 		// if not default name and location, then add context param
 		ParamValue foundCP = null;
 		ParamValue cp = null;
@@ -251,7 +266,7 @@ public class JSFUtils12 extends JSFUtils {
 				cp = (org.eclipse.jst.javaee.core.ParamValue) it.next();
 				if (cp != null && 
 						cp.getParamName()!= null &&
-						cp.getParamName().equals(JSF_CONFIG_CONTEXT_PARAM)) {
+						cp.getParamName().trim().equals(JSF_CONFIG_CONTEXT_PARAM)) {
 					foundCP = cp;
 					found = true;
 				}
@@ -279,13 +294,13 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param webApp
 	 * @return the default file extension from the context param.  Default is "jsp" if no context param
 	 */
-	public static String getDefaultSuffix(WebApp webApp) {
+	public static String getDefaultSuffix(final WebApp webApp) {
 		String defaultSuffix = "jsp"; //$NON-NLS-1$
 		for (Iterator it = webApp.getContextParams().iterator();it.hasNext();) {		
 			ParamValue cp = (ParamValue) it.next();		
 			if (cp != null && 
 					cp.getParamName() != null && 
-					cp.getParamName().equals(JSF_DEFAULT_SUFFIX_CONTEXT_PARAM)){				
+					cp.getParamName().trim().equals(JSF_DEFAULT_SUFFIX_CONTEXT_PARAM)){				
 				String defSuffix = cp.getParamValue();
 				if (defSuffix.startsWith(".")) //$NON-NLS-1$
 					defSuffix = defSuffix.substring(1);
@@ -300,7 +315,7 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param map
 	 * @return prefix mapping 
 	 */
-	public static String getPrefixMapping(ServletMapping map) {
+	public static String getPrefixMapping(final ServletMapping map) {
 		List urls = map.getUrlPatterns();
 		for (Iterator it=urls.iterator();it.hasNext();){
 			IPath extPath = new Path(((UrlPatternType)it.next()).getValue());
@@ -324,7 +339,7 @@ public class JSFUtils12 extends JSFUtils {
 	 * @param map
 	 * @return extension from map.  Will return null if file extension not found in url patterns.
 	 */
-	public static String getFileExtensionFromMap(ServletMapping map) {
+	public static String getFileExtensionFromMap(final ServletMapping map) {
 		List urls = map.getUrlPatterns();
 		for (Iterator it=urls.iterator();it.hasNext();){
 			IPath extPath = new Path(((UrlPatternType)it.next()).getValue());