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

Reserve PID at publication if needed #7147

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -155,32 +155,30 @@ protected void tidyUpFields(DatasetVersion dsv) {
* @param ctxt
* @throws CommandException
*/
protected void registerExternalIdentifier(Dataset theDataset, CommandContext ctxt) throws CommandException {
protected void registerExternalIdentifier(Dataset theDataset, CommandContext ctxt, boolean retry) throws CommandException {
if (!theDataset.isIdentifierRegistered()) {
GlobalIdServiceBean globalIdServiceBean = GlobalIdServiceBean.getBean(theDataset.getProtocol(), ctxt);
if ( globalIdServiceBean != null ) {
if (globalIdServiceBean instanceof FakePidProviderServiceBean) {
try {
globalIdServiceBean.createIdentifier(theDataset);
} catch (Throwable ex) {
logger.warning("Problem running createIdentifier for FakePidProvider: " + ex);
}
theDataset.setGlobalIdCreateTime(getTimestamp());
theDataset.setIdentifierRegistered(true);
return;
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 think this was just to avoid the retry attempts - same set of calls otherwise.

retry=false; //No reason to allow a retry with the FakeProvider, so set false for efficiency
}
try {
if (globalIdServiceBean.alreadyExists(theDataset)) {
int attempts = 0;

while (globalIdServiceBean.alreadyExists(theDataset) && attempts < FOOLPROOF_RETRIAL_ATTEMPTS_LIMIT) {
Copy link
Member Author

Choose a reason for hiding this comment

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

With while, there's an extra call to alreadyExists in the first iteration, so switched to do while loop.

theDataset.setIdentifier(ctxt.datasets().generateDatasetIdentifier(theDataset, globalIdServiceBean));
logger.log(Level.INFO, "Attempting to register external identifier for dataset {0} (trying: {1}).",
new Object[]{theDataset.getId(), theDataset.getIdentifier()});
attempts++;
if(retry) {
do {
theDataset.setIdentifier(ctxt.datasets().generateDatasetIdentifier(theDataset, globalIdServiceBean));
logger.log(Level.INFO, "Attempting to register external identifier for dataset {0} (trying: {1}).",
new Object[]{theDataset.getId(), theDataset.getIdentifier()});
attempts++;
} while (globalIdServiceBean.alreadyExists(theDataset) && attempts <= FOOLPROOF_RETRIAL_ATTEMPTS_LIMIT);
}

if (globalIdServiceBean.alreadyExists(theDataset)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Similar - extra call to alreadyExists since attempts will be over the limit if the while call above doesn't find alreadyExists()==false by the end.

if(!retry) {
logger.warning("Reserving PID for: " + getDataset().getId() + " during publication failed.");
throw new IllegalCommandException(BundleUtil.getStringFromBundle("publishDatasetCommand.pidNotReserved"), this);
}
if(attempts > FOOLPROOF_RETRIAL_ATTEMPTS_LIMIT) {
//Didn't work - we existed the loop with too many tries
throw new CommandExecutionException("This dataset may not be published because its identifier is already in use by another dataset; "
+ "gave up after " + attempts + " attempts. Current (last requested) identifier: " + theDataset.getIdentifier(), this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected void handlePid(Dataset theDataset, CommandContext ctxt) throws Command
GlobalIdServiceBean idServiceBean = GlobalIdServiceBean.getBean(ctxt);
if ( !idServiceBean.registerWhenPublished() ) {
// pre-register a persistent id
registerExternalIdentifier(theDataset, ctxt);
registerExternalIdentifier(theDataset, ctxt, true);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,18 @@ public Dataset execute(CommandContext ctxt) throws CommandException {
validateDataFiles(theDataset, ctxt);
// (this will throw a CommandException if it fails)
}


/*
* Try to register the dataset identifier. For PID providers that have registerWhenPublished == false (all except the FAKE provider at present)
* the registerExternalIdentifier command will make one try to create the identifier if needed (e.g. if reserving at dataset creation wasn't done/failed).
* For registerWhenPublished == true providers, if a PID conflict is found, the call will retry with new PIDs.
*/
if ( theDataset.getGlobalIdCreateTime() == null ) {
// If this were a "reservable" type of a global id
// (Datacite, EZID), an attempt to publish it would have already
// failed, in the PublishDatasetCommand earlier. So we can try
// to register it now.
// This can potentially throw a CommandException, so let's make
// sure we exit cleanly:
try {
registerExternalIdentifier(theDataset, ctxt);
// This can potentially throw a CommandException, so let's make
// sure we exit cleanly:

registerExternalIdentifier(theDataset, ctxt, false);
} catch (CommandException comEx) {
// Send failure notification to the user:
notifyUsersDatasetPublishStatus(ctxt, theDataset, UserNotification.Type.PUBLISHFAILED_PIDREG);
Expand Down Expand Up @@ -159,12 +161,12 @@ public Dataset execute(CommandContext ctxt) throws CommandException {

if (theDataset.getLatestVersion().getVersionState() != RELEASED) {
// some imported datasets may already be released.

if (!datasetExternallyReleased) {
publicizeExternalIdentifier(theDataset, ctxt);
// Will throw a CommandException, unless successful.
// This will end the execution of the command, but the method
// above takes proper care to "cleani after itself" in case of
// above takes proper care to "clean up after itself" in case of
// a failure - it will remove any locks, and it will send a
// proper notification to the user(s).
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.Optional;
import java.util.logging.Logger;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.joining;

/**
* Kick-off a dataset publication process. The process may complete immediately,
Expand Down Expand Up @@ -69,16 +68,6 @@ public PublishDatasetResult execute(CommandContext ctxt) throws CommandException

Dataset theDataset = getDataset();

// If PID can be reserved, only allow publishing if it is.
String protocol = getDataset().getProtocol();
GlobalIdServiceBean idServiceBean = GlobalIdServiceBean.getBean(protocol, ctxt);
boolean reservingPidsSupported = !idServiceBean.registerWhenPublished();
if (reservingPidsSupported) {
if (theDataset.getGlobalIdCreateTime() == null) {
throw new IllegalCommandException(BundleUtil.getStringFromBundle("publishDatasetCommand.pidNotReserved"), this);
}
}

// Set the version numbers:

if (theDataset.getPublicationDate() == null) {
Expand Down