Update NIHMS metadata, Adding Grant and Funder to bulk_meta.xml #79
Conversation
- Add tests for the grants and changed test data
- Should be 8 authors now, adding 1 more PI + 2 co-pis
| @@ -210,7 +210,40 @@ private void write_metadata(Document doc) { | |||
| } | |||
| } | |||
|
|
|||
| // Could add grant information here if it was useful. | |||
| // Can only be used for a set list of funders | |||
| List<DepositMetadata.Grant> grantsList = metadata.getGrantsMetadata(); | |||
There was a problem hiding this comment.
This method is already very long. It would be good to pull this new code out into a new method.
There was a problem hiding this comment.
I agree write_metadata is rather long. I added the grants here because it fit with the rest. I didn't want to touch what was already written, but perhaps I could create a method for each component of the XML file: manuscript, article, journal etc? Something along the lines of:
private void write_metadata(Document doc) {
Element root = initializeRootElement(doc);
addManuscriptMetadata(doc, root);
addArticleMetadata(doc, root);
addJournalMetadata(doc, root);
addManuscriptTitle(doc, root);
addContacts(doc, root);
addGrants(doc, root);
}
There was a problem hiding this comment.
I thought about creating a method just for the grants, but it would seem awkward for only that component to have it's own method?
There was a problem hiding this comment.
Yes, your suggestion would be good clean up.
| Element grantElement = doc.createElement("grant"); | ||
| grantsElement.appendChild(grantElement); | ||
|
|
||
| if (grant.getGrantId() != null) { |
There was a problem hiding this comment.
Looking at these conditionals, it is possible we could end up with a grant element with no attributes. Should the conditionals be changed to check for at least one non-null attribute before creating the grant element?
Same question for the PI element below.
There was a problem hiding this comment.
This is a good point. Now that I'm thinking about it some more, I think all these attributes represent a minimum set. I would say at least for the grant element it should definitely have an award number and funder before writing out the grant element.
- I checked the data in stage, and there doesn't exist a grant without an awardnumber or funder.
- In staging, all PIs associated with a grant have a first name and last name. There are some cases where email is null.
So maybe all attributes are required, except for the email? I'm not sure if a grant should exist without a PI. I will have to double check that.
There was a problem hiding this comment.
Check has been added to ensure at least 1 nihms funder grant exists and then will create the grant element.
| grantElement.setAttribute("id", grant.getGrantId()); | ||
| } | ||
|
|
||
| if (grant.getFunder() != null) { |
There was a problem hiding this comment.
Should this be changed to use StringUtils.isNotBlank?
| @@ -241,6 +244,49 @@ public void testPackageMetadata() throws Exception { | |||
| candidate.getType() == person.getType())); | |||
| }); | |||
|
|
|||
| //Ensure that the grants in the metadata matches a Grant on the submission, Check the attributes of a grant on | |||
| //submission against what is found in the metadata | |||
| List<Element> grantElements = asList(root.getElementsByTagName("grant")); | |||
There was a problem hiding this comment.
Probably cleaner to pull this out into a separate method if not too many params are needed to be passed.
There was a problem hiding this comment.
Same question as the first comment. Should I do the same for the other components?
There was a problem hiding this comment.
This method isn't as long, so probably just adding a separate method to test the grant/funder change is fine.
|
From our discussion this morning. Will need to add the funders abbreviations that aligns with the NIHMS dtd. Will add a configuration in the repositories.json and add a property to |
- Extract out the grant and funder into a separate test
- Update sample data
| @@ -28,6 +31,9 @@ public class RepositoryDepositConfig { | |||
| @JsonProperty("mapping") | |||
| private StatusMapping statusMapping; | |||
|
|
|||
| @JsonProperty("funder-mapping") | |||
| private Map<String, String> funderMapping = new HashMap<>(); | |||
There was a problem hiding this comment.
Ah yes, that's not longer used. Good catch!
| * @return True if at least one nihms funderKey exists in the metadata grant list | ||
| */ | ||
| private boolean funderLocalKeyExists(List<String> metaDataGrantFunderKeys) { | ||
| for (String key : metaDataGrantFunderKeys) { |
There was a problem hiding this comment.
I think you can use Stream.anyMatch here.
There was a problem hiding this comment.
Yes, that's right. Much cleaner using Stream.anyMatch
| @@ -31,7 +31,44 @@ | |||
| "algorithms": [ | |||
| "sha512", | |||
| "md5" | |||
| ] | |||
| ], | |||
| "funder-mapping": { | |||
There was a problem hiding this comment.
I'm thinking we don't want this map here in the src repositories.json since it is specific to JHU. I think it should be removed, which means you will need to create a new test-repositories.json most likely to test your code, but I think it is cleaner to this for the community.
- This is institution specific. Has been moved to the test resources for testing JHU specific needs.
Adding the Grant and Funder information to the serialized XML
bulk_meta.xmlprovided to NIHMS.Unit test added/updated:
Update 1.10.2024:
funder-mappingnow exists in theoptionsof theassemblerinrespositories.json