Skip to content

Commit

Permalink
Refactor AndroidString classes to be able to apply proper and consist…
Browse files Browse the repository at this point in the history
…ent escaping.

This also greatly simplify the logic
  • Loading branch information
ja-openai committed Sep 27, 2024
1 parent b9046e2 commit 474f244
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 174 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package com.box.l10n.mojito.android.strings;

import static com.google.common.base.Preconditions.checkArgument;

import java.util.ArrayList;
import java.util.List;
import org.w3c.dom.Node;

public class AndroidStringDocument {

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

public void addSingular(AndroidStringElement element, Node comment) {

checkArgument(element.isSingular(), "element should be singular");

addSingular(
new AndroidSingular(
element.getIdAttribute(),
element.getNameAttribute(),
element.getUnescapedContent(),
comment != null ? comment.getTextContent() : null));
}

public void addPlural(AndroidPlural plural) {
plurals.add(plural);
strings.add(plural);
}

public void addPlural(AndroidStringElement element, Node comment) {

checkArgument(element.isPlural(), "element should be plural");

AndroidPlural.AndroidPluralBuilder builder = AndroidPlural.builder();
builder.setName(element.getNameAttribute());
if (comment != null) {
builder.setComment(comment.getTextContent());
}

element.forEachPluralItem(builder::addItem);

addPlural(builder.build());
}

public void addElement(Node node, Node comment) {

AndroidStringElement element = new AndroidStringElement(node);

if (element.isSingular()) {
addSingular(element, comment);
} else if (element.isPlural()) {
addPlural(element, comment);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ Stream<TextUnitDTO> singularToTextUnit(AndroidSingular singular) {
textUnit.setName(singular.getName());
textUnit.setComment(singular.getComment());
textUnit.setTmTextUnitId(singular.getId());
textUnit.setTarget(unescape(singular.getContent()));
textUnit.setTarget(singular.getContent());
addTextUnitDTOAttributes(textUnit);

return Stream.of(textUnit);
Expand All @@ -226,7 +226,7 @@ Stream<TextUnitDTO> pluralToTextUnits(AndroidPlural plural) {
textUnit.setTmTextUnitId(item.getId());
textUnit.setPluralForm(quantity);
textUnit.setPluralFormOther(pluralFormOther);
textUnit.setTarget(unescape(item.getContent()));
textUnit.setTarget(item.getContent());
addTextUnitDTOAttributes(textUnit);

return textUnit;
Expand Down Expand Up @@ -286,23 +286,4 @@ static String removeInvalidControlCharacter(String str) {

return withoutControlCharacters;
}

/** should use {@link com.box.l10n.mojito.okapi.filters.AndroidFilter#unescape(String)} */
static String unescape(String str) {

String unescape = str;

if (StringUtils.startsWith(unescape, "\"") && StringUtils.endsWith(unescape, "\"")) {
unescape = unescape.substring(1, unescape.length() - 1);
}

unescape =
Strings.nullToEmpty(unescape)
.replaceAll("\\\\'", "'")
.replaceAll("\\\\\"", "\"")
.replaceAll("\\\\@", "@")
.replaceAll("\\\\n", "\n");

return unescape;
}
}
Original file line number Diff line number Diff line change
@@ -1,23 +1,28 @@
package com.box.l10n.mojito.android.strings;

import static com.box.l10n.mojito.android.strings.AndroidStringDocumentUtils.documentBuilder;

import com.google.common.base.Strings;
import org.apache.commons.lang3.StringUtils;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;

import java.io.File;
import java.io.IOException;
import java.io.StringReader;
import java.util.LinkedList;
import java.util.Optional;
import java.util.Queue;
import javax.xml.parsers.ParserConfigurationException;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import java.util.function.Function;

import static com.box.l10n.mojito.android.strings.AndroidStringDocumentUtils.documentBuilder;

public class AndroidStringDocumentReader {

public static AndroidStringDocument fromFile(String fileName)
throws ParserConfigurationException, IOException, SAXException {
throws IOException, SAXException {
return buildFromDocument(documentBuilder().parse(new File(fileName)));
}

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

} else if (Node.ELEMENT_NODE == node.getNodeType()) {
document.addElement(node, commentNodes.poll());
Element nodeAsElement = (Element) node;
Node comment = commentNodes.poll();

if (nodeAsElement.getTagName().equals(AndroidStringElement.SINGULAR_ELEMENT_NAME)) {
document.addSingular(
new AndroidSingular(
getAttributeAs(
nodeAsElement, AndroidStringElement.ID_ATTRIBUTE_NAME, Long::valueOf),
getNameAttribute(nodeAsElement),
unescape(
nodeAsElement
.getTextContent()),
comment != null ? comment.getTextContent() : null));
} else if (nodeAsElement.getTagName().equals(AndroidStringElement.PLURAL_ELEMENT_NAME)) {

AndroidPlural.AndroidPluralBuilder builder = AndroidPlural.builder();

builder.setName(nodeAsElement.getAttribute(AndroidStringElement.NAME_ATTRIBUTE_NAME));
if (comment != null) {
builder.setComment(comment.getTextContent());
}

NodeList nodeList =
nodeAsElement.getElementsByTagName(AndroidStringElement.PLURAL_ITEM_ELEMENT_NAME);

for (int j = 0; j < nodeList.getLength(); j++) {
Element item = (Element) nodeList.item(j);

builder.addItem(
new AndroidPluralItem(
getAttribute(item, AndroidStringElement.QUANTITY_ATTRIBUTE_NAME),
getAttributeAs(item, AndroidStringElement.ID_ATTRIBUTE_NAME, Long::valueOf),
unescape(getTextContent(item))));

}

document.addPlural(builder.build());
}
}
}

return document;
}

public static String getAttribute(Element element, String name) {
// looks like ofNullable is useless and then the orElse
return Optional.ofNullable(element.getAttribute(name)).orElse("");
}

public static <T> T getAttributeAs(
Element element, String attributeName, Function<String, T> converter) {

T result = null;

if (element.hasAttribute(attributeName)) {
result = converter.apply(element.getAttribute(attributeName));
}

return result;
}

public static String getTextContent(Element element) {
return Optional.ofNullable(element).map(Element::getTextContent).orElse("");
}

public static String getNameAttribute(Element element) {
return element.getAttribute(AndroidStringElement.NAME_ATTRIBUTE_NAME);
}


/** should use {@link com.box.l10n.mojito.okapi.filters.AndroidFilter#unescape(String)} */
static String unescape(String str) {

String unescape = str;

if (!Strings.isNullOrEmpty(str)) {

if (StringUtils.startsWith(unescape, "\"") && StringUtils.endsWith(unescape, "\"")) {
unescape = unescape.substring(1, unescape.length() - 1);
}

unescape =
Strings.nullToEmpty(unescape)
.replaceAll("\\\\'", "'")
.replaceAll("\\\\\"", "\"")
.replaceAll("\\\\@", "@")
.replaceAll("\\\\n", "\n");

}
return unescape;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private Element addChild(Node node, String name) {

private void setContent(Element element, String content) {
if (!Strings.isNullOrEmpty(content)) {
element.setTextContent(escapeQuotes(content));
element.setTextContent(escapeQuotes(content, escapeType));
}
}

Expand All @@ -159,7 +159,7 @@ private void setAttribute(Element element, String name, String value) {
}
}

String escapeQuotes(String str) {
static String escapeQuotes(String str, EscapeType escapeType) {
String escaped = str;
if (!Strings.isNullOrEmpty(str)) {
escaped =
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
package com.box.l10n.mojito.android.strings;

import com.google.common.base.Strings;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Function;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;

public class AndroidStringElement {

static final String ROOT_ELEMENT_NAME = "resources";
Expand All @@ -17,88 +9,4 @@ public class AndroidStringElement {
static final String NAME_ATTRIBUTE_NAME = "name";
static final String QUANTITY_ATTRIBUTE_NAME = "quantity";
static final String ID_ATTRIBUTE_NAME = "tmTextUnitId";

private final Element element;

public AndroidStringElement(Element element) {
this.element = element;
}

public AndroidStringElement(Node node) {
this((Element) node);
}

public boolean isSingular() {
return element.getTagName().equals(SINGULAR_ELEMENT_NAME);
}

public boolean isPlural() {
return element.getTagName().equals(PLURAL_ELEMENT_NAME);
}

public boolean isPluralItem() {
return element.getTagName().equals(PLURAL_ITEM_ELEMENT_NAME);
}

public String getTextContent() {
return Optional.ofNullable(element).map(Element::getTextContent).orElse("");
}

public Long getIdAttribute() {
return getLongAttribute(ID_ATTRIBUTE_NAME);
}

public String getNameAttribute() {
return getAttribute(NAME_ATTRIBUTE_NAME);
}

public String getAttribute(String name) {
return Optional.ofNullable(element.getAttribute(name)).orElse("");
}

public String getUnescapedContent() {
return removeEscape(element.getTextContent());
}

public Long getLongAttribute(String attributeName) {
return getAttributeAs(attributeName, Long::valueOf);
}

public <T> T getAttributeAs(String attributeName, Function<String, T> converter) {

T result = null;

if (element.hasAttribute(attributeName)) {
result = converter.apply(element.getAttribute(attributeName));
}

return result;
}

public void forEachPluralItem(Consumer<AndroidPluralItem> consumer) {

AndroidStringElement node;
NodeList nodeList = element.getElementsByTagName(PLURAL_ITEM_ELEMENT_NAME);

for (int i = 0; i < nodeList.getLength(); i++) {

node = new AndroidStringElement((Element) nodeList.item(i));

if (node.isPluralItem()) {
consumer.accept(
new AndroidPluralItem(
node.getAttribute(QUANTITY_ATTRIBUTE_NAME),
node.getLongAttribute(ID_ATTRIBUTE_NAME),
node.getTextContent()));
}
}
}

private static String removeEscape(String str) {
return Strings.nullToEmpty(str)
.replaceAll("\\\\'", "'")
.replaceAll("\\\\\"", "\"")
.replace("\\\\\n", "\n")
.replaceAll("\\\\@", "@");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -924,13 +924,23 @@ public void testReadingTextUnitsFromFile() throws Exception {
assertThat(textUnits)
.filteredOn(tu -> tu.getName().equalsIgnoreCase("show_options"))
.extracting("name", "target", "comment")
.containsOnly(tuple("show_options", "Mer \" dela", "Options"));
.containsOnly(tuple("show_options", "Mer \" dela", null));

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

assertThat(textUnits)
.filteredOn(tu -> tu.getName().equalsIgnoreCase("show_options2"))
.extracting("name", "target", "comment")
.containsOnly(tuple("show_options2", "Mer \\\" dela", "Options"));

assertThat(textUnits)
.filteredOn(tu -> tu.getName().equalsIgnoreCase("without_comment2"))
.extracting("target", "comment")
.containsOnly(tuple("ei \\' kommentteja", null));

assertThat(textUnits)
.filteredOn(tu -> tu.getName().equalsIgnoreCase("line_break"))
.extracting("target", "comment")
Expand Down
Loading

0 comments on commit 474f244

Please sign in to comment.