[402635] Bugfix: correctly compare URI fragments (including slashes) Change-Id: Iea0eb2a68e52f88879b0eff9af6906a635995cd8 Signed-off-by: Dietrich Travkin <travkin@gmx.de>
diff --git a/plugins/org.eclipse.sphinx.emf.validation/src/org/eclipse/sphinx/emf/validation/markers/ValidationMarkerManager.java b/plugins/org.eclipse.sphinx.emf.validation/src/org/eclipse/sphinx/emf/validation/markers/ValidationMarkerManager.java index e0dc814..bdf33a4 100644 --- a/plugins/org.eclipse.sphinx.emf.validation/src/org/eclipse/sphinx/emf/validation/markers/ValidationMarkerManager.java +++ b/plugins/org.eclipse.sphinx.emf.validation/src/org/eclipse/sphinx/emf/validation/markers/ValidationMarkerManager.java
@@ -63,6 +63,8 @@ */ public class ValidationMarkerManager { + private static final String URI_FRAGMENT_PARTS_SEPARATOR = "/"; //$NON-NLS-1$ + /** * The singleton instance */ @@ -475,37 +477,10 @@ if (tmp != null && tmp.length > 0) { markerURI = tmp[0]; markerEObjType = tmp.length > 1 ? tmp[1] : null; - Integer hash = (Integer) marker.getAttribute(IValidationMarker.HASH_ATTRIBUTE); - switch (depth) { - case EObjectUtil.DEPTH_ZERO: - // FIXME Remove hash code comparison by ResourceSetListener that deletes associated problem - // markers when model file gets deleted or reloaded (see ResourceProblemHandler for details) - if (URI.createURI(markerURI).fragment().equals(URI.createURI(eObjURI).fragment()) - && (markerEObjType == null || eObjType.equals(markerEObjType))) { - result.add(marker); - } - break; - case EObjectUtil.DEPTH_ONE: // same treatment, let's modify it later if necessary - boolean shouldbeDeleted = true; - shouldbeDeleted &= markerURI.contains(eObjURI); - if (shouldbeDeleted) { - String subPart = markerURI.substring(eObjURI.length()); - shouldbeDeleted &= subPart.indexOf('/') == subPart.lastIndexOf('/'); - } - - if (shouldbeDeleted) { - result.add(marker); - } - break; - case EObjectUtil.DEPTH_INFINITE: - if (markerURI.equals(eObjURI) || markerURI.contains(eObjURI + "/")) { //$NON-NLS-1$ - result.add(marker); - } - break; - default: - // Something is rotten in my kingdom, do nothing - break; + boolean showMarker = showMarker(eObjURI, markerURI, eObjType, markerEObjType, depth); + if (showMarker) { + result.add(marker); } } } else { @@ -519,6 +494,39 @@ } /** + * Internal method, only public to allow testing + */ + public boolean showMarker(String selectedObjectUri, String markerUri, String selectedEObjectType, String markerEObjType, int depth) { + if (depth == EObjectUtil.DEPTH_ZERO) { + return markerUri.equals(selectedObjectUri) && (markerEObjType == null || selectedEObjectType.equals(markerEObjType)); + } + + String selectedObjectUriFragment = URI.createURI(selectedObjectUri).fragment(); + String markerUriFragment = URI.createURI(markerUri).fragment(); + + String[] elementUriFragmentParts = selectedObjectUriFragment.split(URI_FRAGMENT_PARTS_SEPARATOR); + String[] markerUriFragmentParts = markerUriFragment.split(URI_FRAGMENT_PARTS_SEPARATOR); + + int indexOfLastElementUriFragmentPart = elementUriFragmentParts.length - 1; + + // elementURI is prefix of markerURI + boolean showMarker = markerUri.startsWith(selectedObjectUri) + // last fragment part (String after last '/') has to be equal, + // not just prefix of the other String + // e.g. fragments "abc/xyz2" and "abc/xyz2/z" are not children of fragment "abc/xyz" + && elementUriFragmentParts[indexOfLastElementUriFragmentPart].equals(markerUriFragmentParts[indexOfLastElementUriFragmentPart]); + + switch (depth) { + case EObjectUtil.DEPTH_ONE: + return showMarker && elementUriFragmentParts.length + 1 >= markerUriFragmentParts.length; + case EObjectUtil.DEPTH_INFINITE: + return showMarker && elementUriFragmentParts.length <= markerUriFragmentParts.length; + default: + return true; + } + } + + /** * check if an array of IMarker contains a marker with the status IStatus.ERROR * * @param markers
diff --git a/tests/org.eclipse.sphinx.tests.emf.validation/src/org/eclipse/sphinx/emf/validation/markers/ValidationMarkerManagerUriComparisonTest.java b/tests/org.eclipse.sphinx.tests.emf.validation/src/org/eclipse/sphinx/emf/validation/markers/ValidationMarkerManagerUriComparisonTest.java new file mode 100644 index 0000000..e769564 --- /dev/null +++ b/tests/org.eclipse.sphinx.tests.emf.validation/src/org/eclipse/sphinx/emf/validation/markers/ValidationMarkerManagerUriComparisonTest.java
@@ -0,0 +1,85 @@ +package org.eclipse.sphinx.emf.validation.markers; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; + +import java.util.Arrays; +import java.util.List; + +import org.eclipse.sphinx.emf.util.EObjectUtil; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class ValidationMarkerManagerUriComparisonTest { + + private static final String SELECTED_ELEMENT_URI = "platform:/resource/validation-1/default.arxml#/ARRoot/Adc"; //$NON-NLS-1$ + + private String markerUri; + private boolean showMarkerForDepthZero, showMarkerForDepthOne, showMarkerForDepthInfinite; + private static ValidationMarkerManager markerManager; + + // EObjectUtil.DEPTH_ZERO, EObjectUtil.DEPTH_ONE, EObjectUtil.DEPTH_INFINITE + + @Parameterized.Parameters + public static List<Object[]> markerUriTestData() { + // | Marker URI | Show marker for DEPTH_ZERO | DEPTH_ONE | DEPTH_INFINITE | + return Arrays.asList(new Object[][] { { "platform:/resource/validation-1/default.arxml#/ARRoot/Adc", true, true, true }, //$NON-NLS-1$ + { "platform:/resource/validation-1/default.arxml#/ARRoot/Adc/AdcConfigSet", false, true, true }, //$NON-NLS-1$ + { "platform:/resource/validation-1/default.arxml#/ARRoot/Adc/AdcConfigSet/Abc", false, false, true }, //$NON-NLS-1$ + { "platform:/resource/validation-1/default.arxml#/ARRoot/Adc2", false, false, false }, //$NON-NLS-1$ + { "platform:/resource/validation-1/default.arxml#/ARRoot/Adc2/AdcConfigSet", false, false, false }, //$NON-NLS-1$ + { "platform:/resource/validation-1/default.arxml#/ARRoot/Adc2/AdcConfigSet/Abc", false, false, false } }); //$NON-NLS-1$ + } + + public ValidationMarkerManagerUriComparisonTest(String markerUri, boolean showMarkerForDepthZero, boolean showMarkerForDepthOne, + boolean showMarkerForDepthInfinite) { + this.markerUri = markerUri; + this.showMarkerForDepthZero = showMarkerForDepthZero; + this.showMarkerForDepthOne = showMarkerForDepthOne; + this.showMarkerForDepthInfinite = showMarkerForDepthInfinite; + } + + @BeforeClass + public static void setUpOnce() { + markerManager = ValidationMarkerManager.getInstance(); + } + + @Test + public void checkMarkerCorrectlyFilteredForDepthZero() { + int depth = EObjectUtil.DEPTH_ZERO; + + boolean showMarker = markerManager.showMarker(SELECTED_ELEMENT_URI, markerUri, "EObject", null, depth); //$NON-NLS-1$ + + assertThat(showMarker, is(showMarkerForDepthZero)); + + showMarker = markerManager.showMarker(SELECTED_ELEMENT_URI, markerUri, "EObject", "EObject", depth); //$NON-NLS-1$ //$NON-NLS-2$ + + assertThat(showMarker, is(showMarkerForDepthZero)); + + showMarker = markerManager.showMarker(SELECTED_ELEMENT_URI, markerUri, "EObject", "OtherType", depth); //$NON-NLS-1$ //$NON-NLS-2$ + + assertThat(showMarker, is(false)); + } + + @Test + public void checkMarkerCorrectlyFilteredForDepthOne() { + int depth = EObjectUtil.DEPTH_ONE; + + boolean showMarker = markerManager.showMarker(SELECTED_ELEMENT_URI, markerUri, "EObject", null, depth); //$NON-NLS-1$ + + assertThat(showMarker, is(showMarkerForDepthOne)); + } + + @Test + public void checkMarkerCorrectlyFilteredForDepthInfinite() { + int depth = EObjectUtil.DEPTH_INFINITE; + + boolean showMarker = markerManager.showMarker(SELECTED_ELEMENT_URI, markerUri, "EObject", null, depth); //$NON-NLS-1$ + + assertThat(showMarker, is(showMarkerForDepthInfinite)); + } + +}