Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Comparing changes

Choose two branches to see what's changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
...
Checking mergeability… Don't worry, you can still create the pull request.
  • 3 commits
  • 4 files changed
  • 0 commit comments
  • 1 contributor
View
100 server/zanata-war/src/main/java/org/zanata/webtrans/shared/validation/action/HtmlXmlTagValidation.java
@@ -21,7 +21,7 @@
package org.zanata.webtrans.shared.validation.action;
import java.util.ArrayList;
-import java.util.List;
+//import java.util.List;
import org.zanata.webtrans.client.resources.ValidationMessages;
import org.zanata.webtrans.shared.validation.ValidationUtils;
@@ -52,52 +52,108 @@ public void validate(String source, String target)
{
if (!ValidationUtils.isEmpty(target))
{
- List<String> error = runValidation(source, target);
+ ArrayList<String> sourceTags = getTagList(source);
+ ArrayList<String> targetTags = getTagList(target);
+
+ ArrayList<String> error = listMissing(source, target);
if (!error.isEmpty())
{
-
addError(getMessages().tagsMissing(error));
}
- error = runValidation(target, source);
+ boolean noError = error.isEmpty();
+
+ error = listMissing(target, source);
if (!error.isEmpty())
{
addError(getMessages().tagsAdded(error));
}
- if (getError().isEmpty())
+ noError &= error.isEmpty();
+
+ if (noError)
{
- orderValidation(source, target);
+ orderValidation(sourceTags, targetTags);
}
}
}
- private void orderValidation(String source, String target)
+ private void orderValidation(ArrayList<String> srcTags, ArrayList<String> trgTags)
{
- // TODO improve for cases such as first node moved to end and last node
- // moved to start. Currently reports every node in these cases, should
- // only report the one moved node.
- List<String> from = getTagList(source);
- List<String> to = getTagList(target);
- List<String> outOfOrder = new ArrayList<String>();
-
- for (int i = 0; i < from.size(); i++)
+ ArrayList<String> longestRun = null;
+ ArrayList<String> currentRun = new ArrayList<String>();
+
+ String[] src = srcTags.toArray(new String[srcTags.size()]);
+ String[] trg = trgTags.toArray(new String[trgTags.size()]);
+
+ for (int i = 0; i < src.length; i++)
{
- if (!to.get(i).equals(from.get(i)))
+ String token = src[i];
+ int srcIndex = i;
+ int trgIndex = trgTags.indexOf(token);
+
+ if (trgIndex > -1)
{
- outOfOrder.add(from.get(i));
+ currentRun = new ArrayList<String>();
+ currentRun.add(token);
+
+ int j = trgIndex + 1;
+
+ while (j < trg.length && srcIndex < src.length - 1)
+ {
+ int nextIndexInSrc = findInTail(trg[j], src, srcIndex + 1);
+ if (nextIndexInSrc > -1)
+ {
+ srcIndex = nextIndexInSrc;
+ currentRun.add(src[srcIndex]);
+ }
+ j++;
+ }
+
+ if (currentRun.size() == srcTags.size())
+ {
+ // must all match
+ return;
+ }
+
+ if (longestRun == null || longestRun.size() < currentRun.size())
+ {
+ longestRun = currentRun;
+ }
}
}
- if (!outOfOrder.isEmpty())
+ if (longestRun != null && longestRun.size() > 0)
{
+ ArrayList<String> outOfOrder = new ArrayList<String>();
+
+ for (int i = 0; i < src.length; i++)
+ {
+ if (!longestRun.contains(src[i]))
+ {
+ outOfOrder.add(src[i]);
+ }
+ }
+
addError(getMessages().tagsWrongOrder(outOfOrder));
}
}
- private List<String> getTagList(String src)
+ private int findInTail(String toFind, String[] findIn, int startIndex)
+ {
+ for (int i = startIndex; i < findIn.length; i++)
+ {
+ if (findIn[i].equals(toFind))
+ {
+ return i;
+ }
+ }
+ return -1;
+ }
+
+ private ArrayList<String> getTagList(String src)
{
- List<String> list = new ArrayList<String>();
+ ArrayList<String> list = new ArrayList<String>();
MatchResult result = regExp.exec(src);
while (result != null)
{
@@ -108,10 +164,10 @@ private void orderValidation(String source, String target)
return list;
}
- private List<String> runValidation(String compareFrom, String compareTo)
+ private ArrayList<String> listMissing(String compareFrom, String compareTo)
{
String tmp = compareTo;
- List<String> unmatched = new ArrayList<String>();
+ ArrayList<String> unmatched = new ArrayList<String>();
MatchResult result = regExp.exec(compareFrom);
while (result != null)
View
6 server/zanata-war/src/main/java/org/zanata/webtrans/shared/validation/action/VariablesValidation.java
@@ -41,9 +41,11 @@ public VariablesValidation(final ValidationMessages messages)
super(messages.variablesValidatorName(), messages.variablesValidatorDescription(), messages);
}
+
+ // derived from translate toolkit printf style variable matching regex. See:
+ // http://translate.svn.sourceforge.net/viewvc/translate/src/trunk/translate/filters/checks.py?revision=17978&view=markup
+ private final static String varRegex = "%((?:\\d+\\$|\\(\\w+\\))?[+#-]*(\\d+)?(\\.\\d+)?(hh|h|ll|l|L|z|j|t)?[\\w%])";
// private final static String varRegex = "%[\\w]+";
- // private final static String varRegex = "%[^\\s,$]+(?![^\\s,$])";
- private final static String varRegex = "%[^\\s,$]*";
@Override
public void validate(String source, String target)
View
135 server/zanata-war/src/test/java/org/zanata/webtrans/shared/validation/HtmlXmlTagValidationTests.java
@@ -194,10 +194,128 @@ public void missingTagsError()
assertThat(capturedTagsMissing.getValue().size(), is(3));
}
- // TODO update algorithm, this test will fail so will need updating
+ @Test
+ public void orderOnlyValidatedWithSameTags()
+ {
+ htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
+ String source = "<one><two><three></four></five>";
+ String target = "<two></five></four><three><six>";
+ htmlXmlTagValidation.validate(source, target);
+
+ assertThat(htmlXmlTagValidation.hasError(), is(true));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_MISSING_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_ADDED_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError().size(), is(2));
+
+ assertThat(capturedTagsMissing.getValue(), hasItem("<one>"));
+ assertThat(capturedTagsMissing.getValue().size(), is(1));
+ assertThat(capturedTagsAdded.getValue(), hasItem("<six>"));
+ assertThat(capturedTagsAdded.getValue().size(), is(1));
+ assertThat(capturedTagsOutOfOrder.hasCaptured(), is(false));
+ }
+
+ @Test
+ public void lastTagMovedToFirstError()
+ {
+ htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
+ String source = "<one><two><three></four></five><six>";
+ String target = "<six><one><two><three></four></five>";
+ htmlXmlTagValidation.validate(source, target);
+
+ assertThat(htmlXmlTagValidation.hasError(), is(true));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_OUT_OF_ORDER_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError().size(), is(1));
+
+ assertThat(capturedTagsOutOfOrder.getValue(), hasItem("<six>"));
+ assertThat("when one tag has moved, only that tag should be reported out of order", capturedTagsOutOfOrder.getValue().size(), is(1));
+ }
+
+ @Test
+ public void firstTagMovedToLastError()
+ {
+ htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
+ String source = "<one><two><three></four></five><six>";
+ String target = "<two><three></four></five><six><one>";
+ htmlXmlTagValidation.validate(source, target);
+
+ assertThat(htmlXmlTagValidation.hasError(), is(true));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_OUT_OF_ORDER_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError().size(), is(1));
+
+ assertThat(capturedTagsOutOfOrder.getValue(), hasItem("<one>"));
+ assertThat("when one tag has moved, only that tag should be reported out of order", capturedTagsOutOfOrder.getValue().size(), is(1));
+ }
+
+ @Test
+ public void tagMovedToMiddleError()
+ {
+ htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
+ String source = "<one><two><three></four></five><six>";
+ String target = "<two><three><one></four></five><six>";
+ htmlXmlTagValidation.validate(source, target);
+
+ assertThat(htmlXmlTagValidation.hasError(), is(true));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_OUT_OF_ORDER_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError().size(), is(1));
+
+ assertThat(capturedTagsOutOfOrder.getValue(), hasItem("<one>"));
+ assertThat("when one tag has moved, only that tag should be reported out of order", capturedTagsOutOfOrder.getValue().size(), is(1));
+ }
+
+ @Test
+ public void reversedTagsError()
+ {
+ htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
+ String source = "<one><two><three></four></five><six>";
+ String target = "<six></five></four><three><two><one>";
+ htmlXmlTagValidation.validate(source, target);
+
+ assertThat(htmlXmlTagValidation.hasError(), is(true));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_OUT_OF_ORDER_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError().size(), is(1));
+
+ // <one> is the first in-order tag, so is not reported
+ assertThat(capturedTagsOutOfOrder.getValue(), hasItems("<six>", "</five>", "</four>", "<three>", "<two>"));
+ assertThat(capturedTagsOutOfOrder.getValue().size(), is(5));
+ }
+
+ @Test
+ public void reportFirstTagsOutOfOrder()
+ {
+ htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
+ String source = "<one><two><three></four></five><six>";
+ String target = "</four></five><six><one><two><three>";
+ htmlXmlTagValidation.validate(source, target);
+
+ assertThat(htmlXmlTagValidation.hasError(), is(true));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_OUT_OF_ORDER_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError().size(), is(1));
+
+ assertThat(capturedTagsOutOfOrder.getValue(), hasItems("</four>", "</five>", "<six>"));
+ assertThat(capturedTagsOutOfOrder.getValue().size(), is(3));
+ }
+
+ @Test
+ public void reportLeastTagsOutOfOrder()
+ {
+ htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
+ String source = "<one><two><three></four></five><six>";
+ String target = "<six></four></five><one><two><three>";
+ htmlXmlTagValidation.validate(source, target);
+
+ assertThat(htmlXmlTagValidation.hasError(), is(true));
+ assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_OUT_OF_ORDER_MESSAGE));
+ assertThat(htmlXmlTagValidation.getError().size(), is(1));
+
+ // <one><two><three> in order
+ // should not use </four></five> as there are less tags
+ assertThat("should report the least number of tags to move to restore order", capturedTagsOutOfOrder.getValue(), hasItems("</four>", "</five>", "<six>"));
+ assertThat(capturedTagsOutOfOrder.getValue().size(), is(3));
+ }
+
@SuppressWarnings("unchecked")
@Test
- public void wrongOrderTagError()
+ public void swapSomeTagsError()
{
htmlXmlTagValidation = new HtmlXmlTagValidation(mockMessages);
String source = "<one><two><three></three></two><four></four></one>";
@@ -208,17 +326,10 @@ public void wrongOrderTagError()
assertThat(htmlXmlTagValidation.getError(), hasItem(MOCK_TAGS_OUT_OF_ORDER_MESSAGE));
assertThat(htmlXmlTagValidation.getError().size(), is(1));
- // Note: only <three> and </three> have been moved in this string, but the
- // algorithm just looks for changed index at the moment.
- assertThat(capturedTagsOutOfOrder.getValue(), hasItems("<three>", "</three>", "</two>", "<four>"));
- assertThat(capturedTagsOutOfOrder.getValue(), not(anyOf(hasItem("<one>"), hasItem("<two>"), hasItem("</four>"), hasItem("</one>"))));
- assertThat(capturedTagsOutOfOrder.getValue().size(), is(4));
+ assertThat(capturedTagsOutOfOrder.getValue(), hasItems("<three>", "</three>"));
+ assertThat(capturedTagsOutOfOrder.getValue(), not(anyOf(hasItem("<one>"), hasItem("<two>"), hasItem("</two>"), hasItem("<four>"), hasItem("</four>"), hasItem("</one>"))));
+ assertThat(capturedTagsOutOfOrder.getValue().size(), is(2));
}
-
- // TODO test cases for moving one tag a long way (when the algorithm
- // handles that better). e.g.
- // String source = "<one><two><three></three></two><four></four></one>";
- // String target = "</one><one><two><three></three></two><four></four>";
}
View
64 server/zanata-war/src/test/java/org/zanata/webtrans/shared/validation/VariablesValidationTests.java
@@ -101,8 +101,8 @@ public void descriptionIsSet()
public void noErrorForMatchingVars()
{
variablesValidation = new VariablesValidation(mockMessages);
- String source = "Testing string with variable %var1 and %var2";
- String target = "%var2 and %var1 included, order not relevant";
+ String source = "Testing string with variable %1v and %2v";
+ String target = "%2v and %1v included, order not relevant";
variablesValidation.validate(source, target);
assertThat(variablesValidation.hasError(), is(false));
@@ -116,7 +116,7 @@ public void noErrorForMatchingVars()
public void missingVarInTarget()
{
variablesValidation = new VariablesValidation(mockMessages);
- String source = "Testing string with variable %var1";
+ String source = "Testing string with variable %1v";
String target = "Testing string with no variables";
variablesValidation.validate(source, target);
@@ -124,7 +124,7 @@ public void missingVarInTarget()
assertThat(variablesValidation.getError(), hasItem(MOCK_VARIABLES_MISSING_MESSAGE));
assertThat(variablesValidation.getError().size(), is(1));
- assertThat(capturedVarsMissing.getValue(), hasItem("%var1"));
+ assertThat(capturedVarsMissing.getValue(), hasItem("%1v"));
assertThat(capturedVarsMissing.getValue().size(), is(1));
assertThat(capturedVarsAdded.hasCaptured(), is(false));
}
@@ -133,7 +133,7 @@ public void missingVarInTarget()
public void missingVarsThroughoutTarget()
{
variablesValidation = new VariablesValidation(mockMessages);
- String source = "%var1 variables in all parts %var2 of the string %var3";
+ String source = "%a variables in all parts %b of the string %c";
String target = "Testing string with no variables";
variablesValidation.validate(source, target);
@@ -141,7 +141,7 @@ public void missingVarsThroughoutTarget()
assertThat(variablesValidation.getError(), hasItem(MOCK_VARIABLES_MISSING_MESSAGE));
assertThat(variablesValidation.getError().size(), is(1));
- assertThat(capturedVarsMissing.getValue(), hasItems("%var1", "%var2", "%var3"));
+ assertThat(capturedVarsMissing.getValue(), hasItems("%a", "%b", "%c"));
assertThat(capturedVarsMissing.getValue().size(), is(3));
assertThat(capturedVarsAdded.hasCaptured(), is(false));
}
@@ -151,14 +151,14 @@ public void addedVarInTarget()
{
variablesValidation = new VariablesValidation(mockMessages);
String source = "Testing string with no variables";
- String target = "Testing string with variable %var1";
+ String target = "Testing string with variable %2$#x";
variablesValidation.validate(source, target);
assertThat(variablesValidation.hasError(), is(true));
assertThat(variablesValidation.getError(), hasItem(MOCK_VARIABLES_ADDED_MESSAGE));
assertThat(variablesValidation.getError().size(), is(1));
- assertThat(capturedVarsAdded.getValue(), hasItem("%var1"));
+ assertThat(capturedVarsAdded.getValue(), hasItem("%2$#x"));
assertThat(capturedVarsAdded.getValue().size(), is(1));
assertThat(capturedVarsMissing.hasCaptured(), is(false));
}
@@ -168,14 +168,14 @@ public void addedVarsThroughoutTarget()
{
variablesValidation = new VariablesValidation(mockMessages);
String source = "Testing string with no variables";
- String target = "%var1 variables in all parts %var2 of the string %var3";
+ String target = "%1$-0lls variables in all parts %2$-0hs of the string %3$-0ls";
variablesValidation.validate(source, target);
assertThat(variablesValidation.hasError(), is(true));
assertThat(variablesValidation.getError(), hasItem(MOCK_VARIABLES_ADDED_MESSAGE));
assertThat(variablesValidation.getError().size(), is(1));
- assertThat(capturedVarsAdded.getValue(), hasItems("%var1", "%var2", "%var3"));
+ assertThat(capturedVarsAdded.getValue(), hasItems("%1$-0lls", "%2$-0hs", "%3$-0ls"));
assertThat(capturedVarsAdded.getValue().size(), is(3));
assertThat(capturedVarsMissing.hasCaptured(), is(false));
}
@@ -184,17 +184,17 @@ public void addedVarsThroughoutTarget()
public void bothAddedAndMissingVars()
{
variablesValidation = new VariablesValidation(mockMessages);
- String source = "String with %var1 and %var2 only, not 3";
- String target = "String with %var2 and %var3, not 1";
+ String source = "String with %x and %y only, not z";
+ String target = "String with %y and %z, not x";
variablesValidation.validate(source, target);
assertThat(variablesValidation.hasError(), is(true));
assertThat(variablesValidation.getError(), hasItems(MOCK_VARIABLES_ADDED_MESSAGE, MOCK_VARIABLES_MISSING_MESSAGE));
assertThat(variablesValidation.getError().size(), is(2));
- assertThat(capturedVarsAdded.getValue(), hasItem("%var3"));
+ assertThat(capturedVarsAdded.getValue(), hasItem("%z"));
assertThat(capturedVarsAdded.getValue().size(), is(1));
- assertThat(capturedVarsMissing.getValue(), hasItem("%var1"));
+ assertThat(capturedVarsMissing.getValue(), hasItem("%x"));
assertThat(capturedVarsMissing.getValue().size(), is(1));
}
@@ -203,15 +203,15 @@ public void bothAddedAndMissingVars()
public void substringVariablesDontMatch()
{
variablesValidation = new VariablesValidation(mockMessages);
- String source = "%testing";
- String target = "%test %testing";
+ String source = "%ll";
+ String target = "%l %ll";
variablesValidation.validate(source, target);
assertThat(variablesValidation.hasError(), is(true));
assertThat(variablesValidation.getError(), hasItem(MOCK_VARIABLES_ADDED_MESSAGE));
assertThat(variablesValidation.getError().size(), is(1));
- assertThat(capturedVarsAdded.getValue(), allOf(hasItem("%test"), not(hasItem("%testing"))));
+ assertThat(capturedVarsAdded.getValue(), allOf(hasItem("%l"), not(hasItem("%ll"))));
assertThat(capturedVarsAdded.getValue().size(), is(1));
assertThat(capturedVarsMissing.hasCaptured(), is(false));
}
@@ -222,15 +222,15 @@ public void superstringVariablesDontMatch()
{
variablesValidation = new VariablesValidation(mockMessages);
- String source = "%what %whatever";
- String target = "%whatever";
+ String source = "%l %ll";
+ String target = "%ll";
variablesValidation.validate(source, target);
assertThat(variablesValidation.hasError(), is(true));
assertThat(variablesValidation.getError(), hasItem(MOCK_VARIABLES_MISSING_MESSAGE));
assertThat(variablesValidation.getError().size(), is(1));
- assertThat(capturedVarsMissing.getValue(), allOf(hasItem("%what"), not(hasItem("%whatever"))));
+ assertThat(capturedVarsMissing.getValue(), allOf(hasItem("%l"), not(hasItem("%ll"))));
assertThat(capturedVarsAdded.hasCaptured(), is(false));
}
@@ -240,16 +240,32 @@ public void superstringVariablesDontMatch2()
{
variablesValidation = new VariablesValidation(mockMessages);
- String source = "%test";
- String target = "%testing";
+ String source = "%z";
+ String target = "%zz";
variablesValidation.validate(source, target);
assertThat(variablesValidation.hasError(), is(true));
assertThat(variablesValidation.getError(), hasItems(MOCK_VARIABLES_MISSING_MESSAGE, MOCK_VARIABLES_ADDED_MESSAGE));
assertThat(variablesValidation.getError().size(), is(2));
- assertThat(capturedVarsMissing.getValue(), allOf(hasItem("%test"), not(hasItem("%testing"))));
- assertThat(capturedVarsAdded.getValue(), allOf(hasItem("%testing"), not(hasItem("%test"))));
+ assertThat(capturedVarsMissing.getValue(), allOf(hasItem("%z"), not(hasItem("%zz"))));
+ assertThat(capturedVarsAdded.getValue(), allOf(hasItem("%zz"), not(hasItem("%z"))));
+ }
+
+ @Test
+ public void checkWithRealWorldExamples()
+ {
+ variablesValidation = new VariablesValidation(mockMessages);
+ // examples from strings in translate.zanata.org
+ String source = "%s %d %-25s %r";
+ String target = "no variables";
+ variablesValidation.validate(source, target);
+
+ assertThat(variablesValidation.hasError(), is(true));
+ assertThat(variablesValidation.getError(), hasItems(MOCK_VARIABLES_MISSING_MESSAGE));
+ assertThat(variablesValidation.getError().size(), is(1));
+
+ assertThat(capturedVarsMissing.getValue(), hasItems("%s", "%d", "%-25s", "%r"));
}
}

No commit comments for this range

Something went wrong with that request. Please try again.