Skip to content

Commit

Permalink
Xml processing fixes:
Browse files Browse the repository at this point in the history
* Support for <item name="foo" type="id">7x0000</item>
* Adds <eat-comment/> after source attributions to ensure they are not sent to the final binary
* Store attributes for arrays to the string translatable=false case

--
MOS_MIGRATED_REVID=124684322
  • Loading branch information
Googler authored and hermione521 committed Jun 13, 2016
1 parent c69eaac commit 4f854d4
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ private static XmlResourceValue valueFromProto(SerializeFormat.DataValueXml prot
private static XmlResourceValue parseXmlElements(
ResourceType resourceType, XMLEventReader eventReader, StartElement start)
throws XMLStreamException {
// Handle ids first, as they are a special kind of item.
if (resourceType == ID) {
// Handle unary ids first, as they are a special kind of item.
if (resourceType == ID && XmlResourceValues.isEndTag(eventReader.peek(), start.getName())) {
return XmlResourceValues.parseId();
}
// Handle item stubs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,22 @@ static XmlResourceValue parseId() {
static XmlResourceValue parseSimple(
XMLEventReader eventReader, ResourceType resourceType, StartElement start)
throws XMLStreamException {
String contents;
// Check that the element is unary. If it is, the contents is null
if (isEndTag(eventReader.peek(), start.getName())) {
contents = null;
} else {
contents = readContentsAsString(eventReader, start.getName());
}
return SimpleXmlResourceValue.of(
start.getName().equals(TAG_ITEM)
? SimpleXmlResourceValue.Type.ITEM
: SimpleXmlResourceValue.Type.from(resourceType),
ImmutableMap.copyOf(parseTagAttributes(start)),
contents);
}

public static Map<String, String> parseTagAttributes(StartElement start) {
// Using a map to deduplicate xmlns declarations on the attributes.
Map<String, String> attributeMap = new LinkedHashMap<>();
Iterator<Attribute> attributes = iterateAttributesFrom(start);
Expand All @@ -190,19 +206,7 @@ static XmlResourceValue parseSimple(
attributeMap.put(attribute.getName().getLocalPart(), value);
}
}
String contents;
// Check and see if the element is unary. If it is, the contents is null
if (isEndTag(eventReader.peek(), start.getName())) {
contents = null;
} else {
contents = readContentsAsString(eventReader, start.getName());
}
return SimpleXmlResourceValue.of(
start.getName().equals(TAG_ITEM)
? SimpleXmlResourceValue.Type.ITEM
: SimpleXmlResourceValue.Type.from(resourceType),
ImmutableMap.copyOf(attributeMap),
contents);
return attributeMap;
}

// TODO(corysmith): Replace this with real escaping system, preferably a performant high level xml
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.android.AndroidDataWritingVisitor;
import com.google.devtools.build.android.FullyQualifiedName;
import com.google.devtools.build.android.XmlResourceValue;
Expand All @@ -30,6 +31,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map.Entry;
import java.util.Objects;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -94,8 +96,19 @@ public static ArrayType fromTagName(StartElement start) {
String.format("%s not found in %s", tagQName, Arrays.toString(values())));
}

String openTag(FullyQualifiedName key) {
return String.format("<%s name='%s'>", tagName.getLocalPart(), key.name());
String openTag(FullyQualifiedName key, ImmutableMap<String, String> attributes) {
StringBuilder xmlString = new StringBuilder("<");
xmlString.append(tagName.getLocalPart());
xmlString.append(" name='").append(key.name()).append("'");
for (Entry<String, String> entry : attributes.entrySet()) {
xmlString
.append(" ")
.append(entry.getKey())
.append("='")
.append(entry.getValue())
.append("'");
}
return xmlString.append(">").toString();
}

String closeTag() {
Expand All @@ -105,10 +118,13 @@ String closeTag() {

private final ImmutableList<String> values;
private final ArrayType arrayType;
private final ImmutableMap<String, String> attributes;

private ArrayXmlResourceValue(ArrayType arrayType, ImmutableList<String> values) {
private ArrayXmlResourceValue(
ArrayType arrayType, ImmutableList<String> values, ImmutableMap<String, String> attributes) {
this.arrayType = arrayType;
this.values = values;
this.attributes = attributes;
}

@VisibleForTesting
Expand All @@ -117,11 +133,19 @@ public static XmlResourceValue of(ArrayType arrayType, String... values) {
}

public static XmlResourceValue of(ArrayType arrayType, List<String> values) {
return new ArrayXmlResourceValue(arrayType, ImmutableList.copyOf(values));
return of(arrayType, values, ImmutableMap.<String, String>of());
}

public static XmlResourceValue of(
ArrayType arrayType, List<String> values, ImmutableMap<String, String> attributes) {
return new ArrayXmlResourceValue(arrayType, ImmutableList.copyOf(values), attributes);
}

public static XmlResourceValue from(SerializeFormat.DataValueXml proto) {
return of(ArrayType.valueOf(proto.getValueType()), proto.getListValueList());
return of(
ArrayType.valueOf(proto.getValueType()),
proto.getListValueList(),
ImmutableMap.copyOf(proto.getMappedStringValueMap()));
}

@Override
Expand All @@ -130,7 +154,8 @@ public void write(
mergedDataWriter.writeToValuesXml(
key,
FluentIterable.from(
ImmutableList.of(String.format("<!-- %s -->", source), arrayType.openTag(key)))
ImmutableList.of(
String.format("<!-- %s -->", source), arrayType.openTag(key, attributes)))
.append(FluentIterable.from(values).transform(ITEM_TO_XML))
.append(arrayType.closeTag()));
}
Expand All @@ -144,12 +169,13 @@ public int serializeTo(Path source, OutputStream output) throws IOException {
SerializeFormat.DataValueXml.newBuilder()
.addAllListValue(values)
.setType(SerializeFormat.DataValueXml.XmlType.ARRAY)
.putAllMappedStringValue(attributes)
.setValueType(arrayType.toString())));
}

@Override
public int hashCode() {
return Objects.hash(arrayType, values);
return Objects.hash(arrayType, values, attributes);
}

@Override
Expand All @@ -158,14 +184,17 @@ public boolean equals(Object obj) {
return false;
}
ArrayXmlResourceValue other = (ArrayXmlResourceValue) obj;
return Objects.equals(arrayType, other.arrayType) && Objects.equals(values, other.values);
return Objects.equals(arrayType, other.arrayType)
&& Objects.equals(values, other.values)
&& Objects.equals(attributes, other.attributes);
}

@Override
public String toString() {
return MoreObjects.toStringHelper(getClass())
.add("arrayType", arrayType)
.add("values", values)
.add("attributes", attributes)
.toString();
}

Expand All @@ -185,13 +214,16 @@ public static XmlResourceValue parseArray(XMLEventReader eventReader, StartEleme
throw new XMLStreamException(
String.format("Expected start element %s", element), element.getLocation());
}
String contents = XmlResourceValues.readContentsAsString(eventReader,
element.asStartElement().getName());
String contents =
XmlResourceValues.readContentsAsString(eventReader, element.asStartElement().getName());
values.add(contents != null ? contents : "");
}
}
try {
return of(ArrayType.fromTagName(start), values);
return of(
ArrayType.fromTagName(start),
values,
ImmutableMap.copyOf(XmlResourceValues.parseTagAttributes(start)));
} catch (IllegalArgumentException e) {
throw new XMLStreamException(e.getMessage(), start.getLocation());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,21 +289,39 @@ public String toString() {
@Override
public void write(
FullyQualifiedName key, Path source, AndroidDataWritingVisitor mergedDataWriter) {

ImmutableList<String> formatKeys = Ordering.natural().immutableSortedCopy(formats.keySet());
FluentIterable<String> iterable =
FluentIterable.from(
ImmutableList.of(
String.format("<!-- %s -->", source),
formatKeys.isEmpty()
? String.format("<attr name='%s'>", key.name())
: String.format(
"<attr name='%s' format='%s'>",
key.name(),
Joiner.on('|').join(formatKeys))));
for (String formatKey : formatKeys) {
iterable = formats.get(formatKey).appendTo(iterable);
}
mergedDataWriter.writeToValuesXml(key, iterable.append("</attr>"));
if (formatKeys.isEmpty()) {
mergedDataWriter.writeToValuesXml(key,
ImmutableList.of(
String.format("<!-- %s -->", source),
String.format(
"<attr name='%s'/>",
key.name())));
} else if (formats.containsKey(FLAGS) || formats.containsKey(ENUM)) {
FluentIterable<String> iterable =
FluentIterable.from(
ImmutableList.of(
String.format("<!-- %s -->", source),
formatKeys.isEmpty()
? String.format("<attr name='%s'>", key.name())
: String.format(
"<attr format='%s' name='%s' >",
Joiner.on('|').join(formatKeys), key.name())));
for (String formatKey : formatKeys) {
iterable = formats.get(formatKey).appendTo(iterable);
}
mergedDataWriter.writeToValuesXml(key, iterable.append("</attr>"));
} else {
mergedDataWriter.writeToValuesXml(key,
ImmutableList.of(
String.format("<!-- %s -->", source),
String.format(
"<attr format='%s' name='%s'/>",
Joiner.on('|').join(formatKeys),
key.name()
)));
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,6 @@ public static XmlResourceValue itemPlaceHolderFor(ResourceType resourceType) {
return withAttributes(Type.ITEM, ImmutableMap.of("type", resourceType.getName()));
}

@Deprecated
public static XmlResourceValue of(Type valueType, @Nullable String value) {
return of(valueType, ImmutableMap.<String, String>of(), value);
}

public static XmlResourceValue of(
Type valueType, ImmutableMap<String, String> attributes, @Nullable String value) {
return new SimpleXmlResourceValue(valueType, attributes, value);
Expand Down Expand Up @@ -226,7 +221,7 @@ public void write(
public static XmlResourceValue from(SerializeFormat.DataValueXml proto) {
return of(
Type.valueOf(proto.getValueType()),
ImmutableMap.copyOf(proto.getMappedStringValue()),
ImmutableMap.copyOf(proto.getMappedStringValueMap()),
proto.hasValue() ? proto.getValue() : null);
}

Expand Down

0 comments on commit 4f854d4

Please sign in to comment.