Reading metadata from manifest#1861
Conversation
WonderCsabo
left a comment
There was a problem hiding this comment.
Thanks for this pull request!
Can you address my comments and squash your commits into a single commit?
This will collect all <meta-data> elements from all components into a single map. Is this okay for the general use-case?
| } | ||
|
|
||
| public Map<String, String> getMetaDataQualifiedNames() { | ||
| return metaDataQualifiedNames; |
There was a problem hiding this comment.
Please use Collections.unmodifiableMap(metaDataQualifiedNames) here.
| Node nameAttribute = node.getAttributes().getNamedItem("android:name"); | ||
| Node valueAttribute = node.getAttributes().getNamedItem("android:value"); | ||
|
|
||
| if (nameAttribute == null || valueAttribute == null) { |
There was a problem hiding this comment.
Can you flatten this if?
if (nameAttribute == null && valueAttribute == null) {
// malformed
} else if (nameAttribute != null) {
// malformed with name
} else {
// add it
}There was a problem hiding this comment.
Hi, I will make a review, and I will check directly the docs https://developer.android.com/guide/topics/manifest/meta-data-element.html, in General, I solved it to the needs of collecting the tags from within the manifest, and following the general structure:
<meta-data android:name="zoo" android:value="@string/kangaroo" />I used also a Key-Value (a Map) to store and read all the elements. Checking in the docs, there's also a 3rd attribute that can be written in the Manifest, android:resource, never use it before to be honest, but in the general case should be included.
Maybe return a Map<String, SomeObject>, with SomeObject being a pair Value, Resource?.
On the other hands, I'm collecting the Metadata at the root level (Application), but it can be applied to:
<activity>
<activity-alias>
<application>
<provider>
<receiver>
<service>In a general scenario, a reference to the parent tag should be also stored.
Any suggestion about this?
.
There was a problem hiding this comment.
Checking now the code, I see that also a specific method is called to collect other tasks, I will do the same, to follow the same "rules". This was something I did like 1year ago, so probably the structure of the file was different for then, I will review this too.
There was a problem hiding this comment.
Ah, about "flatting the if", now checking, I followed same structure for collecting other tags.. Do I flag them all?
private List<String> extractComponentNames(String applicationPackage, NodeList componentNodes) {
List<String> componentQualifiedNames = new ArrayList<>();
for (int i = 0; i < componentNodes.getLength(); i++) {
Node activityNode = componentNodes.item(i);
Node nameAttribute = activityNode.getAttributes().getNamedItem("android:name");
String qualifiedName = manifestNameToValidQualifiedName(applicationPackage, nameAttribute);
if (qualifiedName != null) {
componentQualifiedNames.add(qualifiedName);
} else {
if (nameAttribute != null) {
LOGGER.warn("A class activity declared in the AndroidManifest.xml cannot be found in the compile path: [{}]", nameAttribute.getNodeValue());
} else {
LOGGER.warn("The {} activity node in the AndroidManifest.xml has no android:name attribute", i);
}
}
}
return componentQualifiedNames;
}There was a problem hiding this comment.
OOps, i missed the already existing ifs. In this case, i think you did the right thing, this way the code is consistent. We can refactor all ifs later.
There was a problem hiding this comment.
i think documentElement.getElementsByTagName() will traverse this recursively and return all <meta-data> in components as well, doesn't it?
There was a problem hiding this comment.
Yes it returns all the tags....
2103b7f to
508bf6c
Compare
|
Hi, I updated the requested changes, and merged all into a single commit. |
|
Thanks for your work! |
#1860 .