diff --git a/dspace-api/src/main/java/org/dspace/app/bulkedit/BulkImport.java b/dspace-api/src/main/java/org/dspace/app/bulkedit/BulkImport.java index ba348be0fe4..999839ed3ce 100644 --- a/dspace-api/src/main/java/org/dspace/app/bulkedit/BulkImport.java +++ b/dspace-api/src/main/java/org/dspace/app/bulkedit/BulkImport.java @@ -59,7 +59,7 @@ import org.dspace.app.bulkimport.model.ImportAction; import org.dspace.app.bulkimport.model.MetadataGroup; import org.dspace.app.bulkimport.model.UploadDetails; -import org.dspace.app.bulkimport.util.BulkImportFileUtil; +import org.dspace.app.bulkimport.util.ImportFileUtil; import org.dspace.app.util.DCInputsReader; import org.dspace.app.util.DCInputsReaderException; import org.dspace.authority.service.ItemSearchService; @@ -210,7 +210,7 @@ public class BulkImport extends DSpaceRunnable inputStream = bulkImportFileUtil.getInputStream(filePath); + Optional inputStream = importFileUtil.getInputStream(filePath); if (inputStream.isEmpty()) { handler.logError("Cannot create bitstream from file at path " + filePath); @@ -1633,6 +1635,10 @@ private boolean isAppendModeDisabled() { return !configurationService.getBooleanProperty("core.authorization.installitem.inheritance-read.append-mode"); } + public void setImportFileUtil(ImportFileUtil importFileUtil) { + this.importFileUtil = importFileUtil; + } + @Override @SuppressWarnings("unchecked") public BulkImportScriptConfiguration getScriptConfiguration() { diff --git a/dspace-api/src/main/java/org/dspace/app/bulkimport/util/BulkImportFileUtil.java b/dspace-api/src/main/java/org/dspace/app/bulkimport/util/BulkImportFileUtil.java deleted file mode 100644 index d9c1c6785c2..00000000000 --- a/dspace-api/src/main/java/org/dspace/app/bulkimport/util/BulkImportFileUtil.java +++ /dev/null @@ -1,125 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ - -package org.dspace.app.bulkimport.util; - -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.net.URL; -import java.net.URLConnection; -import java.util.Optional; - -import org.apache.commons.io.FileUtils; -import org.apache.commons.lang3.StringUtils; -import org.dspace.scripts.handler.DSpaceRunnableHandler; -import org.dspace.services.ConfigurationService; -import org.dspace.utils.DSpace; - -/* - * @author Jurgen Mamani - */ -public class BulkImportFileUtil { - - public static final String REMOTE = "REMOTE"; - - private static final String LOCAL = "LOCAL"; - - public static final String FTP = "FTP"; - - private static final String HTTP_PREFIX = "http:"; - - private static final String HTTPS_PREFIX = "https:"; - - private static final String LOCAL_PREFIX = "file:"; - - private static final String FTP_PREFIX = "ftp:"; - - private static final String UNKNOWN = "UNKNOWN"; - - protected DSpaceRunnableHandler handler; - - public BulkImportFileUtil(DSpaceRunnableHandler handler) { - this.handler = handler; - } - - public Optional getInputStream(String path) { - String fileLocationType = getFileLocationTypeByPath(path); - - if (UNKNOWN.equals(fileLocationType)) { - handler.logWarning("File path is of UNKNOWN type: [" + path + "]"); - return Optional.empty(); - } - - return getInputStream(path, fileLocationType); - } - - private String getFileLocationTypeByPath(String path) { - if (StringUtils.isNotBlank(path)) { - if (path.startsWith(HTTP_PREFIX) || path.startsWith(HTTPS_PREFIX)) { - return REMOTE; - } else if (path.startsWith(LOCAL_PREFIX)) { - return LOCAL; - } else if (path.startsWith(FTP_PREFIX)) { - return FTP; - } else { - return UNKNOWN; - } - } - - return UNKNOWN; - } - - private Optional getInputStream(String path, String fileLocationType) { - try { - switch (fileLocationType) { - case REMOTE: - return Optional.of(getInputStreamOfRemoteFile(path)); - case LOCAL: - return Optional.of(getInputStreamOfLocalFile(path)); - case FTP: - return Optional.of(getInputStreamOfFtpFile(path)); - default: - } - } catch (IOException e) { - handler.logError(e.getMessage()); - } - - return Optional.empty(); - } - - - private InputStream getInputStreamOfLocalFile(String path) throws IOException { - String orginalPath = path; - path = path.replace(LOCAL_PREFIX + "//", ""); - ConfigurationService configurationService = new DSpace().getConfigurationService(); - String bulkUploadFolder = configurationService.getProperty("bulk-uploads.local-folder"); - if (!StringUtils.startsWith(path, "/")) { - path = bulkUploadFolder + (StringUtils.endsWith(bulkUploadFolder, "/") ? path : "/" + path); - } - File file = new File(path); - String canonicalPath = file.getCanonicalPath(); - if (!StringUtils.startsWith(canonicalPath, bulkUploadFolder)) { - throw new IOException("Access to the specified file " + orginalPath + " is not allowed"); - } - if (!file.exists()) { - throw new IOException("file " + orginalPath + " is not found"); - } - return FileUtils.openInputStream(file); - } - - private InputStream getInputStreamOfRemoteFile(String url) throws IOException { - return new URL(url).openStream(); - } - - private InputStream getInputStreamOfFtpFile(String url) throws IOException { - URL urlObject = new URL(url); - URLConnection urlConnection = urlObject.openConnection(); - return urlConnection.getInputStream(); - } -} diff --git a/dspace-api/src/main/java/org/dspace/app/bulkimport/util/ImportFileUtil.java b/dspace-api/src/main/java/org/dspace/app/bulkimport/util/ImportFileUtil.java new file mode 100644 index 00000000000..35277373794 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/app/bulkimport/util/ImportFileUtil.java @@ -0,0 +1,181 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ + +package org.dspace.app.bulkimport.util; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.URL; +import java.net.URLConnection; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Optional; +import java.util.Set; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.dspace.scripts.handler.DSpaceRunnableHandler; +import org.dspace.services.factory.DSpaceServicesFactory; + +/* + * @author Jurgen Mamani + */ +public class ImportFileUtil { + + private static final Logger log = LogManager.getLogger(ImportFileUtil.class); + + public static final String REMOTE = "REMOTE"; + + private static final String LOCAL = "LOCAL"; + + public static final String FTP = "FTP"; + + private static final String HTTP_PREFIX = "http:"; + + private static final String HTTPS_PREFIX = "https:"; + + private static final String LOCAL_PREFIX = "file:"; + + private static final String FTP_PREFIX = "ftp:"; + + private static final String UNKNOWN = "UNKNOWN"; + + /** + * This method check if the given {@param possibleChild} is contained in the specified + * {@param possibleParent}, i.e. it's a sub-path of it. + * + * @param possibleParent + * @param possibleChild + * @return true if sub-path, false otherwise. + */ + private static boolean isChild(File possibleParent, File possibleChild) { + File parent = possibleChild.getParentFile(); + while (parent != null) { + if (parent.equals(possibleParent)) { + return true; + } + parent = parent.getParentFile(); + } + return false; + } + + private static String[] getAllowedIps() { + return DSpaceServicesFactory.getInstance() + .getConfigurationService() + .getArrayProperty("allowed.ips.import"); + } + + protected DSpaceRunnableHandler handler; + + public ImportFileUtil() {} + + public ImportFileUtil(DSpaceRunnableHandler handler) { + this.handler = handler; + } + + public Optional getInputStream(String path) { + String fileLocationType = getFileLocationTypeByPath(path); + + if (UNKNOWN.equals(fileLocationType)) { + logWarning("File path is of UNKNOWN type: [" + path + "]"); + return Optional.empty(); + } + + return getInputStream(path, fileLocationType); + } + + protected void logWarning(String message) { + log.warn(message); + if (handler != null) { + handler.logWarning(message); + } + } + + private String getFileLocationTypeByPath(String path) { + if (StringUtils.isNotBlank(path)) { + if (path.startsWith(HTTP_PREFIX) || path.startsWith(HTTPS_PREFIX)) { + return REMOTE; + } else if (path.startsWith(LOCAL_PREFIX)) { + return LOCAL; + } else if (path.startsWith(FTP_PREFIX)) { + return FTP; + } else { + return UNKNOWN; + } + } + + return UNKNOWN; + } + + private Optional getInputStream(String path, String fileLocationType) { + try { + switch (fileLocationType) { + case REMOTE: + return getInputStreamOfRemoteFile(path); + case LOCAL: + return getInputStreamOfLocalFile(path); + case FTP: + return getInputStreamOfFtpFile(path); + default: + } + } catch (IOException e) { + logError(e.getMessage()); + } + + return Optional.empty(); + } + + private void logError(String message) { + log.error(message); + if (handler != null) { + handler.logError(message); + } + } + + private Optional getInputStreamOfLocalFile(String path) throws IOException { + Path uploadPath = Paths.get( + DSpaceServicesFactory.getInstance() + .getConfigurationService() + .getProperty("uploads.local-folder") + ); + File file = uploadPath.resolve(path.replace(LOCAL_PREFIX + "//", "")) + .toFile() + .getCanonicalFile(); + File possibleParent = uploadPath.toFile().getCanonicalFile(); + if (!isChild(possibleParent, file)) { + throw new IOException("Access to the specified file " + path + " is not allowed"); + } + if (!file.exists()) { + throw new IOException("file " + path + " is not found"); + } + return Optional.of(FileUtils.openInputStream(file)); + } + + private Optional getInputStreamOfRemoteFile(String path) throws IOException { + return Optional.of(new URL(path)) + .filter(url -> Set.of(getAllowedIps()).contains(url.getHost())) + .map(this::openStream); + } + + protected InputStream openStream(URL url) { + try { + return url.openStream(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private Optional getInputStreamOfFtpFile(String url) throws IOException { + URL urlObject = new URL(url); + URLConnection urlConnection = urlObject.openConnection(); + return Optional.of(urlConnection.getInputStream()); + } +} diff --git a/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java index a884f9b0756..7c80e1ea7dc 100644 --- a/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java @@ -490,7 +490,7 @@ public void exportAsZip(Context context, Iterator items, File wkDir = new File(workDir); if (!wkDir.exists() && !wkDir.mkdirs()) { - logError("Unable to create working direcory"); + logError("Unable to create working directory"); } File dnDir = new File(destDirName); @@ -498,11 +498,18 @@ public void exportAsZip(Context context, Iterator items, logError("Unable to create destination directory"); } - // export the items using normal export method - exportItem(context, items, workDir, seqStart, migrate, excludeBitstreams); + try { + // export the items using normal export method (this exports items to our workDir) + exportItem(context, items, workDir, seqStart, migrate, excludeBitstreams); - // now zip up the export directory created above - zip(workDir, destDirName + System.getProperty("file.separator") + zipFileName); + // now zip up the workDir directory created above + zip(workDir, destDirName + System.getProperty("file.separator") + zipFileName); + } finally { + // Cleanup workDir created above, if it still exists + if (wkDir.exists()) { + deleteDirectory(wkDir); + } + } } @Override diff --git a/dspace-api/src/main/java/org/dspace/content/authority/OrcidAuthority.java b/dspace-api/src/main/java/org/dspace/content/authority/OrcidAuthority.java index 1b88b784c01..df21cbc8b3c 100644 --- a/dspace-api/src/main/java/org/dspace/content/authority/OrcidAuthority.java +++ b/dspace-api/src/main/java/org/dspace/content/authority/OrcidAuthority.java @@ -18,6 +18,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.commons.lang3.ArrayUtils; @@ -49,6 +51,10 @@ public class OrcidAuthority extends ItemAuthority { public static final String DEFAULT_INSTITUTION_KEY = "oairecerif_author_affiliation"; + public static final String ORCID_REGEX = "\\d{4}-\\d{4}-\\d{4}-\\d{4}"; + + public static final Pattern ORCID_PATTERN = Pattern.compile(ORCID_REGEX); + private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); private static String accessToken; @@ -92,6 +98,11 @@ private Choices performOrcidSearch(String text, int start, int rows) { } private String formatQuery(String text) { + text = text.trim(); + Matcher matcher = ORCID_PATTERN.matcher(text); + if (matcher.matches()) { + return format("(orcid:%s)", text); + } return Arrays.stream(replaceCommaWithSpace(text).split(" ")) .map(name -> format("(given-names:%s+OR+family-name:%s+OR+other-names:%s)", name, name, name)) .collect(Collectors.joining("+AND+")); diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java index 4bebcd95cf0..02203db4e8b 100644 --- a/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java +++ b/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java @@ -26,11 +26,14 @@ import com.amazonaws.AmazonClientException; import com.amazonaws.ClientConfiguration; -import com.amazonaws.SystemDefaultDnsResolver; +import com.amazonaws.ClientConfigurationFactory; import com.amazonaws.auth.AWSCredentials; +import com.amazonaws.auth.AWSCredentialsProvider; import com.amazonaws.auth.AWSStaticCredentialsProvider; import com.amazonaws.auth.BasicAWSCredentials; +import com.amazonaws.auth.DefaultAWSCredentialsProviderChain; import com.amazonaws.client.builder.AwsClientBuilder.EndpointConfiguration; +import com.amazonaws.regions.DefaultAwsRegionProviderChain; import com.amazonaws.regions.Region; import com.amazonaws.regions.Regions; import com.amazonaws.services.s3.AmazonS3; @@ -42,6 +45,8 @@ import com.amazonaws.services.s3.transfer.TransferManager; import com.amazonaws.services.s3.transfer.TransferManagerBuilder; import com.amazonaws.services.s3.transfer.Upload; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.DefaultParser; import org.apache.commons.cli.HelpFormatter; @@ -75,6 +80,7 @@ public class S3BitStoreService extends BaseBitStoreService { protected static final String DEFAULT_BUCKET_PREFIX = "dspace-asset-"; + protected static final Gson GSON = new GsonBuilder().serializeNulls().setPrettyPrinting().create(); // Prefix indicating a registered bitstream protected final String REGISTERED_FLAG = "-R"; /** @@ -147,30 +153,37 @@ public class S3BitStoreService extends BaseBitStoreService { protected static Supplier getClientConfiguration( Integer maxConnections, Integer connectionTimeout ) { - return () -> new ClientConfiguration() - .withDnsResolver( - new SystemDefaultDnsResolver() - ) - .withMaxConnections( - Optional.ofNullable(maxConnections).orElse(ClientConfiguration.DEFAULT_MAX_CONNECTIONS) - ) - .withConnectionTimeout( - Optional.ofNullable(connectionTimeout).orElse(ClientConfiguration.DEFAULT_CONNECTION_TIMEOUT) + return () -> { + ClientConfiguration clientConfiguration = + new ClientConfigurationFactory().getConfig() + .withMaxConnections( + Optional.ofNullable(maxConnections) + .orElse(ClientConfiguration.DEFAULT_MAX_CONNECTIONS) + ) + .withConnectionTimeout( + Optional.ofNullable(connectionTimeout) + .orElse(ClientConfiguration.DEFAULT_CONNECTION_TIMEOUT) + ); + log.debug( + "AmazonS3Client client configuration: {}", + GSON.toJson(clientConfiguration) ); + return clientConfiguration; + }; } /** * Utility method for generate AmazonS3 builder * - * @param regions wanted regions in client + * @param regionsSupplier wanted regionsSupplier in client * @param credentialsProvider credentials of the client * @param clientConfiguration client connection details * @param endpoint optional custom endpoint * @return builder with the specified parameters */ protected static Supplier amazonClientBuilderBy( - @NotNull Regions regions, - @NotNull Supplier credentialsProvider, + @NotNull Supplier regionsSupplier, + @NotNull Supplier credentialsProvider, @NotNull Supplier clientConfiguration, String endpoint ) { @@ -178,9 +191,8 @@ protected static Supplier amazonClientBuilderBy( withEndpointConfiguration( AmazonS3ClientBuilder.standard() .withCredentials(credentialsProvider.get()) - //.withClientConfiguration(clientConfiguration.get()) - , - regions, + .withClientConfiguration(clientConfiguration.get()), + regionsSupplier.get(), endpoint ).build(); } @@ -189,10 +201,12 @@ protected static Supplier amazonClientBuilderBy( * Utility method for generate AmazonS3 builder * * @param clientConfigurationSupplier client connection details + * @param regionsSupplier the region of the configured endpoint * @param endpoint optional custom endpoint * @return builder with the specified parameters */ protected static Supplier amazonClientBuilderBy( + @NotNull Supplier regionsSupplier, @NotNull Supplier clientConfigurationSupplier, String endpoint ) { @@ -200,7 +214,7 @@ protected static Supplier amazonClientBuilderBy( withEndpointConfiguration( AmazonS3ClientBuilder.standard() .withClientConfiguration(clientConfigurationSupplier.get()), - Regions.DEFAULT_REGION, + regionsSupplier.get(), endpoint ).build(); } @@ -224,8 +238,16 @@ protected static AmazonS3ClientBuilder withEndpointConfiguration( if (StringUtils.isNotBlank(endpoint)) { clientBuilder = clientBuilder.withEndpointConfiguration(getEndpointConfiguration(endpoint, regions)); + log.info( + "AmazonS3Client endpoint-configuration: {}", + GSON.toJson(clientBuilder.getEndpoint()) + ); } else { clientBuilder = clientBuilder.withRegion(regions); + log.info( + "AmazonS3Client regions: {}", + GSON.toJson(clientBuilder.getRegion()) + ); } return clientBuilder; } @@ -239,9 +261,13 @@ protected static EndpointConfiguration getEndpointConfiguration(String endpoint, protected static Supplier getAwsCredentialsSupplier( String awsAccessKey, String awsSecretKey ) { - return getAwsCredentialsSupplier( - new BasicAWSCredentials(awsAccessKey, awsSecretKey) + BasicAWSCredentials credentials = new BasicAWSCredentials(awsAccessKey, awsSecretKey); + log.info( + "AmazonS3Client credentials - accessKey: {}, secretKey: {}", + credentials.getAWSAccessKeyId().replaceFirst("^(.{3})(.*)(.{3})$", "$1***$3"), + credentials.getAWSSecretKey().replaceFirst("^(.{3})(.*)(.{3})$", "$1***$3") ); + return getAwsCredentialsSupplier(credentials); } protected static Supplier getAwsCredentialsSupplier( @@ -250,6 +276,21 @@ protected static Supplier getAwsCredentialsSupplie return () -> new AWSStaticCredentialsProvider(credentials); } + protected static Regions getDefaultRegion() { + return Optional.ofNullable(new DefaultAwsRegionProviderChain().getRegion()) + .filter(StringUtils::isNotBlank) + .map(S3BitStoreService::parseRegion) + .orElse(Regions.DEFAULT_REGION); + } + + private static Regions parseRegion(String awsRegionName) { + try { + return Regions.fromName(awsRegionName); + } catch (IllegalArgumentException e) { + log.warn("Invalid aws_region: " + awsRegionName); + } + return null; + } public S3BitStoreService() {} @@ -282,40 +323,25 @@ public void init() throws IOException { } try { + Supplier awsCredentialsSupplier; if (StringUtils.isNotBlank(getAwsAccessKey()) && StringUtils.isNotBlank(getAwsSecretKey())) { log.warn("Use local defined S3 credentials"); - // region - Regions regions = Regions.DEFAULT_REGION; - if (StringUtils.isNotBlank(awsRegionName)) { - try { - regions = Regions.fromName(awsRegionName); - } catch (IllegalArgumentException e) { - log.warn("Invalid aws_region: " + awsRegionName); - } - } - // init client - s3Service = - FunctionalUtils.getDefaultOrBuild( - this.s3Service, - amazonClientBuilderBy( - regions, - getAwsCredentialsSupplier(getAwsAccessKey(), getAwsSecretKey()), - getClientConfiguration(maxConnections, connectionTimeout), - endpoint - ) - ); - log.warn("S3 Region set to: " + regions.getName()); + awsCredentialsSupplier = getAwsCredentialsSupplier(getAwsAccessKey(), getAwsSecretKey()); } else { - log.info("Using a IAM role or aws environment credentials"); - s3Service = - FunctionalUtils.getDefaultOrBuild( - this.s3Service, - amazonClientBuilderBy( - getClientConfiguration(maxConnections, connectionTimeout), - endpoint - ) - ); + log.info("Use an IAM role or aws environment credentials"); + awsCredentialsSupplier = DefaultAWSCredentialsProviderChain::new; } + // init client + s3Service = + FunctionalUtils.getDefaultOrBuild( + this.s3Service, + amazonClientBuilderBy( + this::getRegions, + awsCredentialsSupplier, + getClientConfiguration(maxConnections, connectionTimeout), + endpoint + ) + ); // bucket name if (StringUtils.isEmpty(bucketName)) { @@ -341,14 +367,20 @@ public void init() throws IOException { log.error("Can't initialize this store!", e); } - log.info("AWS S3 Assetstore ready to go! bucket:" + bucketName); - tm = FunctionalUtils.getDefaultOrBuild(tm, () -> TransferManagerBuilder.standard() .withAlwaysCalculateMultipartMd5(true) .withS3Client(s3Service) .build()); } + protected Regions getRegions() { + // region + return Optional.ofNullable(awsRegionName) + .filter(StringUtils::isNotBlank) + .map(S3BitStoreService::parseRegion) + .orElseGet(S3BitStoreService::getDefaultRegion); + } + /** * Return an identifier unique to this asset store instance * diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2024.04.23__alter_table_process_add_foreign_key_on_user_id.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2024.04.23__alter_table_process_add_foreign_key_on_user_id.sql new file mode 100644 index 00000000000..85ae350f459 --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2024.04.23__alter_table_process_add_foreign_key_on_user_id.sql @@ -0,0 +1,17 @@ +-- +-- The contents of this file are subject to the license and copyright +-- detailed in the LICENSE and NOTICE files at the root of the source +-- tree and available online at +-- +-- http://www.dspace.org/license/ +-- + +----------------------------------------------------------------------------------- +-- Alter TABLE process +----------------------------------------------------------------------------------- + +-- Drop the NOT NULL constraint on user_id column +ALTER TABLE process ALTER COLUMN user_id DROP NOT NULL; + +-- Add the foreign key constraint with ON DELETE SET NULL +ALTER TABLE process ADD CONSTRAINT user_id FOREIGN KEY (user_id) REFERENCES eperson (uuid) ON DELETE SET NULL; \ No newline at end of file diff --git a/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2024.04.23__alter_table_process_add_foreign_key_on_user_id.sql b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2024.04.23__alter_table_process_add_foreign_key_on_user_id.sql new file mode 100644 index 00000000000..85ae350f459 --- /dev/null +++ b/dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2024.04.23__alter_table_process_add_foreign_key_on_user_id.sql @@ -0,0 +1,17 @@ +-- +-- The contents of this file are subject to the license and copyright +-- detailed in the LICENSE and NOTICE files at the root of the source +-- tree and available online at +-- +-- http://www.dspace.org/license/ +-- + +----------------------------------------------------------------------------------- +-- Alter TABLE process +----------------------------------------------------------------------------------- + +-- Drop the NOT NULL constraint on user_id column +ALTER TABLE process ALTER COLUMN user_id DROP NOT NULL; + +-- Add the foreign key constraint with ON DELETE SET NULL +ALTER TABLE process ADD CONSTRAINT user_id FOREIGN KEY (user_id) REFERENCES eperson (uuid) ON DELETE SET NULL; \ No newline at end of file diff --git a/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-bitstream-with-http-url-to-item.xls b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-bitstream-with-http-url-to-item.xls new file mode 100644 index 00000000000..b0b97f20c78 Binary files /dev/null and b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-bitstream-with-http-url-to-item.xls differ diff --git a/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-to-items.xls b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-to-items.xls index e891108a169..c1ee1f4bf47 100644 Binary files a/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-to-items.xls and b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-to-items.xls differ diff --git a/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-with-local-path.xls b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-with-local-path.xls new file mode 100644 index 00000000000..08c3e05db7a Binary files /dev/null and b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-with-local-path.xls differ diff --git a/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-with-wrong-local-path.xls b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-with-wrong-local-path.xls new file mode 100644 index 00000000000..c02b8c9b86d Binary files /dev/null and b/dspace-api/src/test/data/dspaceFolder/assetstore/bulk-import/add-multiple-bitstreams-with-wrong-local-path.xls differ diff --git a/dspace-api/src/test/java/org/dspace/app/bulkedit/BulkImportIT.java b/dspace-api/src/test/java/org/dspace/app/bulkedit/BulkImportIT.java index e1d04f314c6..601d77d8b91 100644 --- a/dspace-api/src/test/java/org/dspace/app/bulkedit/BulkImportIT.java +++ b/dspace-api/src/test/java/org/dspace/app/bulkedit/BulkImportIT.java @@ -39,10 +39,15 @@ import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.CombinableMatcher.both; import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.net.URL; import java.nio.charset.StandardCharsets; import java.sql.SQLException; import java.util.Iterator; @@ -54,6 +59,7 @@ import org.apache.commons.io.IOUtils; import org.dspace.AbstractIntegrationTestWithDatabase; +import org.dspace.app.bulkimport.util.ImportFileUtil; import org.dspace.app.launcher.ScriptLauncher; import org.dspace.app.matcher.DSpaceObjectMatcher; import org.dspace.app.scripts.handler.impl.TestDSpaceRunnableHandler; @@ -85,6 +91,13 @@ import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.service.GroupService; +import org.dspace.scripts.DSpaceRunnable; +import org.dspace.scripts.configuration.ScriptConfiguration; +import org.dspace.scripts.factory.ScriptServiceFactory; +import org.dspace.scripts.handler.DSpaceRunnableHandler; +import org.dspace.scripts.service.ScriptService; +import org.dspace.services.ConfigurationService; +import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.workflow.WorkflowItem; import org.junit.Before; import org.junit.Test; @@ -98,6 +111,18 @@ @SuppressWarnings("unchecked") public class BulkImportIT extends AbstractIntegrationTestWithDatabase { + protected static final class ImportFileUtilMockClass extends ImportFileUtil { + + public ImportFileUtilMockClass(DSpaceRunnableHandler handler) { + super(handler); + } + + @Override + public InputStream openStream(URL url) { + return super.openStream(url); + } + } + private static final String BASE_XLS_DIR_PATH = "./target/testing/dspace/assetstore/bulk-import/"; private static final String PLACEHOLDER = CrisConstants.PLACEHOLDER_PARENT_METADATA_VALUE; @@ -117,6 +142,10 @@ public class BulkImportIT extends AbstractIntegrationTestWithDatabase { private SearchService searchService = SearchUtils.getSearchService(); + private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + + private final ScriptService scriptService = ScriptServiceFactory.getInstance().getScriptService(); + private Community community; private Collection collection; @@ -1464,9 +1493,9 @@ public void testUploadMultipleBitstreams() throws Exception { assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE2"), contains( bitstreamWith("Test title 2", "test file description 2", - "this is a second test file for uploading bitstreams"), - bitstreamWith("Test title 3", "test file description 3", - "this is a second test file for uploading bitstreams"))); + "this is a test file for uploading bitstreams"), + bitstreamWith("Test title 3", "test file description 3", + "this is a second test file for uploading bitstreams"))); assertThat(getItemBitstreams(item), hasSize(4)); @@ -1523,6 +1552,203 @@ public void testUploadMultipleBitstreamWithPathTraversal() throws Exception { } + @Test + public void testUploadBitstreamWithRemoteFilePathNotFromAllowedIps() throws Exception { + + try { + context.turnOffAuthorisationSystem(); + Collection publication = createCollection(context, community) + .withSubmissionDefinition("publication") + .withAdminGroup(eperson) + .build(); + context.commit(); + context.restoreAuthSystemState(); + + configurationService.setProperty("allowed.ips.import", new String[]{"127.0.1.2"}); + + String fileLocation = getXlsFilePath("add-bitstream-with-http-url-to-item.xls"); + + String[] args = new String[] { "bulk-import", "-c", publication.getID().toString(), "-f", fileLocation, + "-e", eperson.getEmail()}; + TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler(); + + handleScript(args, ScriptLauncher.getConfig(kernelImpl), handler, kernelImpl, eperson); + + assertThat(handler.getErrorMessages(), contains( + "Cannot create bitstream from file at path http://127.0.1.1")); + assertThat(handler.getWarningMessages(), contains( + containsString("Row 2 - Invalid item left in workspace"), + containsString("Row 3 - Invalid item left in workspace"))); + assertThat(handler.getInfoMessages(), contains( + is("Start reading all the metadata group rows"), + is("Found 4 metadata groups to process"), + is("Start reading all the bitstream rows"), + is("Found 1 bitstreams to process"), + is("Found 2 items to process"))); + + Item item = getItemFromMessage(handler.getWarningMessages().get(0)); + Item item2 = getItemFromMessage(handler.getWarningMessages().get(1)); + + assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE"), empty()); + assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE2"), empty()); + assertThat(getItemBitstreamsByBundle(item2, "SECOND-BUNDLE"), empty()); + + } finally { + configurationService.setProperty("allowed.ips.import", new String[]{}); + } + } + + @Test + public void testUploadBitstreamWithRemoteFilePathFromAllowedIps() throws Exception { + try { + InputStream mockInputStream = new ByteArrayInputStream("mocked content".getBytes()); + + context.turnOffAuthorisationSystem(); + Collection publication = createCollection(context, community) + .withSubmissionDefinition("publication") + .withAdminGroup(eperson) + .build(); + context.commit(); + context.restoreAuthSystemState(); + + configurationService.setProperty("allowed.ips.import", new String[]{"127.0.1.1"}); + + String fileLocation = getXlsFilePath("add-bitstream-with-http-url-to-item.xls"); + + String[] args = new String[] { "bulk-import", "-c", publication.getID().toString(), "-f", fileLocation, + "-e", eperson.getEmail()}; + TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler(); + + ImportFileUtilMockClass importFileUtilSpy = spy(new ImportFileUtilMockClass(handler)); + doReturn(mockInputStream).when(importFileUtilSpy).openStream(any(URL.class)); + + ScriptService scriptService = ScriptServiceFactory.getInstance().getScriptService(); + ScriptConfiguration scriptConfiguration = scriptService.getScriptConfiguration(args[0]); + + BulkImport script = null; + if (scriptConfiguration != null) { + script = (BulkImport) scriptService.createDSpaceRunnableForScriptConfiguration(scriptConfiguration); + script.setImportFileUtil(importFileUtilSpy); + } + if (script != null) { + if (DSpaceRunnable.StepResult.Continue.equals(script.initialize(args, handler, eperson))) { + script.run(); + } + } + + assertThat(handler.getErrorMessages(), empty()); + assertThat(handler.getWarningMessages(), contains( + containsString("Row 2 - Invalid item left in workspace"), + containsString("Row 3 - Invalid item left in workspace"))); + assertThat(handler.getInfoMessages(), contains( + is("Start reading all the metadata group rows"), + is("Found 4 metadata groups to process"), + is("Start reading all the bitstream rows"), + is("Found 1 bitstreams to process"), + is("Found 2 items to process"), + containsString("Sheet bitstream-metadata - Row 2 - Bitstream created successfully"))); + + Item item = getItemFromMessage(handler.getWarningMessages().get(0)); + + assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE"), contains( + bitstreamWith("Test title", "test file description", + "mocked content"))); + + } finally { + configurationService.setProperty("allowed.ips.import", new String[]{}); + } + } + + @Test + public void testUploadMultipleBitstreamWithCorrectLocalPath() throws Exception { + + context.turnOffAuthorisationSystem(); + Collection publication = createCollection(context, community) + .withSubmissionDefinition("publication") + .withAdminGroup(eperson) + .build(); + context.commit(); + context.restoreAuthSystemState(); + + String fileLocation = getXlsFilePath("add-multiple-bitstreams-with-local-path.xls"); + + String[] args = new String[] { "bulk-import", "-c", publication.getID().toString(), "-f", fileLocation, + "-e", eperson.getEmail()}; + TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler(); + + handleScript(args, ScriptLauncher.getConfig(kernelImpl), handler, kernelImpl, eperson); + + assertThat(handler.getErrorMessages(), empty()); + assertThat(handler.getWarningMessages(), contains( + containsString("Row 2 - Invalid item left in workspace"), + containsString("Row 3 - Invalid item left in workspace"))); + assertThat(handler.getInfoMessages(), contains( + is("Start reading all the metadata group rows"), + is("Found 4 metadata groups to process"), + is("Start reading all the bitstream rows"), + is("Found 2 bitstreams to process"), + is("Found 2 items to process"), + containsString("Sheet bitstream-metadata - Row 2 - Bitstream created successfully"), + containsString("Sheet bitstream-metadata - Row 3 - Bitstream created successfully"))); + + Item item = getItemFromMessage(handler.getWarningMessages().get(0)); + Item item2 = getItemFromMessage(handler.getWarningMessages().get(1)); + + assertThat(getItemBitstreamsByBundle(item, "FIRST-BUNDLE"), contains( + bitstreamWith("Test title 2", "test file description 2", + "this is a second test file for uploading bitstreams"))); + assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE2"), empty()); + assertThat(getItemBitstreamsByBundle(item2, "SECOND-BUNDLE"), contains( + bitstreamWith("Test title 3", "test file description 3", + "this is a third test file for uploading bitstreams"))); + + } + + @Test + public void testUploadMultipleBitstreamWithWrongLocalPath() throws Exception { + + context.turnOffAuthorisationSystem(); + Collection publication = createCollection(context, community) + .withSubmissionDefinition("publication") + .withAdminGroup(eperson) + .build(); + context.commit(); + context.restoreAuthSystemState(); + + String fileLocation = getXlsFilePath("add-multiple-bitstreams-with-wrong-local-path.xls"); + + String[] args = new String[] { "bulk-import", "-c", publication.getID().toString(), "-f", fileLocation, + "-e", eperson.getEmail()}; + TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler(); + + handleScript(args, ScriptLauncher.getConfig(kernelImpl), handler, kernelImpl, eperson); + + assertThat(handler.getErrorMessages(), contains( + "Access to the specified file file://../test_2.txt is not allowed", + "Cannot create bitstream from file at path file://../test_2.txt", + "Access to the specified file file:///bulk-uploads/test_2.txt is not allowed", + "Cannot create bitstream from file at path file:///bulk-uploads/test_2.txt", + "Access to the specified file file:///subfolder/test_2.txt is not allowed", + "Cannot create bitstream from file at path file:///subfolder/test_2.txt")); + assertThat(handler.getWarningMessages(), contains( + containsString("Row 2 - Invalid item left in workspace"), + containsString("Row 3 - Invalid item left in workspace"))); + assertThat(handler.getInfoMessages(), contains( + is("Start reading all the metadata group rows"), + is("Found 4 metadata groups to process"), + is("Start reading all the bitstream rows"), + is("Found 3 bitstreams to process"), + is("Found 2 items to process"))); + + Item item = getItemFromMessage(handler.getWarningMessages().get(0)); + Item item2 = getItemFromMessage(handler.getWarningMessages().get(1)); + + assertThat(getItemBitstreamsByBundle(item, "FIRST-BUNDLE"), empty()); + assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE2"), empty()); + assertThat(getItemBitstreamsByBundle(item2, "SECOND-BUNDLE"), empty()); + + } + @Test public void testUploadSingleBitstreamUpdate() throws Exception { context.turnOffAuthorisationSystem(); diff --git a/dspace-api/src/test/java/org/dspace/app/bulkimport/service/BulkImportWorkbookBuilderIT.java b/dspace-api/src/test/java/org/dspace/app/bulkimport/service/BulkImportWorkbookBuilderIT.java index 4d4af87ddaa..43035f8b197 100644 --- a/dspace-api/src/test/java/org/dspace/app/bulkimport/service/BulkImportWorkbookBuilderIT.java +++ b/dspace-api/src/test/java/org/dspace/app/bulkimport/service/BulkImportWorkbookBuilderIT.java @@ -118,7 +118,7 @@ public void setup() throws SQLException, AuthorizeException { @SuppressWarnings("unchecked") public void testWorkbookBuildingFromItemDtos() throws Exception { - configurationService.setProperty("bulk-uploads.local-folder", System.getProperty("java.io.tmpdir")); + configurationService.setProperty("uploads.local-folder", System.getProperty("java.io.tmpdir")); context.turnOffAuthorisationSystem(); diff --git a/dspace-api/src/test/java/org/dspace/proccess/ProcessIT.java b/dspace-api/src/test/java/org/dspace/proccess/ProcessIT.java index b79bb04fa59..45e834546e4 100644 --- a/dspace-api/src/test/java/org/dspace/proccess/ProcessIT.java +++ b/dspace-api/src/test/java/org/dspace/proccess/ProcessIT.java @@ -6,7 +6,9 @@ * http://www.dspace.org/license/ */ package org.dspace.proccess; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.Collections; @@ -15,10 +17,13 @@ import java.util.UUID; import org.dspace.AbstractIntegrationTestWithDatabase; +import org.dspace.builder.EPersonBuilder; import org.dspace.builder.GroupBuilder; import org.dspace.builder.ProcessBuilder; +import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; +import org.dspace.eperson.service.EPersonService; import org.dspace.eperson.service.GroupService; import org.dspace.scripts.Process; import org.dspace.scripts.factory.ScriptServiceFactory; @@ -34,6 +39,7 @@ public class ProcessIT extends AbstractIntegrationTestWithDatabase { protected ProcessService processService = ScriptServiceFactory.getInstance().getProcessService(); protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + protected EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); @Test public void checkProcessGroupsTest() throws Exception { @@ -90,4 +96,42 @@ public void removeOneGroupTest() throws Exception { assertFalse(isPresent); } + + @Test + public void checkProcessCreatorTest() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson userA = EPersonBuilder.createEPerson(context) + .withEmail("userA@email.com") + .withPassword(password) + .withCanLogin(true).build(); + + Process processA = ProcessBuilder.createProcess(context, userA, "mock-script", new LinkedList<>()).build(); + + context.restoreAuthSystemState(); + + Process process = processService.find(context, processA.getID()); + assertEquals(process.getEPerson(), context.reloadEntity(userA)); + } + @Test + public void removeProcessCreatorTest() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson userA = EPersonBuilder.createEPerson(context) + .withEmail("userA@email.com") + .withPassword(password) + .withCanLogin(true).build(); + + Process processA = ProcessBuilder.createProcess(context, userA, "mock-script", new LinkedList<>()).build(); + + ePersonService.delete(context, userA); + context.commit(); + + context.restoreAuthSystemState(); + + Process process = processService.find(context, processA.getID()); + assertNull(process.getEPerson()); + } } diff --git a/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java b/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java index fb581d8361b..d6001da9de6 100644 --- a/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java +++ b/dspace-api/src/test/java/org/dspace/storage/bitstore/S3BitStoreServiceIT.java @@ -7,7 +7,6 @@ */ package org.dspace.storage.bitstore; -import static com.amazonaws.regions.Regions.DEFAULT_REGION; import static java.nio.charset.StandardCharsets.UTF_8; import static org.dspace.storage.bitstore.S3BitStoreService.CSA; import static org.dspace.storage.bitstore.S3BitStoreService.getAwsCredentialsSupplier; @@ -34,6 +33,7 @@ import java.util.Map; import com.amazonaws.auth.AnonymousAWSCredentials; +import com.amazonaws.regions.Regions; import com.amazonaws.services.s3.AmazonS3; import com.amazonaws.services.s3.model.AmazonS3Exception; import com.amazonaws.services.s3.model.Bucket; @@ -406,7 +406,7 @@ private byte[] generateChecksum(String content) { private AmazonS3 createAmazonS3Client(String endpoint) { return S3BitStoreService.amazonClientBuilderBy( - DEFAULT_REGION, + () -> Regions.DEFAULT_REGION, getAwsCredentialsSupplier(new AnonymousAWSCredentials()), getClientConfiguration(MAX_CONNECTIONS, CONNECTION_TIMEOUT), endpoint diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/RestResourceController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/RestResourceController.java index ec6fced9513..44cac53249e 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/RestResourceController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/RestResourceController.java @@ -761,7 +761,6 @@ public ResponseEntity> patchInt modelObject = repository.patch(request, apiCategory, model, id, patch); } catch (RepositoryMethodNotImplementedException | UnprocessableEntityException | DSpaceBadRequestException | ResourceNotFoundException e) { - log.error(e.getMessage(), e); throw e; } if (modelObject != null) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/enhancer/RelatedItemEnhancerUpdatePoller.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/enhancer/RelatedItemEnhancerUpdatePoller.java index b4af863e654..391a5fc7963 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/enhancer/RelatedItemEnhancerUpdatePoller.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/enhancer/RelatedItemEnhancerUpdatePoller.java @@ -36,6 +36,7 @@ public void pollItemToUpdateAndProcess() { try { log.debug("item enhancer poller executed"); Context context = new Context(); + context.setDispatcher(RelatedItemEnhancerUpdatePoller.class.getSimpleName()); context.turnOffAuthorisationSystem(); UUID extractedUuid; while ((extractedUuid = itemEnhancerService.pollItemToUpdate(context)) != null) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java index f42628c96f0..54c7815d83e 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java @@ -202,6 +202,18 @@ protected ResponseEntity handleCustomUnprocessableEditException(HttpServ HttpServletResponse response, UnprocessableEditException ex) throws IOException { + String location; + String exceptionMessage; + if (null == ex) { + exceptionMessage = "none"; + location = "unknown"; + } else { + exceptionMessage = ex.getMessage(); + StackTraceElement[] trace = ex.getStackTrace(); + location = trace.length <= 0 ? "unknown" : trace[0].toString(); + } + log.warn("{} (status:{} exception: {} at: {})", "unprocessable edit item", HttpStatus.UNPROCESSABLE_ENTITY, + exceptionMessage, location); return new ResponseEntity<>(ex.getErrors(), null, HttpStatus.UNPROCESSABLE_ENTITY); } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EditItemRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EditItemRestRepository.java index 87bc0b46b01..2e4177651e2 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EditItemRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EditItemRestRepository.java @@ -279,7 +279,8 @@ public void patch(Context context, HttpServletRequest request, String apiCategor "The current user does not have rights to edit mode <" + modeName + ">"); } context.turnOffAuthorisationSystem(); - + List initialErrors = validationService.validate(context, source); + int numInitialErrors = calculateErrors(initialErrors); EditItemRest eir = findOne(context, data); for (Operation op : operations) { // the value in the position 0 is a null value @@ -294,14 +295,24 @@ public void patch(Context context, HttpServletRequest request, String apiCategor } List errors = validationService.validate(context, source); - if (errors != null && !errors.isEmpty()) { - throw new UnprocessableEditException(errors); + int editErrors = calculateErrors(errors); + if (numInitialErrors < editErrors) { + throw new UnprocessableEditException(errors, "The number of validation errors in the item increase from " + + numInitialErrors + " to " + editErrors); } eis.update(context, source); context.restoreAuthSystemState(); } + private int calculateErrors(List errors) { + if (errors == null || errors.isEmpty()) { + return 0; + } else { + return errors.stream().mapToInt(e -> e.getPaths().size()).sum(); + } + } + private void evaluatePatch(Context context, HttpServletRequest request, EditItem source, EditItemRest eir, String section, Operation op) { boolean sectionExist = false; diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EditItemRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EditItemRestRepositoryIT.java index 3e8a8e18b44..16b2b36a036 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EditItemRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EditItemRestRepositoryIT.java @@ -58,6 +58,7 @@ import org.dspace.eperson.service.GroupService; import org.dspace.services.ConfigurationService; import org.hamcrest.Matchers; +import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.mock.web.MockMultipartFile; @@ -1424,6 +1425,8 @@ public void testPatchWithValidationErrors() throws Exception { .build(); Item itemA = ItemBuilder.createItem(context, collection) + .withTitle("Intial title") + .withIssueDate("2023-02-04") .withFulltext("bitstream.txt", "source", InputStream.nullInputStream()) .build(); @@ -1434,8 +1437,10 @@ public void testPatchWithValidationErrors() throws Exception { String tokenAdmin = getAuthToken(admin.getEmail(), password); List operations = new ArrayList(); + operations.add(new RemoveOperation("/sections/titleAndIssuedDate/dc.date.issued")); operations.add(new AddOperation("/sections/titleAndIssuedDate/dc.title", of(Map.of("value", "My Title")))); + // we should be unable to change the title removing the date getClient(tokenAdmin).perform(patch("/api/core/edititems/" + editItem.getID() + ":FIRST") .content(getPatchContent(operations)) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) @@ -1445,13 +1450,102 @@ public void testPatchWithValidationErrors() throws Exception { hasJsonPath("paths", contains("/sections/titleAndIssuedDate/dc.date.issued")))))); operations.add(new AddOperation("/sections/titleAndIssuedDate/dc.date.issued", of(Map.of("value", "2022")))); - + // this should succeed now as we are also providing a new date getClient(tokenAdmin).perform(patch("/api/core/edititems/" + editItem.getID() + ":FIRST") .content(getPatchContent(operations)) .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) .andExpect(status().isOk()); } + @Test + public void testPatchItemWithExistingValidationErrors() throws Exception { + context.turnOffAuthorisationSystem(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + + Collection collection = CollectionBuilder.createCollection(context, parentCommunity) + .withEntityType("Publication") + .withSubmissionDefinition("traditional-cris") + .withName("Collection 1") + .build(); + + // the item below miss the mandatory dc.title and dc.date.issued + Item itemA = ItemBuilder.createItem(context, collection) + .withAuthor("Wayne, Bruce") + .grantLicense() + .build(); + // this one miss the mandatory dc.date.issued + Item itemB = ItemBuilder.createItem(context, collection) + .withTitle("At least the title...") + .withAuthor("Wayne, Bruce") + .grantLicense() + .build(); + + EditItem editItemA = new EditItem(context, itemA); + EditItem editItemB = new EditItem(context, itemB); + + context.restoreAuthSystemState(); + + String tokenAdmin = getAuthToken(admin.getEmail(), password); + + List> titleValues = new ArrayList<>(); + Map titleMap = new HashMap<>(); + List> publisherValues = new ArrayList<>(); + Map publisherMap = new HashMap<>(); + titleMap.put("value", "A title"); + titleValues.add(titleMap); + publisherMap.put("value", "A publisher"); + publisherValues.add(publisherMap); + List listOpA = new ArrayList<>(); + listOpA.add(new AddOperation("/sections/traditionalpageone-cris/dc.title", titleValues)); + listOpA.add(new AddOperation("/sections/traditionalpageone-cris/dc.publisher", publisherValues)); + + List listOpB = new ArrayList<>(); + listOpB.add(new RemoveOperation("/sections/traditionalpageone-cris/dc.title")); + listOpB.add(new AddOperation("/sections/traditionalpageone-cris/dc.publisher", publisherValues)); + + String patchBodyA = getPatchContent(listOpA); + getClient(tokenAdmin).perform(patch("/api/core/edititems/" + editItemA.getID() + ":MODE1") + .content(patchBodyA) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.sections.traditionalpageone-cris['dc.title'][0].value", + is("A title"))) + .andExpect(jsonPath("$.sections.traditionalpageone-cris['dc.publisher'][0].value", + is("A publisher"))) + .andExpect(jsonPath("$.errors[0].message", is("error.validation.required"))) + .andExpect(jsonPath("$.errors[0].paths[0]", is("/sections/traditionalpageone-cris/dc.date.issued"))); + + getClient(tokenAdmin).perform(get("/api/core/edititems/" + editItemA.getID() + ":MODE1")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.sections.traditionalpageone-cris['dc.title'][0].value", is("A title"))) + .andExpect(jsonPath("$.sections.traditionalpageone-cris['dc.publisher'][0].value", is("A publisher"))) + .andExpect(jsonPath("$.errors[0].message", is("error.validation.required"))) + .andExpect(jsonPath("$.errors[0].paths[0]", is("/sections/traditionalpageone-cris/dc.date.issued"))); + + // the list of operation B makes the item worst, so we expect a 422 response and the json should + // describe the errors according to the new state of the item attempted to be generated by the requested + String patchBodyB = getPatchContent(listOpB); + getClient(tokenAdmin).perform(patch("/api/core/edititems/" + editItemB.getID() + ":MODE1") + .content(patchBodyB) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isUnprocessableEntity()) + .andExpect(jsonPath("$.[0].message", is("error.validation.required"))) + .andExpect(jsonPath("$.[0].paths[0]", is("/sections/traditionalpageone-cris/dc.title"))) + .andExpect(jsonPath("$.[0].paths[1]", is("/sections/traditionalpageone-cris/dc.date.issued"))); + // as the request has been rejected, the state should not be persisted + getClient(tokenAdmin).perform(get("/api/core/edititems/" + editItemB.getID() + ":MODE1")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.sections.traditionalpageone-cris['dc.title'][0].value", + is("At least the title..."))) + .andExpect(jsonPath("$.sections.traditionalpageone-cris['dc.publisher']").doesNotExist()) + .andExpect(jsonPath("$.errors[0].message", is("error.validation.required"))) + .andExpect(jsonPath("$.errors[0].paths[0]", is("/sections/traditionalpageone-cris/dc.date.issued"))); + + } + @Test public void testUpload() throws Exception { context.turnOffAuthorisationSystem(); @@ -2024,6 +2118,7 @@ public void testPatchWithValidationErrors3213() throws Exception { } @Test + @Ignore("see DSC-1787") public void testPatchWithValidationErrors32() throws Exception { context.turnOffAuthorisationSystem(); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/OrcidAuthorityIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/OrcidAuthorityIT.java index fed87df674f..f05587d1c90 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/OrcidAuthorityIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/OrcidAuthorityIT.java @@ -804,6 +804,32 @@ public void testMultipleSourcesReferences() throws Exception { .andExpect(jsonPath("$.page.totalElements", Matchers.is(4))); } + @Test + public void testWithORCIDIdentifier() throws Exception { + + List orcidSearchResults = List.of( + expandedResult("Author", "From Orcid 1", "0000-1111-2222-3333")); + + String expectedQuery = "(orcid:0000-1111-2222-3333)"; + + when(orcidClientMock.expandedSearch(READ_PUBLIC_TOKEN, expectedQuery, 0, 20)) + .thenReturn(expandedSearch(2, orcidSearchResults)); + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform(get("/api/submission/vocabularies/AuthorAuthority/entries") + .param("filter", "0000-1111-2222-3333")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.entries", containsInAnyOrder( + orcidEntry("From Orcid 1 Author", REFERENCE, "0000-1111-2222-3333", getSource())))) + .andExpect(jsonPath("$.page.size", Matchers.is(20))) + .andExpect(jsonPath("$.page.totalPages", Matchers.is(1))) + .andExpect(jsonPath("$.page.totalElements", Matchers.is(1))); + + verify(orcidClientMock).getReadPublicAccessToken(); + verify(orcidClientMock).expandedSearch(READ_PUBLIC_TOKEN, expectedQuery, 0, 20); + verifyNoMoreInteractions(orcidClientMock); + } + private ExpandedSearch buildExpandedSearchFromSublist(List totalResults, int start, int rows) { int total = totalResults.size(); if (start > total) { diff --git a/dspace/config/dspace.cfg b/dspace/config/dspace.cfg index d333de23460..fab3a307267 100644 --- a/dspace/config/dspace.cfg +++ b/dspace/config/dspace.cfg @@ -799,12 +799,16 @@ aip.disseminate.dmd = MODS, DIM # default synchronous dispatcher (same behavior as traditional DSpace) event.dispatcher.default.class = org.dspace.event.BasicDispatcher +# default synchronous dispatcher with custom consumers (same behavior as traditional DSpace) +event.dispatcher.RelatedItemEnhancerUpdatePoller.class = org.dspace.event.BasicDispatcher + # Add doi here if you are using org.dspace.identifier.DOIIdentifierProvider to generate DOIs. # Adding doi here makes DSpace send metadata updates to your doi registration agency. # Add rdf here, if you are using dspace-rdf to export your repository content as RDF. # Add iiif here, if you are using dspace-iiif. # Add orcidqueue here, if the integration with ORCID is configured and wish to enable the synchronization queue functionality event.dispatcher.default.consumers = versioning, discovery, eperson, dedup, crisconsumer, orcidqueue, audit, nbeventsdelete, referenceresolver, orcidwebhook, itemenhancer, customurl, iiif, reciprocal, filetypemetadataenhancer, authoritylink +event.dispatcher.RelatedItemEnhancerUpdatePoller.consumers = versioning, discovery, eperson, dedup, crisconsumer, orcidqueue, audit, nbeventsdelete, referenceresolver, orcidwebhook, itemenhancer, customurl, iiif, reciprocal, filetypemetadataenhancer, authoritylink # enable the item enhancer poller related-item-enhancer-poller.enabled = true @@ -1597,8 +1601,11 @@ google-metadata.enable = true # Where to temporarily store uploaded files upload.temp.dir = ${dspace.dir}/upload -# Where to lookup for local files to upload via the bulk import -bulk-uploads.local-folder = ${dspace.dir}/bulk-uploads +# Where to lookup for local files to upload via the import +uploads.local-folder = ${dspace.dir}/bulk-uploads + +# Allowed domain list from which we can retrieve remote files +#allowed.ips.import = 127.10.10.10, 127.10.10.11 ###### Statistical Report Configuration Settings ###### diff --git a/dspace/config/spring/api/cris-layout-tool-render-validation.xml b/dspace/config/spring/api/cris-layout-tool-render-validation.xml index 62befb64940..9f2082832a2 100644 --- a/dspace/config/spring/api/cris-layout-tool-render-validation.xml +++ b/dspace/config/spring/api/cris-layout-tool-render-validation.xml @@ -58,6 +58,7 @@ scopus researcherid mailto + ror diff --git a/dspace/config/submission-forms.xml b/dspace/config/submission-forms.xml index 55068bc250a..f5c1d18373d 100644 --- a/dspace/config/submission-forms.xml +++ b/dspace/config/submission-forms.xml @@ -2511,8 +2511,9 @@ it, please enter the types and the actual numbers or codes.
- cris - owner + dspace + object + owner onebox false diff --git a/dspace/etc/conftool/cris-layout-configuration.xls b/dspace/etc/conftool/cris-layout-configuration.xls index e9996766846..87e84f9c192 100644 Binary files a/dspace/etc/conftool/cris-layout-configuration.xls and b/dspace/etc/conftool/cris-layout-configuration.xls differ