Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

send relatedIdentifier to DataCite #4782 #4795

Merged
merged 12 commits into from
Jul 27, 2018
Merged

send relatedIdentifier to DataCite #4782 #4795

merged 12 commits into from
Jul 27, 2018

Conversation

pdurbin
Copy link
Member

@pdurbin pdurbin commented Jun 28, 2018

connects to #4782

@coveralls
Copy link

coveralls commented Jun 28, 2018

Coverage Status

Coverage decreased (-0.01%) to 15.531% when pulling 7d3e216 on 4782-datacite into 8cfca31 on develop.

@@ -1,20 +0,0 @@
<resource xsi:schemaLocation="http://datacite.org/schema/kernel-3 http://schema.datacite.org/meta/kernel-3/metadata.xsd"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this necessary anymore?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different DataCite schema version maybe (but I'd expect that to be a version change, rather than deletion)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed workingDataCiteMetadataTemplate-Files.xml because it's not used and is sitting in the root of the repo. The file that actually is being used is src/main/resources/edu/harvard/iq/dataverse/datacite_metadata_template.xml but I'm grumpy that it's in the main "dataverse" package, which is a dumping ground, rather than being in a package such as "templates" to help keep the code organized.

benjamin-martinez and others added 6 commits July 13, 2018 12:06
This a topic branch and should be merged to 4782-DataCite
 Conflicts:
	src/main/java/edu/harvard/iq/dataverse/DOIDataCiteRegisterService.java
	src/main/java/edu/harvard/iq/dataverse/DOIDataCiteServiceBean.java
	src/main/java/edu/harvard/iq/dataverse/engine/command/impl/FinalizeDatasetPublicationCommand.java


String xmlMetadata = metadataTemplate.generateXML(dvObject);
logger.log(Level.INFO, "XML to send to DataCite: {0}", xmlMetadata);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda picky, but some of the indentation and whitespace in this file could be cleaned up. Other ones seem ok though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, alt shift f does this for a full file.

@@ -144,8 +147,17 @@ private String getMetadataFromDvObject(String identifier, Map<String, String> me
}
metadataTemplate.setPublisher(producerString);
metadataTemplate.setPublisherYear(metadata.get("datacite.publicationyear"));
String xmlMetadata = metadataTemplate.generateXML();
logger.log(Level.FINE, "XML to send to DataCite: {0}", xmlMetadata);
if (dvObject.isInstanceofDataFile()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do a check for isInstanceofDataFile() a few lines up, can we collapse these two bits of logic together?

@@ -211,6 +223,26 @@ public String modifyIdentifier(String identifier, HashMap<String, String> metada
}
return retString;
}

public String setUpFileIdentifiers(DvObject dvObject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used? I tried searching it in the project and don't see it.

sb.append("</relatedIdentifiers>");
}
}
} else if (dvObject.isInstanceofDataFile()){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever a case where we have a data file but for some reason it doesn't know its owner?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure the answer to that is no. Data files will have an owner if they are in a dataset.

@@ -405,7 +405,7 @@ public void setOwner(Dataset dataset) {
// return this.fileSystemName;
// }
//
// public void setStorageIdentifier(String storageIdentifier) {
// public void setStorageIdentifier(String storageifier) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was an accident.

@@ -1378,7 +1378,7 @@ private String init(boolean initFull) {
// Set Working Version and Dataset by PersistentID
dataset = datasetService.findByGlobalId(persistentId);
if (dataset == null) {
logger.warning("No such dataset: "+persistentId);
logger.warning("++++No such dataset: "+persistentId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this and the one below also an accident? Looks weird.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to keep these warning messages in for production, its good to have a bit more description about where the error is happening so its easier to debug from the log files.

Copy link
Contributor

@matthew-a-dunlap matthew-a-dunlap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!



String xmlMetadata = metadataTemplate.generateXML(dvObject);
logger.log(Level.INFO, "XML to send to DataCite: {0}", xmlMetadata);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, alt shift f does this for a full file.

logger.log(Level.FINE, "XML to send to DataCite: {0}", xmlMetadata);

String xmlMetadata = metadataTemplate.generateXML(dvObject);
logger.log(Level.INFO, "XML to send to DataCite: {0}", xmlMetadata);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably be back to fine


datafileIdentifiers = new ArrayList<>();
for (DataFile dataFile : dataset.getFiles()) {
String add = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use a stringbuilder for performance?

…nerateRelatedIdentifiers method so it used only a StringBuilder instead of strings and StringBuilder
@kcondon kcondon merged commit 8c82278 into develop Jul 27, 2018
@kcondon kcondon deleted the 4782-datacite branch July 27, 2018 21:22
@pdurbin pdurbin added this to the 4.9.2 - Stata Upgrades, etc. milestone Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants