fix style selection menu displays styles twice

When the doctype reference in a style definition matched public id and
system id of a doctype, the style was displayed twice in the styles
menu.

Change-Id: I846e9ad5d158c074c8e1e054809a0b50dc05fdf6
Signed-off-by: Carsten Hiesserich <carsten.hie@gmail.com>

4 files changed, 128 insertions, 26 deletions

diff --git a/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/ConfigurationRegistryTest.java b/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/ConfigurationRegistryTest.java
index c84a4590..85520f96 100644
--- a/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/ConfigurationRegistryTest.java
+++ b/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/ConfigurationRegistryTest.java
@@ -103,7 +103,7 @@ public class ConfigurationRegistryTest {
 		project.getFile("plugintest2.css").create(new ByteArrayInputStream(new byte[0]), true, null);

 		final String fileContent = PluginProjectTest.createVexPluginFileContent(project, "plugintest.dtd", "plugintest.css", "plugintest2.css");

 		registry.addConfigListener(configListener);

-		project.getFile(PluginProject.PLUGIN_XML).setContents(new ByteArrayInputStream(fileContent.getBytes()), true, true, null);

+		PluginProjectTest.writePluginFile(project, fileContent, true);

 		assertFalse(configListener.loaded);

 		assertTrue(configListener.changed);

 		assertNotNull(registry.getPluginProject(project).getItemForResource(project.getFile("plugintest2.css")));

diff --git a/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginProjectTest.java b/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginProjectTest.java
index ff0e63b9..e652ccf4 100644
--- a/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginProjectTest.java
+++ b/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginProjectTest.java
@@ -47,7 +47,7 @@ public class PluginProjectTest {
 

 	public static void createVexPluginFile(final IProject project) throws CoreException {

 		final String fileContent = createVexPluginFileContent(project);

-		project.getFile(PluginProject.PLUGIN_XML).create(new ByteArrayInputStream(fileContent.getBytes()), true, null);

+		writePluginFile(project, fileContent, false);

 	}

 

 	public static String createVexPluginFileContent(final IProject project) {

@@ -57,9 +57,7 @@ public class PluginProjectTest {
 	public static String createVexPluginFileContent(final IProject project, final String dtdFilename, final String... styleFilenames) {

 		final StringWriter result = new StringWriter();

 		final PrintWriter out = new PrintWriter(result);

-		out.println("<?xml version='1.0'?>"); //$NON-NLS-1$

-		// HINT: It is important to set the id attribute, because this is used as the unique identifier for the configuration.

-		out.println("<plugin id=\"" + project.getName() + "\">"); //$NON-NLS-1$ //$NON-NLS-2$

+

 		out.println("<extension id=\"plugintest\" name=\"" + DTD_FILE_NAME + "\" point=\"org.eclipse.vex.ui.doctypes\">"); //$NON-NLS-1$ //$NON-NLS-2$

 		out.println("<doctype systemId=\"" + dtdFilename + "\" dtd=\"" + dtdFilename + "\" publicId=\"" + DTD_DOCTYPE_ID + "\" />"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$

 		out.println("</extension>"); //$NON-NLS-1$

@@ -68,11 +66,27 @@ public class PluginProjectTest {
 			out.println("<style css=\"" + styleFilename + "\"><doctypeRef publicId=\"" + DTD_DOCTYPE_ID + "\" /></style>"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$

 			out.println("</extension>"); //$NON-NLS-1$

 		}

-		out.println("</plugin>"); //$NON-NLS-1$

+

 		out.close();

 		return result.toString();

 	}

 

+	public static void writePluginFile(final IProject project, final String content, final boolean replace) throws CoreException {

+		final StringWriter result = new StringWriter();

+		final PrintWriter out = new PrintWriter(result);

+		out.println("<?xml version='1.0'?>");

+		// HINT: It is important to set the id attribute, because this is used as the unique identifier for the configuration.

+		out.println("<plugin id=\"" + project.getName() + "\">");

+		out.print(content);

+		out.println("</plugin>"); //$NON-NLS-1$

+		out.close();

+		if (!replace) {

+			project.getFile(PluginProject.PLUGIN_XML).create(new ByteArrayInputStream(result.toString().getBytes()), true, null);

+		} else {

+			project.getFile(PluginProject.PLUGIN_XML).setContents(new ByteArrayInputStream(result.toString().getBytes()), true, true, null);

+		}

+	}

+

 	public static void addVexProjectNature(final IProject project) throws CoreException {

 		final IProjectDescription description = project.getDescription();

 		final String[] natures = description.getNatureIds();

diff --git a/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginStyleConfigTest.java b/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginStyleConfigTest.java
index 3dd28dba..708ec979 100644
--- a/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginStyleConfigTest.java
+++ b/org.eclipse.vex.ui.tests/src/org/eclipse/vex/ui/internal/config/tests/PluginStyleConfigTest.java
@@ -20,6 +20,7 @@ import java.util.ArrayList;
 
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IProject;
+import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Shell;
@@ -43,18 +44,18 @@ import org.junit.Test;
  */
 public class PluginStyleConfigTest {
 
-	private IProject pluginProject;
+	private IProject project;
 	private ConfigurationRegistry configurationRegistry;
 
 	@After
 	public void dispose() {
-		if (pluginProject != null) {
+		if (project != null) {
 			try {
-				pluginProject.delete(true, null);
+				project.delete(true, null);
 			} catch (final CoreException e) {
 			}
 		}
-		pluginProject = null;
+		project = null;
 
 		if (configurationRegistry != null) {
 			configurationRegistry.dispose();
@@ -67,13 +68,13 @@ public class PluginStyleConfigTest {
 	 */
 	@Test
 	public void testCssPropertyPage() throws CoreException, NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException {
-		pluginProject = PluginProjectTest.createVexPluginProject("PropertyPageTest");
+		project = PluginProjectTest.createVexPluginProject("PropertyPageTest");
 
 		final Display display = Display.getCurrent();
 		final Shell shell = new Shell(display);
 		final StylePropertyPage page = new StylePropertyPage();
 
-		final IFile cssFile = pluginProject.getFile(PluginProjectTest.CSS_FILE_NAME);
+		final IFile cssFile = project.getFile(PluginProjectTest.CSS_FILE_NAME);
 		page.setElement(cssFile);
 		page.createControl(shell);
 
@@ -95,11 +96,11 @@ public class PluginStyleConfigTest {
 
 	@Test
 	public void testDoctypeStyle() throws Exception {
-		pluginProject = PluginProjectTest.createVexPluginProject("PropertyPageTest");
+		project = PluginProjectTest.createVexPluginProject("PropertyPageTest");
 
 		configurationRegistry = new ConfigurationRegistryImpl(new ConfigLoaderJob());
 		configurationRegistry.loadConfigurations();
-		assertNotNull(configurationRegistry.getPluginProject(pluginProject));
+		assertNotNull(configurationRegistry.getPluginProject(project));
 
 		final DocumentType doctype = configurationRegistry.getDocumentType(PluginProjectTest.DTD_DOCTYPE_ID, null);
 		assertNotNull(doctype);
@@ -111,14 +112,14 @@ public class PluginStyleConfigTest {
 
 	@Test
 	public void testAddStyle() throws Exception {
-		pluginProject = PluginProjectTest.createVexPluginProject("PropertyPageTest");
+		project = PluginProjectTest.createVexPluginProject("PropertyPageTest");
 
 		final Display display = Display.getCurrent();
 		final Shell shell = new Shell(display);
 		final StylePropertyPage page = new StylePropertyPage();
 
 		// Create a new .css file and open the property page
-		final IFile file = pluginProject.getFile("test_new.css");
+		final IFile file = project.getFile("test_new.css");
 		file.create(new ByteArrayInputStream(new byte[0]), true, null);
 		page.setElement(file);
 		page.createControl(shell);
@@ -144,8 +145,8 @@ public class PluginStyleConfigTest {
 		page.dispose();
 
 		// Reload the project
-		pluginProject.close(null);
-		pluginProject.open(null);
+		project.close(null);
+		project.open(null);
 
 		//
 		configurationRegistry = new ConfigurationRegistryImpl(new ConfigLoaderJob());
@@ -159,7 +160,97 @@ public class PluginStyleConfigTest {
 			}
 		}
 
-		assertNotNull("New style should be present an apply to the selected doctype", newStyle);
+		assertNotNull("New style should be present and apply to the selected doctype", newStyle);
 		assertEquals("test_new.css", newStyle.getResourceUri().toString());
 	}
+
+	@Test
+	public void testGetStyles1() throws Exception {
+		project = createStyleTestProject("testGetStyles", getStyle1Css());
+
+		configurationRegistry = new ConfigurationRegistryImpl(new ConfigLoaderJob());
+		configurationRegistry.loadConfigurations();
+
+		final DocumentType doctype = configurationRegistry.getDocumentType("test_pubid", null);
+		assertNotNull(doctype);
+
+		final Style[] styles = configurationRegistry.getStyles(doctype);
+		assertEquals("Style should be returned only once, even when multiple id's match", 1, styles.length);
+		assertEquals("plugin test style", styles[0].getName());
+	}
+
+	@Test
+	public void testGetStyles2() throws Exception {
+		project = createStyleTestProject("testGetStyles", getStyle2Css());
+
+		configurationRegistry = new ConfigurationRegistryImpl(new ConfigLoaderJob());
+		configurationRegistry.loadConfigurations();
+
+		final DocumentType doctype = configurationRegistry.getDocumentType("test_pubid", null);
+		assertNotNull(doctype);
+
+		final Style[] styles = configurationRegistry.getStyles(doctype);
+		assertEquals("Both defined styles should be returned.", 2, styles.length);
+		assertEquals("plugin test style1", styles[0].getName());
+		assertEquals("plugin test style2", styles[1].getName());
+	}
+
+	/**
+	 * One style with two doctype references for the same doctype.
+	 */
+	private String getStyle1Css() {
+		final StringBuilder sb = new StringBuilder();
+		sb.append("<extension id=\"plugintest\" name=\"plugin test style\" point=\"org.eclipse.vex.ui.styles\">");
+		sb.append("<style css=\"test_style1.css\">");
+		sb.append("<doctypeRef doctypeId=\"test_sysid\" />");
+		sb.append("<doctypeRef doctypeId=\"test_pubid\" />");
+		sb.append("</style>");
+		sb.append("</extension>");
+		return sb.toString();
+	}
+
+	/**
+	 * Two different styles for one doctype.
+	 */
+	private String getStyle2Css() {
+		final StringBuilder sb = new StringBuilder();
+		sb.append("<extension id=\"plugintest1\" name=\"plugin test style1\" point=\"org.eclipse.vex.ui.styles\">");
+		sb.append("<style css=\"test_style1.css\">");
+		sb.append("<doctypeRef doctypeId=\"test_sysid\" />");
+		sb.append("<doctypeRef doctypeId=\"test_pubid\" />");
+		sb.append("</style>");
+		sb.append("</extension>");
+
+		sb.append("<extension id=\"plugintest2\" name=\"plugin test style2\" point=\"org.eclipse.vex.ui.styles\">");
+		sb.append("<style css=\"test_style2.css\">");
+		sb.append("<doctypeRef doctypeId=\"test_sysid\" />");
+		sb.append("<doctypeRef doctypeId=\"test_pubid\" />");
+		sb.append("</style>");
+		sb.append("</extension>");
+
+		return sb.toString();
+	}
+
+	private IProject createStyleTestProject(final String name, final String cssConfig) throws CoreException {
+		final IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject(name);
+		if (project.exists()) {
+			project.delete(true, true, null);
+		}
+		project.create(null);
+		project.open(null);
+		project.getFile(name + ".dtd").create(new ByteArrayInputStream(new byte[0]), true, null);
+		project.getFile(name + ".css").create(new ByteArrayInputStream(new byte[0]), true, null);
+
+		final StringBuilder sb = new StringBuilder();
+		sb.append("<extension id=\"plugintest\" name=\"" + name + "\" point=\"org.eclipse.vex.ui.doctypes\">");
+		sb.append("<doctype systemId=\"test_sysid\" dtd=\"" + name + ".dtd\" publicId=\"test_pubid\" />");
+		sb.append("</extension>");
+
+		sb.append(cssConfig);
+
+		PluginProjectTest.writePluginFile(project, sb.toString(), false);
+		PluginProjectTest.addVexProjectNature(project);
+		return project;
+	}
+
 }
diff --git a/org.eclipse.vex.ui/src/org/eclipse/vex/ui/internal/config/ConfigurationRegistryImpl.java b/org.eclipse.vex.ui/src/org/eclipse/vex/ui/internal/config/ConfigurationRegistryImpl.java
index 1911e6c1..b2b02bb2 100644
--- a/org.eclipse.vex.ui/src/org/eclipse/vex/ui/internal/config/ConfigurationRegistryImpl.java
+++ b/org.eclipse.vex.ui/src/org/eclipse/vex/ui/internal/config/ConfigurationRegistryImpl.java
@@ -260,19 +260,16 @@ public class ConfigurationRegistryImpl implements ConfigurationRegistry {
 			final Style style = (Style) configItem;
 			if (style.appliesTo(doctype.getSimpleId())) {
 				resultId.add(style);
-			}
-			if (!doctype.isBlank(doctype.getPublicId()) && style.appliesTo(doctype.getPublicId())) {
+			} else if (!doctype.isBlank(doctype.getPublicId()) && style.appliesTo(doctype.getPublicId())) {
 				resultPublic.add(style);
-			}
-			if (!doctype.isBlank(doctype.getSystemId()) && style.appliesTo(doctype.getSystemId())) {
+			} else if (!doctype.isBlank(doctype.getSystemId()) && style.appliesTo(doctype.getSystemId())) {
 				resultSystem.add(style);
-			}
-			if (!doctype.isBlank(doctype.getNamespaceName()) && style.appliesTo(doctype.getNamespaceName())) {
+			} else if (!doctype.isBlank(doctype.getNamespaceName()) && style.appliesTo(doctype.getNamespaceName())) {
 				resultSchema.add(style);
 			}
 		}
 
-		// The resolved stylesheet are returned in a defined order
+		// The resolved stylesheets are returned in a defined order
 		final ArrayList<Style> result = new ArrayList<Style>();
 		result.addAll(resultId);
 		result.addAll(resultPublic);