From 25e75b5a6eac9f5353b76aef114c47ead13a1392 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Fri, 24 Nov 2017 14:40:21 +0000 Subject: [PATCH] JAL-2808 refine FeatureMatcher interface and matcher toString methods --- src/jalview/datamodel/features/FeatureMatcher.java | 37 +++++++++++-- .../datamodel/features/FeatureMatcherI.java | 19 ++++++- .../datamodel/features/FeatureMatcherSet.java | 10 +++- src/jalview/util/matcher/Matcher.java | 44 +++++++++------ .../datamodel/features/FeatureMatcherSetTest.java | 13 ++--- .../datamodel/features/FeatureMatcherTest.java | 57 ++++++++++++++++++-- test/jalview/util/matcher/MatcherTest.java | 15 ++++-- 7 files changed, 159 insertions(+), 36 deletions(-) diff --git a/src/jalview/datamodel/features/FeatureMatcher.java b/src/jalview/datamodel/features/FeatureMatcher.java index 1fc0e0f..b86468d 100644 --- a/src/jalview/datamodel/features/FeatureMatcher.java +++ b/src/jalview/datamodel/features/FeatureMatcher.java @@ -1,6 +1,7 @@ package jalview.datamodel.features; import jalview.datamodel.SequenceFeature; +import jalview.util.MessageManager; import jalview.util.matcher.Condition; import jalview.util.matcher.Matcher; import jalview.util.matcher.MatcherI; @@ -22,6 +23,12 @@ import jalview.util.matcher.MatcherI; */ public class FeatureMatcher implements FeatureMatcherI { + /* + * a dummy matcher that comes in useful for the 'add a filter' gui row + */ + public static final FeatureMatcherI NULL_MATCHER = FeatureMatcher + .byLabel(Condition.values()[0], ""); + private static final String COLON = ":"; /* @@ -103,7 +110,7 @@ public class FeatureMatcher implements FeatureMatcherI } @Override - public String[] getKey() + public String[] getAttribute() { return key; } @@ -122,9 +129,21 @@ public class FeatureMatcher implements FeatureMatcherI public String toString() { StringBuilder sb = new StringBuilder(); - sb.append(String.join(COLON, key)).append(" ") - .append(matcher.getCondition().toString()); + if (byLabel) + { + sb.append(MessageManager.getString("label.label")); + } + else if (byScore) + { + sb.append(MessageManager.getString("label.score")); + } + else + { + sb.append(String.join(COLON, key)); + } + Condition condition = matcher.getCondition(); + sb.append(" ").append(condition.toString().toLowerCase()); if (condition.isNumeric()) { sb.append(" ").append(matcher.getPattern()); @@ -136,4 +155,16 @@ public class FeatureMatcher implements FeatureMatcherI return sb.toString(); } + + @Override + public boolean isByLabel() + { + return byLabel; + } + + @Override + public boolean isByScore() + { + return byScore; + } } diff --git a/src/jalview/datamodel/features/FeatureMatcherI.java b/src/jalview/datamodel/features/FeatureMatcherI.java index 078f4a4..07b060c 100644 --- a/src/jalview/datamodel/features/FeatureMatcherI.java +++ b/src/jalview/datamodel/features/FeatureMatcherI.java @@ -21,11 +21,26 @@ public interface FeatureMatcherI boolean matches(SequenceFeature feature); /** - * Answers the value key this matcher operates on + * Answers the attribute key this matcher operates on (or null if match is by + * Label or Score) * * @return */ - String[] getKey(); + String[] getAttribute(); + + /** + * Answers true if match is against feature label (description), else false + * + * @return + */ + boolean isByLabel(); + + /** + * Answers true if match is against feature score, else false + * + * @return + */ + boolean isByScore(); /** * Answers the match condition that is applied diff --git a/src/jalview/datamodel/features/FeatureMatcherSet.java b/src/jalview/datamodel/features/FeatureMatcherSet.java index 64ae61b..eb55387 100644 --- a/src/jalview/datamodel/features/FeatureMatcherSet.java +++ b/src/jalview/datamodel/features/FeatureMatcherSet.java @@ -1,12 +1,19 @@ package jalview.datamodel.features; import jalview.datamodel.SequenceFeature; +import jalview.util.MessageManager; import java.util.ArrayList; import java.util.List; public class FeatureMatcherSet implements FeatureMatcherSetI { + private static final String OR_I18N = MessageManager + .getString("label.or"); + + private static final String AND_18N = MessageManager + .getString("label.and"); + List matchConditions; boolean andConditions; @@ -105,7 +112,8 @@ public class FeatureMatcherSet implements FeatureMatcherSetI { if (!first) { - sb.append(andConditions ? " AND " : " OR "); + String joiner = andConditions ? AND_18N : OR_I18N; + sb.append(" ").append(joiner.toLowerCase()).append(" "); } first = false; sb.append("(").append(matcher.toString()).append(")"); diff --git a/src/jalview/util/matcher/Matcher.java b/src/jalview/util/matcher/Matcher.java index 14a8585..353df83 100644 --- a/src/jalview/util/matcher/Matcher.java +++ b/src/jalview/util/matcher/Matcher.java @@ -14,12 +14,17 @@ public class Matcher implements MatcherI Condition condition; /* - * the string value (upper-cased), or the regex, to compare to + * the string pattern as entered, or the regex, to compare to * also holds the string form of float value if a numeric condition */ String pattern; /* + * the pattern in upper case, for non-case-sensitive matching + */ + String uppercasePattern; + + /* * the compiled regex if using a pattern match condition * (reserved for possible future enhancement) */ @@ -44,16 +49,21 @@ public class Matcher implements MatcherI */ public Matcher(Condition cond, String compareTo) { + Objects.requireNonNull(cond); condition = cond; if (cond.isNumeric()) { value = Float.valueOf(compareTo); pattern = String.valueOf(value); + uppercasePattern = pattern; } else { - // pattern matches will be non-case-sensitive - pattern = compareTo == null ? null : compareTo.toUpperCase(); + pattern = compareTo; + if (pattern != null) + { + uppercasePattern = pattern.toUpperCase(); + } } // if we add regex conditions (e.g. matchesPattern), then @@ -71,10 +81,7 @@ public class Matcher implements MatcherI */ public Matcher(Condition cond, float compareTo) { - Objects.requireNonNull(cond); - condition = cond; - value = compareTo; - pattern = String.valueOf(compareTo).toUpperCase(); + this(cond, String.valueOf(compareTo)); } /** @@ -113,16 +120,16 @@ public class Matcher implements MatcherI boolean matched = false; switch(condition) { case Matches: - matched = upper.equals(pattern); + matched = upper.equals(uppercasePattern); break; case NotMatches: - matched = !upper.equals(pattern); + matched = !upper.equals(uppercasePattern); break; case Contains: - matched = upper.indexOf(pattern) > -1; + matched = upper.indexOf(uppercasePattern) > -1; break; case NotContains: - matched = upper.indexOf(pattern) == -1; + matched = upper.indexOf(uppercasePattern) == -1; break; case Present: matched = true; @@ -186,7 +193,7 @@ public class Matcher implements MatcherI /** * equals is overridden so that we can safely remove Matcher objects from - * collections (e.g. delete an attribut match condition for a feature colour) + * collections (e.g. delete an attribute match condition for a feature colour) */ @Override public boolean equals(Object obj) @@ -196,8 +203,15 @@ public class Matcher implements MatcherI return false; } Matcher m = (Matcher) obj; - return condition == m.condition && value == m.value - && pattern.equals(m.pattern); + if (condition != m.condition || value != m.value) + { + return false; + } + if (pattern == null) + { + return m.pattern == null; + } + return uppercasePattern.equals(m.uppercasePattern); } @Override @@ -222,7 +236,7 @@ public class Matcher implements MatcherI public String toString() { StringBuilder sb = new StringBuilder(); - sb.append(condition.name()).append(" "); + sb.append(condition.toString()).append(" "); if (condition.isNumeric()) { sb.append(pattern); diff --git a/test/jalview/datamodel/features/FeatureMatcherSetTest.java b/test/jalview/datamodel/features/FeatureMatcherSetTest.java index a98013b..56644fd 100644 --- a/test/jalview/datamodel/features/FeatureMatcherSetTest.java +++ b/test/jalview/datamodel/features/FeatureMatcherSetTest.java @@ -11,6 +11,7 @@ import jalview.util.matcher.Condition; import java.util.HashMap; import java.util.Iterator; +import java.util.Locale; import java.util.Map; import org.testng.annotations.Test; @@ -111,14 +112,14 @@ public class FeatureMatcherSetTest @Test(groups = "Functional") public void testToString() { + Locale.setDefault(Locale.ENGLISH); FeatureMatcherI fm1 = FeatureMatcher.byAttribute(Condition.LT, "1.2", "AF"); assertEquals(fm1.toString(), "AF < 1.2"); FeatureMatcher fm2 = FeatureMatcher.byAttribute(Condition.NotContains, - "path", - "CLIN_SIG"); - assertEquals(fm2.toString(), "CLIN_SIG Does not contain 'PATH'"); + "path", "CLIN_SIG"); + assertEquals(fm2.toString(), "CLIN_SIG does not contain 'path'"); /* * AND them @@ -129,7 +130,7 @@ public class FeatureMatcherSetTest assertEquals(fms.toString(), "(AF < 1.2)"); fms.and(fm2); assertEquals(fms.toString(), - "(AF < 1.2) AND (CLIN_SIG Does not contain 'PATH')"); + "(AF < 1.2) and (CLIN_SIG does not contain 'path')"); /* * OR them @@ -140,7 +141,7 @@ public class FeatureMatcherSetTest assertEquals(fms.toString(), "(AF < 1.2)"); fms.or(fm2); assertEquals(fms.toString(), - "(AF < 1.2) OR (CLIN_SIG Does not contain 'PATH')"); + "(AF < 1.2) or (CLIN_SIG does not contain 'path')"); try { @@ -265,7 +266,7 @@ public class FeatureMatcherSetTest assertFalse(fms.matches(sf)); csq.put("Consequence", "junk"); assertFalse(fms.matches(sf)); - + /* * a string pattern matcher */ diff --git a/test/jalview/datamodel/features/FeatureMatcherTest.java b/test/jalview/datamodel/features/FeatureMatcherTest.java index f4e9351..62b03a3 100644 --- a/test/jalview/datamodel/features/FeatureMatcherTest.java +++ b/test/jalview/datamodel/features/FeatureMatcherTest.java @@ -2,11 +2,15 @@ package jalview.datamodel.features; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; import jalview.datamodel.SequenceFeature; +import jalview.util.MessageManager; import jalview.util.matcher.Condition; +import java.util.Locale; + import org.testng.annotations.Test; public class FeatureMatcherTest @@ -126,6 +130,8 @@ public class FeatureMatcherTest @Test public void testToString() { + Locale.setDefault(Locale.ENGLISH); + /* * toString uses the i18n translation of the enum conditions */ @@ -137,24 +143,65 @@ public class FeatureMatcherTest * Present / NotPresent omit the value pattern */ fm = FeatureMatcher.byAttribute(Condition.Present, "", "AF"); - assertEquals(fm.toString(), "AF Is present"); + assertEquals(fm.toString(), "AF is present"); fm = FeatureMatcher.byAttribute(Condition.NotPresent, "", "AF"); - assertEquals(fm.toString(), "AF Is not present"); + assertEquals(fm.toString(), "AF is not present"); + + /* + * by Label + */ + fm = FeatureMatcher.byLabel(Condition.Matches, "foobar"); + assertEquals(fm.toString(), + MessageManager.getString("label.label") + " matches 'foobar'"); + + /* + * by Score + */ + fm = FeatureMatcher.byScore(Condition.GE, "12.2"); + assertEquals(fm.toString(), + MessageManager.getString("label.score") + " >= 12.2"); } @Test - public void testGetKey() + public void testGetAttribute() { FeatureMatcherI fm = FeatureMatcher.byAttribute(Condition.GE, "-2", "AF"); - assertEquals(fm.getKey(), new String[] { "AF" }); + assertEquals(fm.getAttribute(), new String[] { "AF" }); /* * compound key (attribute / subattribute) */ fm = FeatureMatcher.byAttribute(Condition.GE, "-2F", "CSQ", "Consequence"); - assertEquals(fm.getKey(), new String[] { "CSQ", "Consequence" }); + assertEquals(fm.getAttribute(), new String[] { "CSQ", "Consequence" }); + + /* + * answers null if match is by Label or by Score + */ + assertNull(FeatureMatcher.byLabel(Condition.NotContains, "foo") + .getAttribute()); + assertNull(FeatureMatcher.byScore(Condition.LE, "-1").getAttribute()); + } + + @Test + public void testIsByLabel() + { + assertTrue(FeatureMatcher.byLabel(Condition.NotContains, "foo") + .isByLabel()); + assertFalse(FeatureMatcher.byScore(Condition.LE, "-1").isByLabel()); + assertFalse(FeatureMatcher.byAttribute(Condition.LE, "-1", "AC") + .isByLabel()); + } + + @Test + public void testIsByScore() + { + assertFalse(FeatureMatcher.byLabel(Condition.NotContains, "foo") + .isByScore()); + assertTrue(FeatureMatcher.byScore(Condition.LE, "-1").isByScore()); + assertFalse(FeatureMatcher.byAttribute(Condition.LE, "-1", "AC") + .isByScore()); } @Test diff --git a/test/jalview/util/matcher/MatcherTest.java b/test/jalview/util/matcher/MatcherTest.java index f4c8181..a47fb60 100644 --- a/test/jalview/util/matcher/MatcherTest.java +++ b/test/jalview/util/matcher/MatcherTest.java @@ -6,8 +6,12 @@ import static org.testng.Assert.assertNotEquals; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; +import java.util.Locale; + import org.testng.annotations.Test; +import junit.extensions.PA; + public class MatcherTest { @Test(groups = "Functional") @@ -15,7 +19,8 @@ public class MatcherTest { MatcherI m = new Matcher(Condition.Contains, "foo"); assertEquals(m.getCondition(), Condition.Contains); - assertEquals(m.getPattern(), "FOO"); // all comparisons upper-cased + assertEquals(m.getPattern(), "foo"); + assertEquals(PA.getValue(m, "uppercasePattern"), "FOO"); assertEquals(m.getFloatValue(), 0f); m = new Matcher(Condition.GT, -2.1f); @@ -25,7 +30,7 @@ public class MatcherTest m = new Matcher(Condition.NotContains, "-1.2f"); assertEquals(m.getCondition(), Condition.NotContains); - assertEquals(m.getPattern(), "-1.2F"); + assertEquals(m.getPattern(), "-1.2f"); assertEquals(m.getFloatValue(), 0f); m = new Matcher(Condition.GE, "-1.2f"); @@ -208,11 +213,13 @@ public class MatcherTest @Test(groups = "Functional") public void testToString() { + Locale.setDefault(Locale.ENGLISH); + MatcherI m = new Matcher(Condition.LT, 1.2e-6f); - assertEquals(m.toString(), "LT 1.2E-6"); + assertEquals(m.toString(), "< 1.2E-6"); m = new Matcher(Condition.NotMatches, "ABC"); - assertEquals(m.toString(), "NotMatches 'ABC'"); + assertEquals(m.toString(), "Does not match 'ABC'"); m = new Matcher(Condition.Contains, -1.2f); assertEquals(m.toString(), "Contains '-1.2'"); -- 1.7.10.2