From 88d74ec764ca879b91375d123cf9cf3c695b3792 Mon Sep 17 00:00:00 2001 From: cramakri <cramakri> Date: Wed, 15 Dec 2010 14:29:12 +0000 Subject: [PATCH] LMS-1909 Fill out DataSetInformation better. More robust error reporting. SVN: 19154 --- .../AlwaysIdentifiedDataStrategyStore.java | 36 -------- .../DataSetRegistrationAlgorithm.java | 6 +- .../etlserver/IExtensibleDataSetHandler.java | 2 +- .../etlserver/TransferredDataSetHandler.java | 9 +- .../SampleAndDataSetControlFileProcessor.java | 25 +++-- .../SampleAndDataSetFolderProcessor.java | 2 +- .../SampleAndDataSetRegistrationHandler.java | 2 +- .../SampleAndDataSetRegistrator.java | 92 ++++++++++++++++--- .../entityregistration/SampleDataSetPair.java | 3 + 9 files changed, 108 insertions(+), 69 deletions(-) delete mode 100644 datastore_server/source/java/ch/systemsx/cisd/etlserver/AlwaysIdentifiedDataStrategyStore.java diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/AlwaysIdentifiedDataStrategyStore.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/AlwaysIdentifiedDataStrategyStore.java deleted file mode 100644 index 83b2654a3d5..00000000000 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/AlwaysIdentifiedDataStrategyStore.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2010 ETH Zuerich, CISD - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package ch.systemsx.cisd.etlserver; - -import java.io.File; - -import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; - -/** - * @author Chandrasekhar Ramakrishnan - */ -public class AlwaysIdentifiedDataStrategyStore implements IDataStrategyStore -{ - private final IdentifiedDataStrategy identifiedStrategy = new IdentifiedDataStrategy(); - - public IDataStoreStrategy getDataStoreStrategy(DataSetInformation dataSetInfo, - File incomingDataSetPath) - { - return identifiedStrategy; - } - -} diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/DataSetRegistrationAlgorithm.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/DataSetRegistrationAlgorithm.java index 58e80921576..4165183217a 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/DataSetRegistrationAlgorithm.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/DataSetRegistrationAlgorithm.java @@ -205,6 +205,7 @@ public abstract class DataSetRegistrationAlgorithm * Registers the data set. */ private void registerDataSetAndInitiateProcessing(final String procedureTypeCode) + throws Throwable { final File markerFile = createProcessingMarkerFile(); try @@ -312,6 +313,7 @@ public abstract class DataSetRegistrationAlgorithm private final void plainRegisterDataSet(NewExternalData data, final String relativePath, final StorageFormat storageFormat, final BooleanOrUnknown isCompleteFlag) + throws Throwable { updateExternalData(data, relativePath, storageFormat, isCompleteFlag); // Finally: register the data set in the database. @@ -320,8 +322,10 @@ public abstract class DataSetRegistrationAlgorithm /** * Contact openBis and register the data set there. Subclasses may override. + * + * @throws Throwable */ - protected void registerDataSetInApplicationServer(NewExternalData data) + protected void registerDataSetInApplicationServer(NewExternalData data) throws Throwable { getOpenBisService().registerDataSet(dataSetInformation, data); } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/IExtensibleDataSetHandler.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/IExtensibleDataSetHandler.java index e80c6ce2774..210e8f8ca98 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/IExtensibleDataSetHandler.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/IExtensibleDataSetHandler.java @@ -29,7 +29,7 @@ public interface IExtensibleDataSetHandler extends IDataSetHandler { public static interface IDataSetRegistrator { - public void registerDataSetInApplicationServer(NewExternalData data); + public void registerDataSetInApplicationServer(NewExternalData data) throws Throwable; } public List<DataSetInformation> handleDataSet(final File dataSet, diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/TransferredDataSetHandler.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/TransferredDataSetHandler.java index 809b0b275d0..9fdb8dcc384 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/TransferredDataSetHandler.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/TransferredDataSetHandler.java @@ -609,16 +609,9 @@ public final class TransferredDataSetHandler implements IPathHandler, ISelfTesta } @Override - protected void registerDataSetInApplicationServer(NewExternalData data) + protected void registerDataSetInApplicationServer(NewExternalData data) throws Throwable { registrator.registerDataSetInApplicationServer(data); } - - @Override - protected IDataStrategyStore getDataStrategyStore() - { - return new AlwaysIdentifiedDataStrategyStore(); - } - } } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetControlFileProcessor.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetControlFileProcessor.java index 24403c55957..c73fab86951 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetControlFileProcessor.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetControlFileProcessor.java @@ -34,6 +34,7 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException; import ch.systemsx.cisd.common.io.DelegatedReader; import ch.systemsx.cisd.common.mail.EMailAddress; import ch.systemsx.cisd.common.utilities.UnicodeUtils; +import ch.systemsx.cisd.etlserver.entityregistration.SampleAndDataSetRegistrator.RegistrationErrorWrapper; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Person; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.SampleType; @@ -48,8 +49,8 @@ class SampleAndDataSetControlFileProcessor extends AbstractSampleAndDataSetProce { private final File controlFile; - private final HashMap<SampleDataSetPair, Exception> errorMap = - new HashMap<SampleDataSetPair, Exception>(); + private final HashMap<SampleDataSetPair, RegistrationErrorWrapper> errorMap = + new HashMap<SampleDataSetPair, RegistrationErrorWrapper>(); /** * Utility class for accessing the properties defined in a control file. @@ -235,12 +236,13 @@ class SampleAndDataSetControlFileProcessor extends AbstractSampleAndDataSetProce List<SampleDataSetPair> loadedSampleDataSetPairs = null; + Reader reader = UnicodeUtils.createReader(new FileInputStream(controlFile)); + DelegatedReader delegatedReader = new DelegatedReader(reader, controlFile.getName()); + try { properties.checkValidity(); - Reader reader = UnicodeUtils.createReader(new FileInputStream(controlFile)); - loadedSampleDataSetPairs = - controlFileLoader.load(new DelegatedReader(reader, controlFile.getName())); + loadedSampleDataSetPairs = controlFileLoader.load(delegatedReader); } catch (UserFailureException e) { // If we don't know which user to send the email to, don't handle this error -- leave it @@ -252,6 +254,15 @@ class SampleAndDataSetControlFileProcessor extends AbstractSampleAndDataSetProce sendEmailWithErrorMessage(properties, e.getMessage()); return; + } finally + { + try + { + delegatedReader.close(); + } catch (IOException ex) + { + // Ignore this failure + } } String userId = properties.getUser().getUserId(); @@ -261,7 +272,7 @@ class SampleAndDataSetControlFileProcessor extends AbstractSampleAndDataSetProce sampleDataSet.getDataSetInformation().setUploadingUserId(userId); SampleAndDataSetRegistrator registrator = new SampleAndDataSetRegistrator(folder, properties, sampleDataSet); - Exception resultOrNull = registrator.register(); + RegistrationErrorWrapper resultOrNull = registrator.register(); if (null != resultOrNull) { errorMap.put(sampleDataSet, resultOrNull); @@ -280,7 +291,7 @@ class SampleAndDataSetControlFileProcessor extends AbstractSampleAndDataSetProce sb.append("Encountered errors in the following lines:\n"); for (SampleDataSetPair pair : errorMap.keySet()) { - Exception error = errorMap.get(pair); + RegistrationErrorWrapper error = errorMap.get(pair); sb.append("# "); sb.append(error.getMessage()); sb.append("\n"); diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetFolderProcessor.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetFolderProcessor.java index 9e790196c38..5de7a300775 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetFolderProcessor.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetFolderProcessor.java @@ -69,7 +69,7 @@ class SampleAndDataSetFolderProcessor extends AbstractSampleAndDataSetProcessor new SampleAndDataSetControlFileProcessor(globalState, folder, controlFile); controlFileProcessor.register(); - } catch (Exception e) + } catch (UserFailureException e) { errorMap.put(controlFile, e); } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrationHandler.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrationHandler.java index 0378b02fbc4..2020b85929b 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrationHandler.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrationHandler.java @@ -145,7 +145,7 @@ public class SampleAndDataSetRegistrationHandler implements IDataSetHandlerWithM folderProcessor.register(); } catch (Exception ex) { - operationLog.error(ex); + operationLog.error("Could not register samples / data sets in ", ex); throw new CheckedExceptionTunnel(ex); } finally { diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrator.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrator.java index b6ce51d9b92..53f5fb60727 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrator.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleAndDataSetRegistrator.java @@ -18,14 +18,15 @@ package ch.systemsx.cisd.etlserver.entityregistration; import java.io.File; -import ch.systemsx.cisd.base.exceptions.CheckedExceptionTunnel; import ch.systemsx.cisd.common.exceptions.UserFailureException; import ch.systemsx.cisd.etlserver.IExtensibleDataSetHandler; import ch.systemsx.cisd.etlserver.TransferredDataSetHandler; import ch.systemsx.cisd.etlserver.entityregistration.SampleAndDataSetControlFileProcessor.ControlFileRegistrationProperties; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.FileFormatType; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Sample; import ch.systemsx.cisd.openbis.generic.shared.dto.NewExternalData; +import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifier; /** * Utitlity class for registering one sample/dataset combination @@ -35,6 +36,47 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.NewExternalData; class SampleAndDataSetRegistrator extends AbstractSampleAndDataSetProcessor implements TransferredDataSetHandler.IDataSetRegistrator { + + /** + * A wrapper around errors encountered during registration. + * + * @author Chandrasekhar Ramakrishnan + */ + static class RegistrationErrorWrapper + { + private final boolean isError; + + private final Throwable errorOrNull; + + RegistrationErrorWrapper(Throwable errorOrNull) + { + this.errorOrNull = errorOrNull; + isError = errorOrNull != null; + } + + public boolean isError() + { + return isError; + } + + public Throwable getErrorOrNull() + { + return errorOrNull; + } + + public String getMessage() + { + if (isError) + { + return errorOrNull.getMessage(); + } else + { + return "Success"; + } + } + + } + private final ControlFileRegistrationProperties properties; private final SampleDataSetPair sampleDataSetPair; @@ -42,7 +84,7 @@ class SampleAndDataSetRegistrator extends AbstractSampleAndDataSetProcessor impl // State that is updated during the registration private boolean didSucceed = false; - private Exception failureException = null; + private RegistrationErrorWrapper failureException = null; SampleAndDataSetRegistrator(File folder, ControlFileRegistrationProperties properties, SampleDataSetPair sampleDataSetPair) @@ -57,13 +99,16 @@ class SampleAndDataSetRegistrator extends AbstractSampleAndDataSetProcessor impl * * @return An exception if one was encountered, otherwise null. */ - public Exception register() + public RegistrationErrorWrapper register() { File dataSetFile = new File(folder, sampleDataSetPair.getFolderName()); - Exception isEmptyException = checkDataSetFileNotEmpty(dataSetFile); - if (null != isEmptyException) + try { - return isEmptyException; + checkDataSetFileNotEmpty(dataSetFile); + checkExperimentExists(); + } catch (UserFailureException ex) + { + return new RegistrationErrorWrapper(ex); } if (globalState.getDelegator() instanceof IExtensibleDataSetHandler) @@ -79,13 +124,30 @@ class SampleAndDataSetRegistrator extends AbstractSampleAndDataSetProcessor impl return failureException; } - private Exception checkDataSetFileNotEmpty(File dataSetFile) + private void checkDataSetFileNotEmpty(File dataSetFile) { if (0 == dataSetFile.list().length) { - return new UserFailureException("The data set folder cannot be empty"); + throw new UserFailureException("The data set folder cannot be empty"); } - return null; + } + + private void checkExperimentExists() + { + ExperimentIdentifier experimentId = + sampleDataSetPair.getDataSetInformation().getExperimentIdentifier(); + if (null == experimentId) + { + throw new UserFailureException("An experiment identifier must be specified"); + } + + Experiment experiment = globalState.getOpenbisService().tryToGetExperiment(experimentId); + if (null == experiment) + { + throw new UserFailureException("The experiment with identifier " + experimentId + + " does not exist"); + } + sampleDataSetPair.getDataSetInformation().setExperiment(experiment); } private void logDataRegistered() @@ -94,7 +156,7 @@ class SampleAndDataSetRegistrator extends AbstractSampleAndDataSetProcessor impl globalState.getOperationLog().info(message); } - public void registerDataSetInApplicationServer(NewExternalData data) + public void registerDataSetInApplicationServer(NewExternalData data) throws Throwable { data.setDataSetType(properties.getDataSetType()); if (null != sampleDataSetPair.getFileFormatTypeCode()) @@ -116,13 +178,15 @@ class SampleAndDataSetRegistrator extends AbstractSampleAndDataSetProcessor impl } catch (UserFailureException e) { didSucceed = false; - failureException = e; + failureException = new RegistrationErrorWrapper(e); throw e; - } catch (Exception e) + } catch (Throwable e) { + globalState.getOperationLog().error( + "Could not register " + sampleDataSetPair + " in openBIS", e); didSucceed = false; - failureException = e; - throw new CheckedExceptionTunnel(e); + failureException = new RegistrationErrorWrapper(e); + throw e; } } diff --git a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleDataSetPair.java b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleDataSetPair.java index b94344a616a..7431b76730c 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleDataSetPair.java +++ b/datastore_server/source/java/ch/systemsx/cisd/etlserver/entityregistration/SampleDataSetPair.java @@ -26,6 +26,7 @@ import ch.systemsx.cisd.openbis.dss.generic.shared.dto.DataSetInformation; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.IEntityProperty; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample; import ch.systemsx.cisd.openbis.generic.shared.dto.NewProperty; +import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory; /** * An object that represents a sample/data set pair defined in a file @@ -106,6 +107,8 @@ public class SampleDataSetPair public void setSampleExperimentIdentifier(String experimentIdentifier) { this.newSample.setExperimentIdentifier(experimentIdentifier); + this.dataSetInformation.setExperimentIdentifier(new ExperimentIdentifierFactory( + experimentIdentifier).createIdentifier()); } @BeanProperty(label = DATA_SET_CODE, optional = true) -- GitLab