Skip to content

Commit 474f244

Browse files
ja-openaija-openai
committed
Refactor AndroidString classes to be able to apply proper and consistent escaping.
This also greatly simplify the logic
1 parent b9046e2 commit 474f244

File tree

8 files changed

+127
-174
lines changed

8 files changed

+127
-174
lines changed

‎webapp/src/main/java/com/box/l10n/mojito/android/strings/AndroidStringDocument.java

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package com.box.l10n.mojito.android.strings;
22

3-
import static com.google.common.base.Preconditions.checkArgument;
4-
53
import java.util.ArrayList;
64
import java.util.List;
7-
import org.w3c.dom.Node;
85

96
public class AndroidStringDocument {
107

@@ -29,46 +26,8 @@ public void addSingular(AndroidSingular string) {
2926
strings.add(string);
3027
}
3128

32-
public void addSingular(AndroidStringElement element, Node comment) {
33-
34-
checkArgument(element.isSingular(), "element should be singular");
35-
36-
addSingular(
37-
new AndroidSingular(
38-
element.getIdAttribute(),
39-
element.getNameAttribute(),
40-
element.getUnescapedContent(),
41-
comment != null ? comment.getTextContent() : null));
42-
}
43-
4429
public void addPlural(AndroidPlural plural) {
4530
plurals.add(plural);
4631
strings.add(plural);
4732
}
48-
49-
public void addPlural(AndroidStringElement element, Node comment) {
50-
51-
checkArgument(element.isPlural(), "element should be plural");
52-
53-
AndroidPlural.AndroidPluralBuilder builder = AndroidPlural.builder();
54-
builder.setName(element.getNameAttribute());
55-
if (comment != null) {
56-
builder.setComment(comment.getTextContent());
57-
}
58-
59-
element.forEachPluralItem(builder::addItem);
60-
61-
addPlural(builder.build());
62-
}
63-
64-
public void addElement(Node node, Node comment) {
65-
66-
AndroidStringElement element = new AndroidStringElement(node);
67-
68-
if (element.isSingular()) {
69-
addSingular(element, comment);
70-
} else if (element.isPlural()) {
71-
addPlural(element, comment);
72-
}
73-
}
7433
}

‎webapp/src/main/java/com/box/l10n/mojito/android/strings/AndroidStringDocumentMapper.java

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ Stream<TextUnitDTO> singularToTextUnit(AndroidSingular singular) {
202202
textUnit.setName(singular.getName());
203203
textUnit.setComment(singular.getComment());
204204
textUnit.setTmTextUnitId(singular.getId());
205-
textUnit.setTarget(unescape(singular.getContent()));
205+
textUnit.setTarget(singular.getContent());
206206
addTextUnitDTOAttributes(textUnit);
207207

208208
return Stream.of(textUnit);
@@ -226,7 +226,7 @@ Stream<TextUnitDTO> pluralToTextUnits(AndroidPlural plural) {
226226
textUnit.setTmTextUnitId(item.getId());
227227
textUnit.setPluralForm(quantity);
228228
textUnit.setPluralFormOther(pluralFormOther);
229-
textUnit.setTarget(unescape(item.getContent()));
229+
textUnit.setTarget(item.getContent());
230230
addTextUnitDTOAttributes(textUnit);
231231

232232
return textUnit;
@@ -286,23 +286,4 @@ static String removeInvalidControlCharacter(String str) {
286286

287287
return withoutControlCharacters;
288288
}
289-
290-
/** should use {@link com.box.l10n.mojito.okapi.filters.AndroidFilter#unescape(String)} */
291-
static String unescape(String str) {
292-
293-
String unescape = str;
294-
295-
if (StringUtils.startsWith(unescape, "\"") && StringUtils.endsWith(unescape, "\"")) {
296-
unescape = unescape.substring(1, unescape.length() - 1);
297-
}
298-
299-
unescape =
300-
Strings.nullToEmpty(unescape)
301-
.replaceAll("\\\\'", "'")
302-
.replaceAll("\\\\\"", "\"")
303-
.replaceAll("\\\\@", "@")
304-
.replaceAll("\\\\n", "\n");
305-
306-
return unescape;
307-
}
308289
}

‎webapp/src/main/java/com/box/l10n/mojito/android/strings/AndroidStringDocumentReader.java

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,28 @@
11
package com.box.l10n.mojito.android.strings;
22

3-
import static com.box.l10n.mojito.android.strings.AndroidStringDocumentUtils.documentBuilder;
4-
53
import com.google.common.base.Strings;
4+
import org.apache.commons.lang3.StringUtils;
5+
import org.w3c.dom.Document;
6+
import org.w3c.dom.Element;
7+
import org.w3c.dom.Node;
8+
import org.w3c.dom.NodeList;
9+
import org.xml.sax.InputSource;
10+
import org.xml.sax.SAXException;
11+
612
import java.io.File;
713
import java.io.IOException;
814
import java.io.StringReader;
915
import java.util.LinkedList;
16+
import java.util.Optional;
1017
import java.util.Queue;
11-
import javax.xml.parsers.ParserConfigurationException;
12-
import org.w3c.dom.Document;
13-
import org.w3c.dom.Node;
14-
import org.xml.sax.InputSource;
15-
import org.xml.sax.SAXException;
18+
import java.util.function.Function;
19+
20+
import static com.box.l10n.mojito.android.strings.AndroidStringDocumentUtils.documentBuilder;
1621

1722
public class AndroidStringDocumentReader {
1823

1924
public static AndroidStringDocument fromFile(String fileName)
20-
throws ParserConfigurationException, IOException, SAXException {
25+
throws IOException, SAXException {
2126
return buildFromDocument(documentBuilder().parse(new File(fileName)));
2227
}
2328

@@ -44,10 +49,96 @@ private static AndroidStringDocument buildFromDocument(Document source) {
4449
commentNodes.offer(node);
4550

4651
} else if (Node.ELEMENT_NODE == node.getNodeType()) {
47-
document.addElement(node, commentNodes.poll());
52+
Element nodeAsElement = (Element) node;
53+
Node comment = commentNodes.poll();
54+
55+
if (nodeAsElement.getTagName().equals(AndroidStringElement.SINGULAR_ELEMENT_NAME)) {
56+
document.addSingular(
57+
new AndroidSingular(
58+
getAttributeAs(
59+
nodeAsElement, AndroidStringElement.ID_ATTRIBUTE_NAME, Long::valueOf),
60+
getNameAttribute(nodeAsElement),
61+
unescape(
62+
nodeAsElement
63+
.getTextContent()),
64+
comment != null ? comment.getTextContent() : null));
65+
} else if (nodeAsElement.getTagName().equals(AndroidStringElement.PLURAL_ELEMENT_NAME)) {
66+
67+
AndroidPlural.AndroidPluralBuilder builder = AndroidPlural.builder();
68+
69+
builder.setName(nodeAsElement.getAttribute(AndroidStringElement.NAME_ATTRIBUTE_NAME));
70+
if (comment != null) {
71+
builder.setComment(comment.getTextContent());
72+
}
73+
74+
NodeList nodeList =
75+
nodeAsElement.getElementsByTagName(AndroidStringElement.PLURAL_ITEM_ELEMENT_NAME);
76+
77+
for (int j = 0; j < nodeList.getLength(); j++) {
78+
Element item = (Element) nodeList.item(j);
79+
80+
builder.addItem(
81+
new AndroidPluralItem(
82+
getAttribute(item, AndroidStringElement.QUANTITY_ATTRIBUTE_NAME),
83+
getAttributeAs(item, AndroidStringElement.ID_ATTRIBUTE_NAME, Long::valueOf),
84+
unescape(getTextContent(item))));
85+
86+
}
87+
88+
document.addPlural(builder.build());
89+
}
4890
}
4991
}
5092

5193
return document;
5294
}
95+
96+
public static String getAttribute(Element element, String name) {
97+
// looks like ofNullable is useless and then the orElse
98+
return Optional.ofNullable(element.getAttribute(name)).orElse("");
99+
}
100+
101+
public static <T> T getAttributeAs(
102+
Element element, String attributeName, Function<String, T> converter) {
103+
104+
T result = null;
105+
106+
if (element.hasAttribute(attributeName)) {
107+
result = converter.apply(element.getAttribute(attributeName));
108+
}
109+
110+
return result;
111+
}
112+
113+
public static String getTextContent(Element element) {
114+
return Optional.ofNullable(element).map(Element::getTextContent).orElse("");
115+
}
116+
117+
public static String getNameAttribute(Element element) {
118+
return element.getAttribute(AndroidStringElement.NAME_ATTRIBUTE_NAME);
119+
}
120+
121+
122+
/** should use {@link com.box.l10n.mojito.okapi.filters.AndroidFilter#unescape(String)} */
123+
static String unescape(String str) {
124+
125+
String unescape = str;
126+
127+
if (!Strings.isNullOrEmpty(str)) {
128+
129+
if (StringUtils.startsWith(unescape, "\"") && StringUtils.endsWith(unescape, "\"")) {
130+
unescape = unescape.substring(1, unescape.length() - 1);
131+
}
132+
133+
unescape =
134+
Strings.nullToEmpty(unescape)
135+
.replaceAll("\\\\'", "'")
136+
.replaceAll("\\\\\"", "\"")
137+
.replaceAll("\\\\@", "@")
138+
.replaceAll("\\\\n", "\n");
139+
140+
}
141+
return unescape;
142+
}
143+
53144
}

‎webapp/src/main/java/com/box/l10n/mojito/android/strings/AndroidStringDocumentWriter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ private Element addChild(Node node, String name) {
137137

138138
private void setContent(Element element, String content) {
139139
if (!Strings.isNullOrEmpty(content)) {
140-
element.setTextContent(escapeQuotes(content));
140+
element.setTextContent(escapeQuotes(content, escapeType));
141141
}
142142
}
143143

@@ -159,7 +159,7 @@ private void setAttribute(Element element, String name, String value) {
159159
}
160160
}
161161

162-
String escapeQuotes(String str) {
162+
static String escapeQuotes(String str, EscapeType escapeType) {
163163
String escaped = str;
164164
if (!Strings.isNullOrEmpty(str)) {
165165
escaped =

‎webapp/src/main/java/com/box/l10n/mojito/android/strings/AndroidStringElement.java

Lines changed: 0 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,5 @@
11
package com.box.l10n.mojito.android.strings;
22

3-
import com.google.common.base.Strings;
4-
import java.util.Optional;
5-
import java.util.function.Consumer;
6-
import java.util.function.Function;
7-
import org.w3c.dom.Element;
8-
import org.w3c.dom.Node;
9-
import org.w3c.dom.NodeList;
10-
113
public class AndroidStringElement {
124

135
static final String ROOT_ELEMENT_NAME = "resources";
@@ -17,88 +9,4 @@ public class AndroidStringElement {
179
static final String NAME_ATTRIBUTE_NAME = "name";
1810
static final String QUANTITY_ATTRIBUTE_NAME = "quantity";
1911
static final String ID_ATTRIBUTE_NAME = "tmTextUnitId";
20-
21-
private final Element element;
22-
23-
public AndroidStringElement(Element element) {
24-
this.element = element;
25-
}
26-
27-
public AndroidStringElement(Node node) {
28-
this((Element) node);
29-
}
30-
31-
public boolean isSingular() {
32-
return element.getTagName().equals(SINGULAR_ELEMENT_NAME);
33-
}
34-
35-
public boolean isPlural() {
36-
return element.getTagName().equals(PLURAL_ELEMENT_NAME);
37-
}
38-
39-
public boolean isPluralItem() {
40-
return element.getTagName().equals(PLURAL_ITEM_ELEMENT_NAME);
41-
}
42-
43-
public String getTextContent() {
44-
return Optional.ofNullable(element).map(Element::getTextContent).orElse("");
45-
}
46-
47-
public Long getIdAttribute() {
48-
return getLongAttribute(ID_ATTRIBUTE_NAME);
49-
}
50-
51-
public String getNameAttribute() {
52-
return getAttribute(NAME_ATTRIBUTE_NAME);
53-
}
54-
55-
public String getAttribute(String name) {
56-
return Optional.ofNullable(element.getAttribute(name)).orElse("");
57-
}
58-
59-
public String getUnescapedContent() {
60-
return removeEscape(element.getTextContent());
61-
}
62-
63-
public Long getLongAttribute(String attributeName) {
64-
return getAttributeAs(attributeName, Long::valueOf);
65-
}
66-
67-
public <T> T getAttributeAs(String attributeName, Function<String, T> converter) {
68-
69-
T result = null;
70-
71-
if (element.hasAttribute(attributeName)) {
72-
result = converter.apply(element.getAttribute(attributeName));
73-
}
74-
75-
return result;
76-
}
77-
78-
public void forEachPluralItem(Consumer<AndroidPluralItem> consumer) {
79-
80-
AndroidStringElement node;
81-
NodeList nodeList = element.getElementsByTagName(PLURAL_ITEM_ELEMENT_NAME);
82-
83-
for (int i = 0; i < nodeList.getLength(); i++) {
84-
85-
node = new AndroidStringElement((Element) nodeList.item(i));
86-
87-
if (node.isPluralItem()) {
88-
consumer.accept(
89-
new AndroidPluralItem(
90-
node.getAttribute(QUANTITY_ATTRIBUTE_NAME),
91-
node.getLongAttribute(ID_ATTRIBUTE_NAME),
92-
node.getTextContent()));
93-
}
94-
}
95-
}
96-
97-
private static String removeEscape(String str) {
98-
return Strings.nullToEmpty(str)
99-
.replaceAll("\\\\'", "'")
100-
.replaceAll("\\\\\"", "\"")
101-
.replace("\\\\\n", "\n")
102-
.replaceAll("\\\\@", "@");
103-
}
10412
}

‎webapp/src/test/java/com/box/l10n/mojito/android/strings/AndroidStringDocumentMapperTest.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -924,13 +924,23 @@ public void testReadingTextUnitsFromFile() throws Exception {
924924
assertThat(textUnits)
925925
.filteredOn(tu -> tu.getName().equalsIgnoreCase("show_options"))
926926
.extracting("name", "target", "comment")
927-
.containsOnly(tuple("show_options", "Mer \" dela", "Options"));
927+
.containsOnly(tuple("show_options", "Mer \" dela", null));
928928

929929
assertThat(textUnits)
930930
.filteredOn(tu -> tu.getName().equalsIgnoreCase("without_comment"))
931931
.extracting("target", "comment")
932932
.containsOnly(tuple("ei ' kommentteja", null));
933933

934+
assertThat(textUnits)
935+
.filteredOn(tu -> tu.getName().equalsIgnoreCase("show_options2"))
936+
.extracting("name", "target", "comment")
937+
.containsOnly(tuple("show_options2", "Mer \\\" dela", "Options"));
938+
939+
assertThat(textUnits)
940+
.filteredOn(tu -> tu.getName().equalsIgnoreCase("without_comment2"))
941+
.extracting("target", "comment")
942+
.containsOnly(tuple("ei \\' kommentteja", null));
943+
934944
assertThat(textUnits)
935945
.filteredOn(tu -> tu.getName().equalsIgnoreCase("line_break"))
936946
.extracting("target", "comment")

0 commit comments

Comments
 (0)