From e02b13ab19f9823a638f6321d9df0d530f486528 Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Mon, 25 Oct 2021 10:53:53 -0700 Subject: [PATCH 01/19] Updated processOptions for iiif. Updated processContentsFile. Style check fix. --- .../app/itemimport/ItemImportServiceImpl.java | 224 ++++++++++++++++-- 1 file changed, 208 insertions(+), 16 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java index d860b0e25a..106e0e532a 100644 --- a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java @@ -1129,7 +1129,7 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea if (iAssetstore == -1 || sFilePath == null) { System.out.println("\tERROR: invalid contents file line"); System.out.println("\t\tSkipping line: " - + sRegistrationLine); + + sRegistrationLine); continue; } @@ -1153,9 +1153,9 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea registerBitstream(c, i, iAssetstore, sFilePath, sBundle, sDescription); System.out.println("\tRegistering Bitstream: " + sFilePath - + "\tAssetstore: " + iAssetstore - + "\tBundle: " + sBundle - + "\tDescription: " + sDescription); + + "\tAssetstore: " + iAssetstore + + "\tBundle: " + sBundle + + "\tDescription: " + sDescription); continue; // process next line in contents file } @@ -1172,6 +1172,59 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea boolean bundleExists = false; boolean permissionsExist = false; boolean descriptionExists = false; + boolean labelExists = false; + boolean heightExists = false; + boolean widthExists = false; + boolean tocExists = false; + + // look for label + String labelMarker = "\tiiif-label"; + int lMarkerIndex = line.indexOf(labelMarker); + int lEndIndex = 0; + if (lMarkerIndex > 0) { + lEndIndex = line.indexOf("\t", lMarkerIndex + 1); + if (lEndIndex == -1) { + lEndIndex = line.length(); + } + labelExists = true; + } + + // look for height + String heightMarker = "\tiiif-height"; + int hMarkerIndex = line.indexOf(heightMarker); + int hEndIndex = 0; + if (hMarkerIndex > 0) { + hEndIndex = line.indexOf("\t", hMarkerIndex + 1); + if (hEndIndex == -1) { + hEndIndex = line.length(); + } + heightExists = true; + } + + // look for width + String widthMarker = "\tiiif-width"; + int wMarkerIndex = line.indexOf(widthMarker); + int wEndIndex = 0; + if (wMarkerIndex > 0) { + wEndIndex = line.indexOf("\t", wMarkerIndex + 1); + if (wEndIndex == -1) { + wEndIndex = line.length(); + } + widthExists = true; + } + + // look for toc + String tocMarker = "\tiiif-toc"; + int tMarkerIndex = line.indexOf(tocMarker); + int tEndIndex = 0; + if (tMarkerIndex > 0) { + tEndIndex = line.indexOf("\t", tMarkerIndex + 1); + if (tEndIndex == -1) { + tEndIndex = line.length(); + } + tocExists = true; + } + // look for a bundle name String bundleMarker = "\tbundle:"; @@ -1220,18 +1273,20 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea if (bundleExists) { String bundleName = line.substring(bMarkerIndex - + bundleMarker.length(), bEndIndex).trim(); + + bundleMarker.length(), bEndIndex).trim(); processContentFileEntry(c, i, path, bitstreamName, bundleName, primary); System.out.println("\tBitstream: " + bitstreamName + - "\tBundle: " + bundleName + - primaryStr); + "\tBundle: " + bundleName + + primaryStr); } else { processContentFileEntry(c, i, path, bitstreamName, null, primary); System.out.println("\tBitstream: " + bitstreamName + primaryStr); } - if (permissionsExist || descriptionExists) { + if (permissionsExist || descriptionExists || labelExists || heightExists + || widthExists || tocExists) { + System.out.println("Gathering options."); String extraInfo = bitstreamName; if (permissionsExist) { @@ -1244,6 +1299,26 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea + line.substring(dMarkerIndex, dEndIndex); } + if (labelExists) { + extraInfo = extraInfo + + line.substring(lMarkerIndex, lEndIndex); + } + + if (heightExists) { + extraInfo = extraInfo + + line.substring(hMarkerIndex, hEndIndex); + } + + if (widthExists) { + extraInfo = extraInfo + + line.substring(wMarkerIndex, wEndIndex); + } + + if (tocExists) { + extraInfo = extraInfo + + line.substring(tMarkerIndex, tEndIndex); + } + options.add(extraInfo); } } @@ -1425,11 +1500,16 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea */ protected void processOptions(Context c, Item myItem, List options) throws SQLException, AuthorizeException { + System.out.println("Processing options."); for (String line : options) { System.out.println("\tprocessing " + line); boolean permissionsExist = false; boolean descriptionExists = false; + boolean labelExists = false; + boolean heightExists = false; + boolean widthExists = false; + boolean tocExists = false; String permissionsMarker = "\tpermissions:"; int pMarkerIndex = line.indexOf(permissionsMarker); @@ -1453,6 +1533,56 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea descriptionExists = true; } + + // look for label + String labelMarker = "\tiiif-label:"; + int lMarkerIndex = line.indexOf(labelMarker); + int lEndIndex = 0; + if (lMarkerIndex > 0) { + lEndIndex = line.indexOf("\t", lMarkerIndex + 1); + if (lEndIndex == -1) { + lEndIndex = line.length(); + } + labelExists = true; + } + + // look for height + String heightMarker = "\tiiif-height:"; + int hMarkerIndex = line.indexOf(heightMarker); + int hEndIndex = 0; + if (hMarkerIndex > 0) { + hEndIndex = line.indexOf("\t", hMarkerIndex + 1); + if (hEndIndex == -1) { + hEndIndex = line.length(); + } + heightExists = true; + } + + // look for width + String widthMarker = "\tiiif-width:"; + int wMarkerIndex = line.indexOf(widthMarker); + int wEndIndex = 0; + if (wMarkerIndex > 0) { + wEndIndex = line.indexOf("\t", wMarkerIndex + 1); + if (wEndIndex == -1) { + wEndIndex = line.length(); + } + widthExists = true; + } + + // look for toc + String tocMarker = "\tiiif-toc:"; + int tMarkerIndex = line.indexOf(tocMarker); + int tEndIndex = 0; + if (tMarkerIndex > 0) { + tEndIndex = line.indexOf("\t", tMarkerIndex + 1); + if (tEndIndex == -1) { + tEndIndex = line.length(); + } + tocExists = true; + } + + int bsEndIndex = line.indexOf("\t"); String bitstreamName = line.substring(0, bsEndIndex); @@ -1461,7 +1591,7 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea Group myGroup = null; if (permissionsExist) { String thisPermission = line.substring(pMarkerIndex - + permissionsMarker.length(), pEndIndex); + + permissionsMarker.length(), pEndIndex); // get permission type ("read" or "write") int pTypeIndex = thisPermission.indexOf('-'); @@ -1478,7 +1608,7 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea } groupName = thisPermission.substring(groupIndex + 1, - groupEndIndex); + groupEndIndex); if (thisPermission.toLowerCase().charAt(pTypeIndex + 1) == 'r') { actionID = Constants.READ; @@ -1490,7 +1620,7 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea myGroup = groupService.findByName(c, groupName); } catch (SQLException sqle) { System.out.println("SQL Exception finding group name: " - + groupName); + + groupName); // do nothing, will check for null group later } } @@ -1498,10 +1628,38 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea String thisDescription = ""; if (descriptionExists) { thisDescription = line.substring( - dMarkerIndex + descriptionMarker.length(), dEndIndex) + dMarkerIndex + descriptionMarker.length(), dEndIndex) .trim(); } + String thisLabel = ""; + if (labelExists) { + thisLabel = line.substring( + lMarkerIndex + labelMarker.length(), lEndIndex) + .trim(); + } + + String thisHeight = ""; + if (heightExists) { + thisHeight = line.substring( + hMarkerIndex + heightMarker.length(), hEndIndex) + .trim(); + } + + String thisWidth = ""; + if (widthExists) { + thisWidth = line.substring( + wMarkerIndex + widthMarker.length(), wEndIndex) + .trim(); + } + + String thisToc = ""; + if (tocExists) { + thisToc = line.substring( + tMarkerIndex + tocMarker.length(), tEndIndex) + .trim(); + } + Bitstream bs = null; boolean notfound = true; if (!isTest) { @@ -1518,28 +1676,62 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea if (notfound && !isTest) { // this should never happen System.out.println("\tdefault permissions set for " - + bitstreamName); + + bitstreamName); } else if (!isTest) { if (permissionsExist) { if (myGroup == null) { System.out.println("\t" + groupName - + " not found, permissions set to default"); + + " not found, permissions set to default"); } else if (actionID == -1) { System.out .println("\tinvalid permissions flag, permissions set to default"); } else { System.out.println("\tSetting special permissions for " - + bitstreamName); + + bitstreamName); setPermission(c, myGroup, actionID, bs); } } if (descriptionExists) { System.out.println("\tSetting description for " - + bitstreamName); + + bitstreamName); bs.setDescription(c, thisDescription); bitstreamService.update(c, bs); } + + if (labelExists) { + MetadataField metadataField = metadataFieldService + .findByElement(c, "iiif", "label", null); + System.out.println("\tSetting label to " + thisLabel + " in element " + + metadataField.getElement() + " on " + bitstreamName); + bitstreamService.addMetadata(c, bs, metadataField, null, thisLabel); + bitstreamService.update(c, bs); + } + + if (heightExists) { + MetadataField metadataField = metadataFieldService + .findByElement(c, "iiif", "image", "height"); + System.out.println("\tSetting height to " + thisHeight + " in element " + + metadataField.getElement() + " on " + bitstreamName); + bitstreamService.addMetadata(c, bs, metadataField, null, thisHeight); + bitstreamService.update(c, bs); + } + if (widthExists) { + MetadataField metadataField = metadataFieldService + .findByElement(c, "iiif", "image", "width"); + System.out.println("\tSetting width to " + thisWidth + " in element " + + metadataField.getElement() + " on " + bitstreamName); + bitstreamService.addMetadata(c, bs, metadataField, null, thisWidth); + bitstreamService.update(c, bs); + } + if (tocExists) { + MetadataField metadataField = metadataFieldService + .findByElement(c, "iiif", "toc", null); + System.out.println("\tSetting toc to " + thisToc + " in element " + + metadataField.getElement() + " on " + bitstreamName); + bitstreamService.addMetadata(c, bs, metadataField, null, thisToc); + bitstreamService.update(c, bs); + } } } } From a39b5097731ff550f7a023148c69d4428c54a911 Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Wed, 27 Oct 2021 12:41:20 -0700 Subject: [PATCH 02/19] Minor changes to comments. --- .../main/java/org/dspace/app/iiif/service/ManifestService.java | 2 -- dspace/config/modules/iiif.cfg | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dspace-iiif/src/main/java/org/dspace/app/iiif/service/ManifestService.java b/dspace-iiif/src/main/java/org/dspace/app/iiif/service/ManifestService.java index 794fbb46ca..006054c8cf 100644 --- a/dspace-iiif/src/main/java/org/dspace/app/iiif/service/ManifestService.java +++ b/dspace-iiif/src/main/java/org/dspace/app/iiif/service/ManifestService.java @@ -175,8 +175,6 @@ public class ManifestService extends AbstractResourceService { // add the range reference to the currRange so to get an identifier currRange.addSubRange(range); - // add the range to the manifest -// manifestGenerator.addRange(range); // move the current range currRange = range; diff --git a/dspace/config/modules/iiif.cfg b/dspace/config/modules/iiif.cfg index d6820b5305..437b73634b 100644 --- a/dspace/config/modules/iiif.cfg +++ b/dspace/config/modules/iiif.cfg @@ -69,7 +69,8 @@ iiif.license.uri = dc.rights.uri # static text to be used as attribution in the iiif manifests iiif.attribution = ${dspace.name} -# URL for logo. If defined, the logo will be added to the manifest. +# URL for logo. A small image that represents an individual or organization associated with the +# resource. It is added to the IIIF manifest. iiif.logo.image = ${dspace.ui.url}/assets/images/dspace-logo.svg # (optional) one of individuals, paged or continuous. Can be overridden at the item level via From 6e5b9634a382fa7c74511a31a49258a7f8eaf330 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 3 Dec 2021 13:31:53 +0100 Subject: [PATCH 03/19] implemented feedback endpoint --- .../DSpaceFeedbackNotFoundException.java | 30 +++++ .../dspace/app/rest/model/FeedbackRest.java | 75 ++++++++++++ .../repository/FeedbackRestRepository.java | 112 ++++++++++++++++++ 3 files changed, 217 insertions(+) create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceFeedbackNotFoundException.java create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceFeedbackNotFoundException.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceFeedbackNotFoundException.java new file mode 100644 index 0000000000..15878e857c --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceFeedbackNotFoundException.java @@ -0,0 +1,30 @@ +/** + * 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.rest.exception; +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +/** + * When a request is malformed, we use this exception to indicate this to the client + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk at 4science.com) + */ +@ResponseStatus(value = HttpStatus.NOT_FOUND, reason = "Not Found") +public class DSpaceFeedbackNotFoundException extends RuntimeException { + + private static final long serialVersionUID = 4631940402294095433L; + + public DSpaceFeedbackNotFoundException(String message) { + this(message, null); + } + + public DSpaceFeedbackNotFoundException(String message, Exception e) { + super(message, e); + } + +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java new file mode 100644 index 0000000000..5ab04aa062 --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java @@ -0,0 +1,75 @@ +/** + * 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.rest.model; +import com.fasterxml.jackson.annotation.JsonIgnore; +import org.dspace.app.rest.RestResourceController; + +/** + * The REST object for the Feedback objects + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk at 4science.com) + */ +public class FeedbackRest extends BaseObjectRest { + + private static final long serialVersionUID = 1L; + + public static final String NAME = "feedback"; + public static final String CATEGORY = RestAddressableModel.CORE; + + private String page; + private String email; + private String message; + + public String getEmail() { + return email; + } + + public void setEmail(String email) { + this.email = email; + } + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } + + public String getPage() { + return page; + } + + public void setPage(String page) { + this.page = page; + } + + @Override + @JsonIgnore + public Integer getId() { + return id; + } + + @Override + @JsonIgnore + public String getType() { + return NAME; + } + + @Override + public String getCategory() { + return CATEGORY; + } + + @Override + @SuppressWarnings("rawtypes") + public Class getController() { + return RestResourceController.class; + } + +} \ No newline at end of file diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java new file mode 100644 index 0000000000..c0e24e549b --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java @@ -0,0 +1,112 @@ +/** + * 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.rest.repository; +import java.io.IOException; +import java.sql.SQLException; +import java.util.Date; +import java.util.Objects; +import javax.mail.MessagingException; +import javax.servlet.http.HttpServletRequest; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.commons.lang.StringUtils; +import org.dspace.app.rest.exception.DSpaceBadRequestException; +import org.dspace.app.rest.exception.DSpaceFeedbackNotFoundException; +import org.dspace.app.rest.exception.RepositoryMethodNotImplementedException; +import org.dspace.app.rest.exception.UnprocessableEntityException; +import org.dspace.app.rest.model.FeedbackRest; +import org.dspace.authorize.AuthorizeException; +import org.dspace.core.Context; +import org.dspace.core.Email; +import org.dspace.core.I18nUtil; +import org.dspace.services.ConfigurationService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.stereotype.Component; + +/** + * This is the Repository that takes care of the operations on the {@link FeedbackRest} objects + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk@4science.com) + */ +@Component(FeedbackRest.CATEGORY + "." + FeedbackRest.NAME) +public class FeedbackRestRepository extends DSpaceRestRepository { + + @Autowired + private ConfigurationService configurationService; + + @Override + @PreAuthorize("hasAuthority('AUTHENTICATED')") + public Page findAll(Context context, Pageable pageable) { + throw new RepositoryMethodNotImplementedException(FeedbackRest.NAME, "findAll"); + } + + @Override + @PreAuthorize("hasAuthority('AUTHENTICATED')") + public FeedbackRest findOne(Context context, Integer id) { + throw new RepositoryMethodNotImplementedException(FeedbackRest.NAME, "findOne"); + } + + @Override + @PreAuthorize("permitAll()") + protected FeedbackRest createAndReturn(Context context) throws AuthorizeException, SQLException { + HttpServletRequest req = getRequestService().getCurrentRequest().getHttpServletRequest(); + ObjectMapper mapper = new ObjectMapper(); + FeedbackRest feedbackRest = null; + + String recipientEmail = configurationService.getProperty("feedback.recipient"); + if (StringUtils.isBlank(recipientEmail)) { + throw new DSpaceFeedbackNotFoundException("Recipient's email was not found!"); + } + + try { + feedbackRest = mapper.readValue(req.getInputStream(), FeedbackRest.class); + } catch (IOException exIO) { + throw new UnprocessableEntityException("error parsing the body " + exIO.getMessage(), exIO); + } + + String session = req.getSession().getId(); + String agent = req.getHeader("User-Agent"); + String currentUserEmail = StringUtils.EMPTY; + + if (Objects.nonNull(context.getCurrentUser())) { + currentUserEmail = context.getCurrentUser().getEmail(); + } + + String senderEmail = feedbackRest.getEmail(); + String message = feedbackRest.getMessage(); + String page = feedbackRest.getPage(); + + if (StringUtils.isBlank(senderEmail) || StringUtils.isBlank(message)) { + throw new DSpaceBadRequestException("Filds as e-mail and message are mandatory!"); + } + + try { + Email email = Email.getEmail(I18nUtil.getEmailFilename(context.getCurrentLocale(), "feedback")); + email.addArgument(new Date()); // Date + email.addArgument(senderEmail); // Email + email.addArgument(currentUserEmail); // Logged in as + email.addArgument(page); // Referring page + email.addArgument(agent); // User agent + email.addArgument(session); // Session ID + email.addArgument(message); // The feedback itself + email.send(); + } catch (IOException | MessagingException e) { + throw new RuntimeException(e.getMessage(), e); + } + return null; + } + + @Override + public Class getDomainClass() { + return FeedbackRest.class; + } + +} \ No newline at end of file From 61bbf3840dae076307209554fbfc0ee1917bb527 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 3 Dec 2021 13:32:12 +0100 Subject: [PATCH 04/19] added tests --- .../app/rest/FeedbackRestRepositoryIT.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java new file mode 100644 index 0000000000..496b52fb6f --- /dev/null +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java @@ -0,0 +1,89 @@ +/** + * 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.rest; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.dspace.app.rest.model.FeedbackRest; +import org.dspace.app.rest.test.AbstractControllerIntegrationTest; +import org.dspace.services.ConfigurationService; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; + +/** + * Integration test class for the feedback endpoint + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk@4science.com) + */ +public class FeedbackRestRepositoryIT extends AbstractControllerIntegrationTest { + + @Autowired + private ConfigurationService configurationService; + + @Test + public void findAllTest() throws Exception { + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(get("/api/core/feedbacks")) + .andExpect(status().isMethodNotAllowed()); + } + + @Test + public void findOneTest() throws Exception { + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(get("/api/core/feedbacks/1")) + .andExpect(status().isMethodNotAllowed()); + } + + @Test + public void sendFeedbackTest() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + FeedbackRest feedbackRest = new FeedbackRest(); + + feedbackRest.setEmail("misha.boychuk@test.com"); + feedbackRest.setMessage("My feedback!"); + + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(post("/api/core/feedbacks") + .content(mapper.writeValueAsBytes(feedbackRest)) + .contentType(contentType)) + .andExpect(status().isCreated()); + } + + @Test + public void sendFeedbackWithRecipientEmailNotConfiguredTest() throws Exception { + configurationService.setProperty("feedback.recipient", null); + ObjectMapper mapper = new ObjectMapper(); + FeedbackRest feedbackRest = new FeedbackRest(); + + feedbackRest.setEmail("misha.boychuk@test.com"); + feedbackRest.setMessage("My feedback!"); + + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(post("/api/core/feedbacks") + .content(mapper.writeValueAsBytes(feedbackRest)) + .contentType(contentType)) + .andExpect(status().isNotFound()); + } + + @Test + public void sendFeedbackBadRequestTest() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + FeedbackRest feedbackRest = new FeedbackRest(); + + feedbackRest.setMessage("My feedback!"); + + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(post("/api/core/feedbacks") + .content(mapper.writeValueAsBytes(feedbackRest)) + .contentType(contentType)) + .andExpect(status().isBadRequest()); + } + +} \ No newline at end of file From 8a5df441045becb893ccd63103de3ebd8d80b707 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 3 Dec 2021 13:32:54 +0100 Subject: [PATCH 05/19] implemented canSendFeedback feature --- .../impl/CanSendFeedbackFeature.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanSendFeedbackFeature.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanSendFeedbackFeature.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanSendFeedbackFeature.java new file mode 100644 index 0000000000..4f88be5266 --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/authorization/impl/CanSendFeedbackFeature.java @@ -0,0 +1,49 @@ +/** + * 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.rest.authorization.impl; +import java.sql.SQLException; + +import org.apache.commons.lang.StringUtils; +import org.dspace.app.rest.authorization.AuthorizationFeature; +import org.dspace.app.rest.authorization.AuthorizationFeatureDocumentation; +import org.dspace.app.rest.model.BaseObjectRest; +import org.dspace.app.rest.model.SiteRest; +import org.dspace.core.Context; +import org.dspace.services.ConfigurationService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +/** + * The send feedback feature. It can be used to verify if the parameter that contain + * recipient e-mail is configured. + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk at 4science.com) + */ +@Component +@AuthorizationFeatureDocumentation(name = CanSendFeedbackFeature.NAME, + description = "It can be used to verify if the parameter that contain recipient e-mail is configured.") +public class CanSendFeedbackFeature implements AuthorizationFeature { + + public static final String NAME = "canSendFeedback"; + + @Autowired + private ConfigurationService configurationService; + + @Override + @SuppressWarnings("rawtypes") + public boolean isAuthorized(Context context, BaseObjectRest object) throws SQLException { + String recipientEmail = configurationService.getProperty("feedback.recipient"); + return StringUtils.isNotBlank(recipientEmail); + } + + @Override + public String[] getSupportedTypes() { + return new String[] { SiteRest.CATEGORY + "." + SiteRest.NAME }; + } + +} \ No newline at end of file From b4da463479eba7a186c8577d4622a7297218e98f Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 3 Dec 2021 13:33:23 +0100 Subject: [PATCH 06/19] added tests for canSendFeedback feature --- .../CanSendFeedbackFeatureIT.java | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanSendFeedbackFeatureIT.java diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanSendFeedbackFeatureIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanSendFeedbackFeatureIT.java new file mode 100644 index 0000000000..b4b9ab39c3 --- /dev/null +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/authorization/CanSendFeedbackFeatureIT.java @@ -0,0 +1,110 @@ +/** + * 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.rest.authorization; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import org.dspace.app.rest.authorization.impl.CanSendFeedbackFeature; +import org.dspace.app.rest.converter.SiteConverter; +import org.dspace.app.rest.matcher.AuthorizationMatcher; +import org.dspace.app.rest.model.SiteRest; +import org.dspace.app.rest.projection.DefaultProjection; +import org.dspace.app.rest.test.AbstractControllerIntegrationTest; +import org.dspace.content.Site; +import org.dspace.content.service.SiteService; +import org.dspace.services.ConfigurationService; +import org.hamcrest.Matchers; +import org.junit.Before; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; + +/** + * Test for the canSendFeedback authorization feature. + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk at 4science.com) + */ +public class CanSendFeedbackFeatureIT extends AbstractControllerIntegrationTest { + + @Autowired + private SiteService siteService; + @Autowired + private SiteConverter siteConverter; + @Autowired + private ConfigurationService configurationService; + @Autowired + private AuthorizationFeatureService authorizationFeatureService; + + final String feature = "canSendFeedback"; + + private AuthorizationFeature canSendFeedbackFeature; + + @Override + @Before + public void setUp() throws Exception { + super.setUp(); + canSendFeedbackFeature = authorizationFeatureService.find(CanSendFeedbackFeature.NAME); + } + + @Test + public void canSendFeedbackFeatureTest() throws Exception { + configurationService.setProperty("feedback.recipient", "myemail@test.com"); + + Site site = siteService.findSite(context); + SiteRest siteRest = siteConverter.convert(site, DefaultProjection.DEFAULT); + + String tokenAdmin = getAuthToken(admin.getEmail(), password); + String tokenEperson = getAuthToken(eperson.getEmail(), password); + + // define authorizations + Authorization authAdminSite = new Authorization(admin, canSendFeedbackFeature, siteRest); + Authorization authAnonymousSite = new Authorization(null, canSendFeedbackFeature, siteRest); + Authorization authEPersonSite = new Authorization(eperson, canSendFeedbackFeature, siteRest); + + getClient(tokenAdmin).perform(get("/api/authz/authorizations/" + authAdminSite.getID())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", Matchers.is( + AuthorizationMatcher.matchAuthorization(authAdminSite)))); + + getClient(tokenEperson).perform(get("/api/authz/authorizations/" + authEPersonSite.getID())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", Matchers.is( + AuthorizationMatcher.matchAuthorization(authEPersonSite)))); + + getClient().perform(get("/api/authz/authorizations/" + authAnonymousSite.getID())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", Matchers.is( + AuthorizationMatcher.matchAuthorization(authAnonymousSite)))); + } + + @Test + public void canSendFeedbackFeatureWithRecipientEmailNotConfiguredTest() throws Exception { + configurationService.setProperty("feedback.recipient", null); + + Site site = siteService.findSite(context); + SiteRest siteRest = siteConverter.convert(site, DefaultProjection.DEFAULT); + + String tokenAdmin = getAuthToken(admin.getEmail(), password); + String tokenEperson = getAuthToken(eperson.getEmail(), password); + + // define authorizations + Authorization authAdminSite = new Authorization(admin, canSendFeedbackFeature, siteRest); + Authorization authAnonymousSite = new Authorization(null, canSendFeedbackFeature, siteRest); + Authorization authEPersonSite = new Authorization(eperson, canSendFeedbackFeature, siteRest); + + getClient(tokenAdmin).perform(get("/api/authz/authorizations/" + authAdminSite.getID())) + .andExpect(status().isNotFound()); + + getClient(tokenEperson).perform(get("/api/authz/authorizations/" + authEPersonSite.getID())) + .andExpect(status().isNotFound()); + + getClient().perform(get("/api/authz/authorizations/" + authAnonymousSite.getID())) + .andExpect(status().isNotFound()); + } + +} \ No newline at end of file From 96b66fe7ff9a5ac839583e9bac25cdee49f4c6bb Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 3 Dec 2021 17:05:08 +0100 Subject: [PATCH 07/19] refactoring FeedbackRestRepository --- .../dspace/content/FeedbackServiceImpl.java | 45 +++++++++++++++++++ .../content/service/FeedbackService.java | 25 +++++++++++ .../repository/FeedbackRestRepository.java | 34 +++++--------- dspace/config/spring/api/core-services.xml | 1 + 4 files changed, 83 insertions(+), 22 deletions(-) create mode 100644 dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java create mode 100644 dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java diff --git a/dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java new file mode 100644 index 0000000000..5b182ab3a2 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java @@ -0,0 +1,45 @@ +/** + * 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.content; +import java.io.IOException; +import java.util.Date; +import java.util.Objects; +import javax.mail.MessagingException; +import javax.servlet.http.HttpServletRequest; + +import org.apache.commons.lang.StringUtils; +import org.dspace.content.service.FeedbackService; +import org.dspace.core.Context; +import org.dspace.core.Email; +import org.dspace.core.I18nUtil; + +public class FeedbackServiceImpl implements FeedbackService { + + @Override + public void sendEmail(Context context, HttpServletRequest request, String recipientEmail, String senderEmail, + String message, String page) throws IOException, MessagingException { + String session = request.getSession().getId(); + String agent = request.getHeader("User-Agent"); + String currentUserEmail = StringUtils.EMPTY; + + if (Objects.nonNull(context.getCurrentUser())) { + currentUserEmail = context.getCurrentUser().getEmail(); + } + Email email = Email.getEmail(I18nUtil.getEmailFilename(context.getCurrentLocale(), "feedback")); + email.addRecipient(recipientEmail); + email.addArgument(new Date()); // Date + email.addArgument(senderEmail); // Email + email.addArgument(currentUserEmail); // Logged in as + email.addArgument(page); // Referring page + email.addArgument(agent); // User agent + email.addArgument(session); // Session ID + email.addArgument(message); // The feedback itself + email.send(); + } + +} \ No newline at end of file diff --git a/dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java b/dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java new file mode 100644 index 0000000000..c643408679 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java @@ -0,0 +1,25 @@ +/** + * 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.content.service; +import java.io.IOException; +import javax.mail.MessagingException; +import javax.servlet.http.HttpServletRequest; + +import org.dspace.core.Context; + +/** + * Service interface class for the Feedback object. + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk@4science.com) + */ +public interface FeedbackService { + + public void sendEmail(Context context, HttpServletRequest request, String recipientEmail, String senderEmail, + String message, String page) throws IOException, MessagingException; + +} \ No newline at end of file diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java index c0e24e549b..c284a01a59 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java @@ -8,8 +8,6 @@ package org.dspace.app.rest.repository; import java.io.IOException; import java.sql.SQLException; -import java.util.Date; -import java.util.Objects; import javax.mail.MessagingException; import javax.servlet.http.HttpServletRequest; @@ -21,9 +19,8 @@ import org.dspace.app.rest.exception.RepositoryMethodNotImplementedException; import org.dspace.app.rest.exception.UnprocessableEntityException; import org.dspace.app.rest.model.FeedbackRest; import org.dspace.authorize.AuthorizeException; +import org.dspace.content.service.FeedbackService; import org.dspace.core.Context; -import org.dspace.core.Email; -import org.dspace.core.I18nUtil; import org.dspace.services.ConfigurationService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; @@ -39,6 +36,8 @@ import org.springframework.stereotype.Component; @Component(FeedbackRest.CATEGORY + "." + FeedbackRest.NAME) public class FeedbackRestRepository extends DSpaceRestRepository { + @Autowired + private FeedbackService feedbackService; @Autowired private ConfigurationService configurationService; @@ -72,32 +71,15 @@ public class FeedbackRestRepository extends DSpaceRestRepository + From 3a600e930abc4ed276499753f5b069f79f81342f Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 3 Dec 2021 17:05:27 +0100 Subject: [PATCH 08/19] refactoring tests --- .../app/rest/FeedbackRestRepositoryIT.java | 96 ++++++++++++++----- 1 file changed, 70 insertions(+), 26 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java index 496b52fb6f..1e67000974 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java @@ -6,13 +6,22 @@ * http://www.dspace.org/license/ */ package org.dspace.app.rest; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNotNull; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import com.fasterxml.jackson.databind.ObjectMapper; import org.dspace.app.rest.model.FeedbackRest; +import org.dspace.app.rest.repository.FeedbackRestRepository; import org.dspace.app.rest.test.AbstractControllerIntegrationTest; +import org.dspace.content.FeedbackServiceImpl; +import org.dspace.content.service.FeedbackService; import org.dspace.services.ConfigurationService; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -26,6 +35,8 @@ public class FeedbackRestRepositoryIT extends AbstractControllerIntegrationTest @Autowired private ConfigurationService configurationService; + @Autowired + private FeedbackRestRepository feedbackRestRepository; @Test public void findAllTest() throws Exception { @@ -43,47 +54,80 @@ public class FeedbackRestRepositoryIT extends AbstractControllerIntegrationTest @Test public void sendFeedbackTest() throws Exception { - ObjectMapper mapper = new ObjectMapper(); - FeedbackRest feedbackRest = new FeedbackRest(); + configurationService.setProperty("feedback.recipient", "recipient.email@test.com"); + FeedbackService originFeedbackService = feedbackRestRepository.getFeedbackService(); + try { + FeedbackService feedbackServiceMock = mock (FeedbackServiceImpl.class); + feedbackRestRepository.setFeedbackService(feedbackServiceMock); - feedbackRest.setEmail("misha.boychuk@test.com"); - feedbackRest.setMessage("My feedback!"); + ObjectMapper mapper = new ObjectMapper(); + FeedbackRest feedbackRest = new FeedbackRest(); + + feedbackRest.setEmail("misha.boychuk@test.com"); + feedbackRest.setMessage("My feedback!"); + + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(post("/api/core/feedbacks") + .content(mapper.writeValueAsBytes(feedbackRest)) + .contentType(contentType)) + .andExpect(status().isCreated()); + + verify(feedbackServiceMock).sendEmail(isNotNull(), isNotNull(), eq("recipient.email@test.com"), + eq("misha.boychuk@test.com"), eq("My feedback!"), isNull()); + + verifyNoMoreInteractions(feedbackServiceMock); + } finally { + feedbackRestRepository.setFeedbackService(originFeedbackService); + } - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(post("/api/core/feedbacks") - .content(mapper.writeValueAsBytes(feedbackRest)) - .contentType(contentType)) - .andExpect(status().isCreated()); } @Test public void sendFeedbackWithRecipientEmailNotConfiguredTest() throws Exception { configurationService.setProperty("feedback.recipient", null); - ObjectMapper mapper = new ObjectMapper(); - FeedbackRest feedbackRest = new FeedbackRest(); + FeedbackService originFeedbackService = feedbackRestRepository.getFeedbackService(); + try { + FeedbackService feedbackServiceMock = mock (FeedbackServiceImpl.class); + feedbackRestRepository.setFeedbackService(feedbackServiceMock); + ObjectMapper mapper = new ObjectMapper(); + FeedbackRest feedbackRest = new FeedbackRest(); - feedbackRest.setEmail("misha.boychuk@test.com"); - feedbackRest.setMessage("My feedback!"); + feedbackRest.setEmail("misha.boychuk@test.com"); + feedbackRest.setMessage("My feedback!"); - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(post("/api/core/feedbacks") - .content(mapper.writeValueAsBytes(feedbackRest)) - .contentType(contentType)) - .andExpect(status().isNotFound()); + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(post("/api/core/feedbacks") + .content(mapper.writeValueAsBytes(feedbackRest)) + .contentType(contentType)) + .andExpect(status().isNotFound()); + + verifyNoMoreInteractions(feedbackServiceMock); + } finally { + feedbackRestRepository.setFeedbackService(originFeedbackService); + } } @Test public void sendFeedbackBadRequestTest() throws Exception { - ObjectMapper mapper = new ObjectMapper(); - FeedbackRest feedbackRest = new FeedbackRest(); + FeedbackService originFeedbackService = feedbackRestRepository.getFeedbackService(); + try { + FeedbackService feedbackServiceMock = mock (FeedbackServiceImpl.class); + feedbackRestRepository.setFeedbackService(feedbackServiceMock); + ObjectMapper mapper = new ObjectMapper(); + FeedbackRest feedbackRest = new FeedbackRest(); - feedbackRest.setMessage("My feedback!"); + feedbackRest.setMessage("My feedback!"); - String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(post("/api/core/feedbacks") - .content(mapper.writeValueAsBytes(feedbackRest)) - .contentType(contentType)) - .andExpect(status().isBadRequest()); + String authToken = getAuthToken(admin.getEmail(), password); + getClient(authToken).perform(post("/api/core/feedbacks") + .content(mapper.writeValueAsBytes(feedbackRest)) + .contentType(contentType)) + .andExpect(status().isBadRequest()); + + verifyNoMoreInteractions(feedbackServiceMock); + } finally { + feedbackRestRepository.setFeedbackService(originFeedbackService); + } } } \ No newline at end of file From 881dd059cc6060f73d55a72d7a92341a3b40e42d Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Wed, 15 Dec 2021 11:41:31 +0100 Subject: [PATCH 09/19] changed category of endpoint from core to tools --- .../java/org/dspace/app/rest/model/FeedbackRest.java | 2 +- .../org/dspace/app/rest/FeedbackRestRepositoryIT.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java index 5ab04aa062..00f1e92c87 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/FeedbackRest.java @@ -19,7 +19,7 @@ public class FeedbackRest extends BaseObjectRest { private static final long serialVersionUID = 1L; public static final String NAME = "feedback"; - public static final String CATEGORY = RestAddressableModel.CORE; + public static final String CATEGORY = RestAddressableModel.TOOLS; private String page; private String email; diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java index 1e67000974..fd61f7a7a2 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/FeedbackRestRepositoryIT.java @@ -41,14 +41,14 @@ public class FeedbackRestRepositoryIT extends AbstractControllerIntegrationTest @Test public void findAllTest() throws Exception { String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/core/feedbacks")) + getClient(authToken).perform(get("/api/tools/feedbacks")) .andExpect(status().isMethodNotAllowed()); } @Test public void findOneTest() throws Exception { String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(get("/api/core/feedbacks/1")) + getClient(authToken).perform(get("/api/tools/feedbacks/1")) .andExpect(status().isMethodNotAllowed()); } @@ -67,7 +67,7 @@ public class FeedbackRestRepositoryIT extends AbstractControllerIntegrationTest feedbackRest.setMessage("My feedback!"); String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(post("/api/core/feedbacks") + getClient(authToken).perform(post("/api/tools/feedbacks") .content(mapper.writeValueAsBytes(feedbackRest)) .contentType(contentType)) .andExpect(status().isCreated()); @@ -96,7 +96,7 @@ public class FeedbackRestRepositoryIT extends AbstractControllerIntegrationTest feedbackRest.setMessage("My feedback!"); String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(post("/api/core/feedbacks") + getClient(authToken).perform(post("/api/tools/feedbacks") .content(mapper.writeValueAsBytes(feedbackRest)) .contentType(contentType)) .andExpect(status().isNotFound()); @@ -119,7 +119,7 @@ public class FeedbackRestRepositoryIT extends AbstractControllerIntegrationTest feedbackRest.setMessage("My feedback!"); String authToken = getAuthToken(admin.getEmail(), password); - getClient(authToken).perform(post("/api/core/feedbacks") + getClient(authToken).perform(post("/api/tools/feedbacks") .content(mapper.writeValueAsBytes(feedbackRest)) .contentType(contentType)) .andExpect(status().isBadRequest()); From 76827e46c4597e3b913aec0d62b086c85566a7f9 Mon Sep 17 00:00:00 2001 From: Corrado Lombardi Date: Tue, 21 Dec 2021 15:17:25 +0100 Subject: [PATCH 10/19] removed TODO --- dspace/config/dspace.cfg | 1 - 1 file changed, 1 deletion(-) diff --git a/dspace/config/dspace.cfg b/dspace/config/dspace.cfg index a2c27ffad9..64ca5b15f2 100644 --- a/dspace/config/dspace.cfg +++ b/dspace/config/dspace.cfg @@ -141,7 +141,6 @@ mail.from.address = dspace-noreply@myu.edu # When feedback is submitted via the Feedback form, it is sent to this address # Currently limited to one recipient! -# TODO: UNSUPPORTED in DSpace 7.0 feedback.recipient = dspace-help@myu.edu # General site administration (Webmaster) e-mail From 189d9a60b9d7167400b1f4a944f4c28650865932 Mon Sep 17 00:00:00 2001 From: Corrado Lombardi Date: Wed, 12 Jan 2022 13:08:41 +0100 Subject: [PATCH 11/19] improved documentation and used x-correlation-id instead of session id --- .../java/org/dspace/content/FeedbackServiceImpl.java | 7 ++++++- .../org/dspace/content/service/FeedbackService.java | 11 +++++++++++ dspace/config/dspace.cfg | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java index 5b182ab3a2..7e34af132b 100644 --- a/dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/FeedbackServiceImpl.java @@ -18,12 +18,17 @@ import org.dspace.core.Context; import org.dspace.core.Email; import org.dspace.core.I18nUtil; +/** + * Implementation of {@link FeedbackService} interface. + * It is responsible for sendint a feedback email with content a DSpace user + * fills from feedback section of DSpace. + */ public class FeedbackServiceImpl implements FeedbackService { @Override public void sendEmail(Context context, HttpServletRequest request, String recipientEmail, String senderEmail, String message, String page) throws IOException, MessagingException { - String session = request.getSession().getId(); + String session = request.getHeader("x-correlation-id"); String agent = request.getHeader("User-Agent"); String currentUserEmail = StringUtils.EMPTY; diff --git a/dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java b/dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java index c643408679..d21afd6780 100644 --- a/dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java +++ b/dspace-api/src/main/java/org/dspace/content/service/FeedbackService.java @@ -19,6 +19,17 @@ import org.dspace.core.Context; */ public interface FeedbackService { + /** + * This method sends the feeback email to the recipient passed as parameter + * @param context current DSpace application context + * @param request current servlet request + * @param recipientEmail recipient to which mail is sent + * @param senderEmail email address of the sender + * @param message message body + * @param page page from which user accessed and filled feedback form + * @throws IOException + * @throws MessagingException + */ public void sendEmail(Context context, HttpServletRequest request, String recipientEmail, String senderEmail, String message, String page) throws IOException, MessagingException; diff --git a/dspace/config/dspace.cfg b/dspace/config/dspace.cfg index 64ca5b15f2..67c9dabb19 100644 --- a/dspace/config/dspace.cfg +++ b/dspace/config/dspace.cfg @@ -141,6 +141,7 @@ mail.from.address = dspace-noreply@myu.edu # When feedback is submitted via the Feedback form, it is sent to this address # Currently limited to one recipient! +# if this property is empty or commented out, feedback form is disabled feedback.recipient = dspace-help@myu.edu # General site administration (Webmaster) e-mail From a6a74463b0acdda2822e92f713d5d45deacf381d Mon Sep 17 00:00:00 2001 From: Corrado Lombardi Date: Wed, 12 Jan 2022 13:09:29 +0100 Subject: [PATCH 12/19] updated messages in exceptions thrown when feedback recipient is disabled and when mandatory fields are missing --- .../dspace/app/rest/repository/FeedbackRestRepository.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java index c284a01a59..8fd28bbe94 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/FeedbackRestRepository.java @@ -62,7 +62,8 @@ public class FeedbackRestRepository extends DSpaceRestRepository Date: Fri, 14 Jan 2022 13:10:10 +0100 Subject: [PATCH 13/19] Fix inputstream connection leak when downloading files --- .../java/org/dspace/curate/CitationPage.java | 7 +- .../CitationDocumentServiceImpl.java | 25 ++++--- .../service/CitationDocumentService.java | 9 ++- .../app/rest/BitstreamRestController.java | 67 +++++++------------ .../app/rest/SitemapRestController.java | 3 +- .../app/rest/utils/BitstreamResource.java | 48 +++++++++++-- .../rest/utils/HttpHeadersInitializer.java | 17 +---- 7 files changed, 97 insertions(+), 79 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/curate/CitationPage.java b/dspace-api/src/main/java/org/dspace/curate/CitationPage.java index 386bf0ba92..0e6d4992b1 100644 --- a/dspace-api/src/main/java/org/dspace/curate/CitationPage.java +++ b/dspace-api/src/main/java/org/dspace/curate/CitationPage.java @@ -15,7 +15,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.Logger; import org.dspace.authorize.AuthorizeException; import org.dspace.content.Bitstream; @@ -154,10 +153,10 @@ public class CitationPage extends AbstractCurationTask { + bitstream.getName() + " is citable."); try { //Create the cited document - Pair citedDocument = - citationDocument.makeCitedDocument(Curator.curationContext(), bitstream); + InputStream citedInputStream = + citationDocument.getCitedDocument(Curator.curationContext(), bitstream); //Add the cited document to the approiate bundle - this.addCitedPageToItem(citedDocument.getLeft(), bundle, pBundle, + this.addCitedPageToItem(citedInputStream, bundle, pBundle, dBundle, displayMap, item, bitstream); } catch (Exception e) { //Could be many things, but nothing that should be diff --git a/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java b/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java index 4f9bdc2bbc..e81066071c 100644 --- a/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java @@ -21,7 +21,6 @@ import java.util.List; import java.util.Set; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.pdfbox.pdmodel.PDDocument; @@ -297,8 +296,21 @@ public class CitationDocumentServiceImpl implements CitationDocumentService, Ini } @Override - public Pair makeCitedDocument(Context context, Bitstream bitstream) - throws IOException, SQLException, AuthorizeException { + public InputStream getCitedDocument(Context context, Bitstream bitstream) + throws SQLException, AuthorizeException, IOException { + byte [] citedDocumentContent = getCitedDocumentContent(context, bitstream); + return new ByteArrayInputStream(citedDocumentContent); + } + + @Override + public Long getCitedDocumentLength(Context context, Bitstream bitstream) + throws SQLException, AuthorizeException, IOException { + byte[] citedDocumentContent = getCitedDocumentContent(context, bitstream); + return Long.valueOf(citedDocumentContent.length); + } + + private byte[] getCitedDocumentContent(Context context, Bitstream bitstream) + throws SQLException, AuthorizeException, IOException { PDDocument document = new PDDocument(); PDDocument sourceDocument = new PDDocument(); try { @@ -307,15 +319,10 @@ public class CitationDocumentServiceImpl implements CitationDocumentService, Ini PDPage coverPage = new PDPage(citationPageFormat); generateCoverPage(context, document, coverPage, item); addCoverPageToDocument(document, sourceDocument, coverPage); - - //We already have the full PDF in memory, so keep it there try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { document.save(out); - - byte[] data = out.toByteArray(); - return Pair.of((InputStream) new ByteArrayInputStream(data), Long.valueOf(data.length)); + return out.toByteArray(); } - } finally { sourceDocument.close(); document.close(); diff --git a/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java b/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java index 4a59de3f5f..a116ccf0b1 100644 --- a/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java +++ b/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java @@ -11,7 +11,6 @@ import java.io.IOException; import java.io.InputStream; import java.sql.SQLException; -import org.apache.commons.lang3.tuple.Pair; import org.apache.pdfbox.pdmodel.PDPage; import org.apache.pdfbox.pdmodel.PDPageContentStream; import org.apache.pdfbox.pdmodel.font.PDFont; @@ -84,8 +83,12 @@ public interface CitationDocumentService { * @throws SQLException if database error * @throws AuthorizeException if authorization error */ - public Pair makeCitedDocument(Context context, Bitstream bitstream) - throws IOException, SQLException, AuthorizeException; + public InputStream getCitedDocument(Context context, Bitstream bitstream) + throws SQLException, AuthorizeException, IOException; + + public Long getCitedDocumentLength(Context context, Bitstream bitstream) + throws SQLException, AuthorizeException, IOException; + /** * @param page page diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java index 2ce1c4ffc7..7185dccfdb 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java @@ -12,7 +12,6 @@ import static org.dspace.app.rest.utils.RegexUtils.REGEX_REQUESTMAPPING_IDENTIFI import static org.springframework.web.bind.annotation.RequestMethod.PUT; import java.io.IOException; -import java.io.InputStream; import java.sql.SQLException; import java.util.List; import java.util.UUID; @@ -22,7 +21,6 @@ import javax.ws.rs.core.Response; import org.apache.catalina.connector.ClientAbortException; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.Logger; import org.dspace.app.rest.converter.ConverterService; import org.dspace.app.rest.exception.DSpaceBadRequestException; @@ -38,6 +36,7 @@ import org.dspace.content.service.BitstreamFormatService; import org.dspace.content.service.BitstreamService; import org.dspace.core.Context; import org.dspace.disseminate.service.CitationDocumentService; +import org.dspace.eperson.EPerson; import org.dspace.services.ConfigurationService; import org.dspace.services.EventService; import org.dspace.usage.UsageEvent; @@ -108,6 +107,11 @@ public class BitstreamRestController { Context context = ContextUtil.obtainContext(request); Bitstream bit = bitstreamService.find(context, uuid); + if (bit == null) { + throw new ResourceNotFoundException(); + } + EPerson currentUser = context.getCurrentUser(); + if (bit == null) { response.sendError(HttpServletResponse.SC_NOT_FOUND); return null; @@ -118,8 +122,6 @@ public class BitstreamRestController { String mimetype = format.getMIMEType(); String name = getBitstreamName(bit, format); - Pair bitstreamTuple = getBitstreamInputStreamAndSize(context, bit); - if (StringUtils.isBlank(request.getHeader("Range"))) { //We only log a download request when serving a request without Range header. This is because //a browser always sends a regular request first to check for Range support. @@ -131,18 +133,15 @@ public class BitstreamRestController { bit)); } - // Pipe the bits - InputStream is = bitstreamTuple.getLeft(); try { - HttpHeadersInitializer httpHeadersInitializer = HttpHeadersInitializer - .fromInputStream(is) - .withBufferSize(BUFFER_SIZE) - .withFileName(name) - .withLength(bitstreamTuple.getRight()) - .withChecksum(bit.getChecksum()) - .withMimetype(mimetype) - .with(request) - .with(response); + HttpHeadersInitializer httpHeadersInitializer = new HttpHeadersInitializer() + .withBufferSize(BUFFER_SIZE) + .withFileName(name) + .withLength(bit.getSizeBytes()) + .withChecksum(bit.getChecksum()) + .withMimetype(mimetype) + .with(request) + .with(response); if (lastModified != null) { httpHeadersInitializer.withLastModified(lastModified); @@ -150,13 +149,21 @@ public class BitstreamRestController { //Determine if we need to send the file as a download or if the browser can open it inline long dispositionThreshold = configurationService.getLongProperty("webui.content_disposition_threshold"); - if (dispositionThreshold >= 0 && bitstreamTuple.getRight() > dispositionThreshold) { + if (dispositionThreshold >= 0 && bit.getSizeBytes() > dispositionThreshold) { httpHeadersInitializer.withDisposition(HttpHeadersInitializer.CONTENT_DISPOSITION_ATTACHMENT); } + long filesize; + if (citationDocumentService.isCitationEnabledForBitstream(bit, context)) { + filesize = citationDocumentService.getCitedDocumentLength(context, bit); + } else { + filesize = bit.getSizeBytes(); + } + org.dspace.app.rest.utils.BitstreamResource bitstreamResource = - new org.dspace.app.rest.utils.BitstreamResource(is, name, uuid, bit.getSizeBytes()); + new org.dspace.app.rest.utils.BitstreamResource( + bit, name, uuid, filesize, currentUser != null ? currentUser.getID() : null); //We have all the data we need, close the connection to the database so that it doesn't stay open during //download/streaming @@ -171,34 +178,12 @@ public class BitstreamRestController { } catch (ClientAbortException ex) { log.debug("Client aborted the request before the download was completed. " + "Client is probably switching to a Range request.", ex); + } catch (Exception e) { + throw e; } return null; } - private Pair getBitstreamInputStreamAndSize(Context context, Bitstream bit) - throws SQLException, IOException, AuthorizeException { - - if (citationDocumentService.isCitationEnabledForBitstream(bit, context)) { - return generateBitstreamWithCitation(context, bit); - } else { - return Pair.of(bitstreamService.retrieve(context, bit),bit.getSizeBytes()); - } - } - - private Pair generateBitstreamWithCitation(Context context, Bitstream bitstream) - throws SQLException, IOException, AuthorizeException { - //Create the cited document - Pair citationDocument = citationDocumentService.makeCitedDocument(context, bitstream); - if (citationDocument.getLeft() == null) { - log.error("CitedDocument was null"); - } else { - if (log.isDebugEnabled()) { - log.debug("CitedDocument was ok, has size " + citationDocument.getRight()); - } - } - return citationDocument; - } - private String getBitstreamName(Bitstream bit, BitstreamFormat format) { String name = bit.getName(); if (name == null) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java index d047f00b7d..efc79f2a39 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/SitemapRestController.java @@ -117,8 +117,7 @@ public class SitemapRestController { HttpServletRequest request) throws SQLException, IOException { // Pipe the bits try (InputStream is = new FileInputStream(foundSitemapFile)) { - HttpHeadersInitializer sender = HttpHeadersInitializer - .fromInputStream(is) + HttpHeadersInitializer sender = new HttpHeadersInitializer() .withBufferSize(BUFFER_SIZE) .withFileName(foundSitemapFile.getName()) .withLength(foundSitemapFile.length()) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java index d338ddfaf7..57ea2bcffa 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java @@ -9,8 +9,19 @@ package org.dspace.app.rest.utils; import java.io.IOException; import java.io.InputStream; +import java.sql.SQLException; import java.util.UUID; +import org.dspace.authorize.AuthorizeException; +import org.dspace.content.Bitstream; +import org.dspace.content.factory.ContentServiceFactory; +import org.dspace.content.service.BitstreamService; +import org.dspace.core.Context; +import org.dspace.disseminate.service.CitationDocumentService; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.factory.EPersonServiceFactory; +import org.dspace.eperson.service.EPersonService; +import org.dspace.utils.DSpace; import org.springframework.core.io.AbstractResource; /** @@ -21,16 +32,24 @@ import org.springframework.core.io.AbstractResource; */ public class BitstreamResource extends AbstractResource { - private InputStream inputStream; + private Bitstream bitstream; private String name; private UUID uuid; private long sizeBytes; + private UUID currentUserUUID; - public BitstreamResource(InputStream inputStream, String name, UUID uuid, long sizeBytes) { - this.inputStream = inputStream; + private BitstreamService bitstreamService = ContentServiceFactory.getInstance().getBitstreamService(); + private EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); + private CitationDocumentService citationDocumentService = + new DSpace().getServiceManager() + .getServicesByType(CitationDocumentService.class).get(0); + + public BitstreamResource(Bitstream bitstream, String name, UUID uuid, long sizeBytes, UUID currentUserUUID) { + this.bitstream = bitstream; this.name = name; this.uuid = uuid; this.sizeBytes = sizeBytes; + this.currentUserUUID = currentUserUUID; } @Override @@ -40,7 +59,28 @@ public class BitstreamResource extends AbstractResource { @Override public InputStream getInputStream() throws IOException { - return inputStream; + Context context = new Context(); + try { + EPerson currentUser = ePersonService.find(context, currentUserUUID); + context.setCurrentUser(currentUser); + InputStream out; + + if (citationDocumentService.isCitationEnabledForBitstream(bitstream, context)) { + out = citationDocumentService.getCitedDocument(context, bitstream); + } else { + out = bitstreamService.retrieve(context, bitstream); + } + + return out; + } catch (SQLException | AuthorizeException e) { + throw new IOException(e); + } finally { + try { + context.complete(); + } catch (SQLException e) { + throw new IOException(e); + } + } } @Override diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java index e5f3a9365c..22ae8ad3d9 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/HttpHeadersInitializer.java @@ -10,9 +10,7 @@ package org.dspace.app.rest.utils; import static java.util.Objects.isNull; import static java.util.Objects.nonNull; -import java.io.BufferedInputStream; import java.io.IOException; -import java.io.InputStream; import java.util.Arrays; import java.util.Collections; import javax.servlet.http.HttpServletRequest; @@ -64,7 +62,6 @@ public class HttpHeadersInitializer { //no-cache so request is always performed for logging private static final String CACHE_CONTROL_SETTING = "private,no-cache"; - private BufferedInputStream inputStream; private HttpServletRequest request; private HttpServletResponse response; private String contentType; @@ -74,14 +71,8 @@ public class HttpHeadersInitializer { private String fileName; private String checksum; - public HttpHeadersInitializer(final InputStream inputStream) { + public HttpHeadersInitializer() { //Convert to BufferedInputStream so we can re-read the stream - this.inputStream = new BufferedInputStream(inputStream); - } - - - public static HttpHeadersInitializer fromInputStream(InputStream inputStream) { - return new HttpHeadersInitializer(inputStream); } public HttpHeadersInitializer with(HttpServletRequest httpRequest) { @@ -198,12 +189,6 @@ public class HttpHeadersInitializer { return false; } - if (inputStream == null) { - log.error("Input stream has no content"); - response.sendError(HttpServletResponse.SC_NOT_FOUND); - return false; - } - if (StringUtils.isEmpty(fileName)) { response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); return false; From 441c62239a61ce3197b0aab986175d94801ba068 Mon Sep 17 00:00:00 2001 From: Kevin Van de Velde Date: Wed, 19 Jan 2022 13:23:02 +0100 Subject: [PATCH 14/19] Fix inputstream connection leak when downloading files: Bugfix for cover pages so that the correct file size is used --- .../app/rest/BitstreamRestController.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java index 7185dccfdb..d22e5c8131 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java @@ -134,10 +134,17 @@ public class BitstreamRestController { } try { + long filesize; + if (citationDocumentService.isCitationEnabledForBitstream(bit, context)) { + filesize = citationDocumentService.getCitedDocumentLength(context, bit); + } else { + filesize = bit.getSizeBytes(); + } + HttpHeadersInitializer httpHeadersInitializer = new HttpHeadersInitializer() .withBufferSize(BUFFER_SIZE) .withFileName(name) - .withLength(bit.getSizeBytes()) + .withLength(filesize) .withChecksum(bit.getChecksum()) .withMimetype(mimetype) .with(request) @@ -149,16 +156,10 @@ public class BitstreamRestController { //Determine if we need to send the file as a download or if the browser can open it inline long dispositionThreshold = configurationService.getLongProperty("webui.content_disposition_threshold"); - if (dispositionThreshold >= 0 && bit.getSizeBytes() > dispositionThreshold) { + if (dispositionThreshold >= 0 && filesize > dispositionThreshold) { httpHeadersInitializer.withDisposition(HttpHeadersInitializer.CONTENT_DISPOSITION_ATTACHMENT); } - long filesize; - if (citationDocumentService.isCitationEnabledForBitstream(bit, context)) { - filesize = citationDocumentService.getCitedDocumentLength(context, bit); - } else { - filesize = bit.getSizeBytes(); - } org.dspace.app.rest.utils.BitstreamResource bitstreamResource = From a15478178dbc4e7a5d9362ba27974a12a392e22e Mon Sep 17 00:00:00 2001 From: Kevin Van de Velde Date: Thu, 20 Jan 2022 16:56:06 +0100 Subject: [PATCH 15/19] Removing duplicate Null check --- .../main/java/org/dspace/app/rest/BitstreamRestController.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java index d22e5c8131..ab227682b4 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java @@ -107,9 +107,6 @@ public class BitstreamRestController { Context context = ContextUtil.obtainContext(request); Bitstream bit = bitstreamService.find(context, uuid); - if (bit == null) { - throw new ResourceNotFoundException(); - } EPerson currentUser = context.getCurrentUser(); if (bit == null) { From 6ede71866aa1d79eefe4349a9485c6e16fa7640f Mon Sep 17 00:00:00 2001 From: Michael Spalti Date: Mon, 24 Jan 2022 16:54:14 -0800 Subject: [PATCH 16/19] Using updated constants in shared utils. --- .../app/itemimport/ItemImportServiceImpl.java | 30 ++++++++++++++----- .../IIIFCanvasDimensionServiceImpl.java | 17 ++++++----- .../org/dspace/iiif/util/IIIFSharedUtils.java | 8 +++-- .../app/iiif/service/utils/IIIFUtils.java | 14 ++++----- 4 files changed, 43 insertions(+), 26 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java index 106e0e532a..6a6a70d574 100644 --- a/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/app/itemimport/ItemImportServiceImpl.java @@ -7,6 +7,13 @@ */ package org.dspace.app.itemimport; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_HEIGHT_QUALIFIER; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_IMAGE_ELEMENT; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_LABEL_ELEMENT; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_SCHEMA; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_TOC_ELEMENT; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_WIDTH_QUALIFIER; + import java.io.BufferedInputStream; import java.io.BufferedOutputStream; import java.io.BufferedReader; @@ -1662,6 +1669,8 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea Bitstream bs = null; boolean notfound = true; + boolean updateRequired = false; + if (!isTest) { // find bitstream List bitstreams = itemService.getNonInternalBitstreams(c, myItem); @@ -1696,40 +1705,45 @@ public class ItemImportServiceImpl implements ItemImportService, InitializingBea System.out.println("\tSetting description for " + bitstreamName); bs.setDescription(c, thisDescription); - bitstreamService.update(c, bs); + updateRequired = true; } if (labelExists) { MetadataField metadataField = metadataFieldService - .findByElement(c, "iiif", "label", null); + .findByElement(c, METADATA_IIIF_SCHEMA, METADATA_IIIF_LABEL_ELEMENT, null); System.out.println("\tSetting label to " + thisLabel + " in element " + metadataField.getElement() + " on " + bitstreamName); bitstreamService.addMetadata(c, bs, metadataField, null, thisLabel); - bitstreamService.update(c, bs); + updateRequired = true; } if (heightExists) { MetadataField metadataField = metadataFieldService - .findByElement(c, "iiif", "image", "height"); + .findByElement(c, METADATA_IIIF_SCHEMA, METADATA_IIIF_IMAGE_ELEMENT, + METADATA_IIIF_HEIGHT_QUALIFIER); System.out.println("\tSetting height to " + thisHeight + " in element " + metadataField.getElement() + " on " + bitstreamName); bitstreamService.addMetadata(c, bs, metadataField, null, thisHeight); - bitstreamService.update(c, bs); + updateRequired = true; } if (widthExists) { MetadataField metadataField = metadataFieldService - .findByElement(c, "iiif", "image", "width"); + .findByElement(c, METADATA_IIIF_SCHEMA, METADATA_IIIF_IMAGE_ELEMENT, + METADATA_IIIF_WIDTH_QUALIFIER); System.out.println("\tSetting width to " + thisWidth + " in element " + metadataField.getElement() + " on " + bitstreamName); bitstreamService.addMetadata(c, bs, metadataField, null, thisWidth); - bitstreamService.update(c, bs); + updateRequired = true; } if (tocExists) { MetadataField metadataField = metadataFieldService - .findByElement(c, "iiif", "toc", null); + .findByElement(c, METADATA_IIIF_SCHEMA, METADATA_IIIF_TOC_ELEMENT, null); System.out.println("\tSetting toc to " + thisToc + " in element " + metadataField.getElement() + " on " + bitstreamName); bitstreamService.addMetadata(c, bs, metadataField, null, thisToc); + updateRequired = true; + } + if (updateRequired) { bitstreamService.update(c, bs); } } diff --git a/dspace-api/src/main/java/org/dspace/iiif/canvasdimension/IIIFCanvasDimensionServiceImpl.java b/dspace-api/src/main/java/org/dspace/iiif/canvasdimension/IIIFCanvasDimensionServiceImpl.java index d985786a04..ad36b65ab9 100644 --- a/dspace-api/src/main/java/org/dspace/iiif/canvasdimension/IIIFCanvasDimensionServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/iiif/canvasdimension/IIIFCanvasDimensionServiceImpl.java @@ -7,10 +7,10 @@ */ package org.dspace.iiif.canvasdimension; -import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_HEIGHT; -import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_IMAGE; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_HEIGHT_QUALIFIER; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_IMAGE_ELEMENT; import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_SCHEMA; -import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_WIDTH; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_WIDTH_QUALIFIER; import java.io.IOException; import java.io.InputStream; @@ -62,7 +62,8 @@ public class IIIFCanvasDimensionServiceImpl implements IIIFCanvasDimensionServic private int processed = 0; // used to check for existing canvas dimension - private static final String IIIF_WIDTH_METADATA = "iiif.image.width"; + private static final String IIIF_WIDTH_METADATA = METADATA_IIIF_SCHEMA + "." + METADATA_IIIF_IMAGE_ELEMENT + + "." + METADATA_IIIF_WIDTH_QUALIFIER; @Override public void setForceProcessing(boolean force) { @@ -209,13 +210,13 @@ public class IIIFCanvasDimensionServiceImpl implements IIIFCanvasDimensionServic */ private boolean setBitstreamMetadata(Context context, Bitstream bitstream, int[] dims) throws SQLException { dSpaceObjectService.clearMetadata(context, bitstream, METADATA_IIIF_SCHEMA, - METADATA_IIIF_IMAGE, METADATA_IIIF_WIDTH, Item.ANY); + METADATA_IIIF_IMAGE_ELEMENT, METADATA_IIIF_WIDTH_QUALIFIER, Item.ANY); dSpaceObjectService.setMetadataSingleValue(context, bitstream, METADATA_IIIF_SCHEMA, - METADATA_IIIF_IMAGE, METADATA_IIIF_WIDTH, null, String.valueOf(dims[0])); + METADATA_IIIF_IMAGE_ELEMENT, METADATA_IIIF_WIDTH_QUALIFIER, null, String.valueOf(dims[0])); dSpaceObjectService.clearMetadata(context, bitstream, METADATA_IIIF_SCHEMA, - METADATA_IIIF_IMAGE, METADATA_IIIF_HEIGHT, Item.ANY); + METADATA_IIIF_IMAGE_ELEMENT, METADATA_IIIF_HEIGHT_QUALIFIER, Item.ANY); dSpaceObjectService.setMetadataSingleValue(context, bitstream, METADATA_IIIF_SCHEMA, - METADATA_IIIF_IMAGE, METADATA_IIIF_HEIGHT, null, String.valueOf(dims[1])); + METADATA_IIIF_IMAGE_ELEMENT, METADATA_IIIF_HEIGHT_QUALIFIER, null, String.valueOf(dims[1])); if (!isQuiet) { System.out.println("Added IIIF canvas metadata to bitstream: " + bitstream.getID()); } diff --git a/dspace-api/src/main/java/org/dspace/iiif/util/IIIFSharedUtils.java b/dspace-api/src/main/java/org/dspace/iiif/util/IIIFSharedUtils.java index 21ddb8710e..ad0f974cb1 100644 --- a/dspace-api/src/main/java/org/dspace/iiif/util/IIIFSharedUtils.java +++ b/dspace-api/src/main/java/org/dspace/iiif/util/IIIFSharedUtils.java @@ -36,9 +36,11 @@ public class IIIFSharedUtils { protected static final String IMAGE_SERVER_PATH = "iiif.image.server"; // IIIF metadata definitions public static final String METADATA_IIIF_SCHEMA = "iiif"; - public static final String METADATA_IIIF_IMAGE = "image"; - public static final String METADATA_IIIF_HEIGHT = "height"; - public static final String METADATA_IIIF_WIDTH = "width"; + public static final String METADATA_IIIF_IMAGE_ELEMENT = "image"; + public static final String METADATA_IIIF_TOC_ELEMENT = "toc"; + public static final String METADATA_IIIF_LABEL_ELEMENT = "label"; + public static final String METADATA_IIIF_HEIGHT_QUALIFIER = "height"; + public static final String METADATA_IIIF_WIDTH_QUALIFIER = "width"; protected static final ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); diff --git a/dspace-iiif/src/main/java/org/dspace/app/iiif/service/utils/IIIFUtils.java b/dspace-iiif/src/main/java/org/dspace/app/iiif/service/utils/IIIFUtils.java index 8889cb5518..cce2b8c6fe 100644 --- a/dspace-iiif/src/main/java/org/dspace/app/iiif/service/utils/IIIFUtils.java +++ b/dspace-iiif/src/main/java/org/dspace/app/iiif/service/utils/IIIFUtils.java @@ -7,10 +7,10 @@ */ package org.dspace.app.iiif.service.utils; -import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_HEIGHT; -import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_IMAGE; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_HEIGHT_QUALIFIER; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_IMAGE_ELEMENT; import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_SCHEMA; -import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_WIDTH; +import static org.dspace.iiif.util.IIIFSharedUtils.METADATA_IIIF_WIDTH_QUALIFIER; import java.sql.SQLException; import java.util.ArrayList; @@ -63,11 +63,11 @@ public class IIIFUtils { // metadata used to set the iiif viewing hint public static final String METADATA_IIIF_VIEWING_HINT = "iiif.viewing.hint"; // metadata used to set the width of the canvas that has not an explicit name - public static final String METADATA_IMAGE_WIDTH = METADATA_IIIF_SCHEMA + "." + METADATA_IIIF_IMAGE - + "." + METADATA_IIIF_WIDTH; + public static final String METADATA_IMAGE_WIDTH = METADATA_IIIF_SCHEMA + "." + METADATA_IIIF_IMAGE_ELEMENT + + "." + METADATA_IIIF_WIDTH_QUALIFIER; // metadata used to set the height of the canvas that has not an explicit name - public static final String METADATA_IMAGE_HEIGHT = METADATA_IIIF_SCHEMA + "." + METADATA_IIIF_IMAGE - + "." + METADATA_IIIF_HEIGHT; + public static final String METADATA_IMAGE_HEIGHT = METADATA_IIIF_SCHEMA + "." + METADATA_IIIF_IMAGE_ELEMENT + + "." + METADATA_IIIF_HEIGHT_QUALIFIER; // string used in the metadata toc as separator among the different levels public static final String TOC_SEPARATOR = "|||"; From 42cae867dbe901131fa3e90b1ea7093ad9cbc002 Mon Sep 17 00:00:00 2001 From: Kevin Van de Velde Date: Wed, 26 Jan 2022 16:02:49 +0100 Subject: [PATCH 17/19] Move the citation generation in a single method again --- .../java/org/dspace/curate/CitationPage.java | 2 +- .../CitationDocumentServiceImpl.java | 32 +++++++++---------- .../service/CitationDocumentService.java | 9 ++---- .../app/rest/BitstreamRestController.java | 6 +++- .../app/rest/utils/BitstreamResource.java | 2 +- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/curate/CitationPage.java b/dspace-api/src/main/java/org/dspace/curate/CitationPage.java index 0e6d4992b1..5d1dca5243 100644 --- a/dspace-api/src/main/java/org/dspace/curate/CitationPage.java +++ b/dspace-api/src/main/java/org/dspace/curate/CitationPage.java @@ -154,7 +154,7 @@ public class CitationPage extends AbstractCurationTask { try { //Create the cited document InputStream citedInputStream = - citationDocument.getCitedDocument(Curator.curationContext(), bitstream); + citationDocument.makeCitedDocument(Curator.curationContext(), bitstream).getLeft(); //Add the cited document to the approiate bundle this.addCitedPageToItem(citedInputStream, bundle, pBundle, dBundle, displayMap, item, bitstream); diff --git a/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java b/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java index e81066071c..d51a3dfc7f 100644 --- a/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/disseminate/CitationDocumentServiceImpl.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Set; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.pdfbox.pdmodel.PDDocument; @@ -296,33 +297,30 @@ public class CitationDocumentServiceImpl implements CitationDocumentService, Ini } @Override - public InputStream getCitedDocument(Context context, Bitstream bitstream) - throws SQLException, AuthorizeException, IOException { - byte [] citedDocumentContent = getCitedDocumentContent(context, bitstream); - return new ByteArrayInputStream(citedDocumentContent); - } - - @Override - public Long getCitedDocumentLength(Context context, Bitstream bitstream) - throws SQLException, AuthorizeException, IOException { - byte[] citedDocumentContent = getCitedDocumentContent(context, bitstream); - return Long.valueOf(citedDocumentContent.length); - } - - private byte[] getCitedDocumentContent(Context context, Bitstream bitstream) - throws SQLException, AuthorizeException, IOException { + public Pair makeCitedDocument(Context context, Bitstream bitstream) + throws IOException, SQLException, AuthorizeException { PDDocument document = new PDDocument(); PDDocument sourceDocument = new PDDocument(); try { Item item = (Item) bitstreamService.getParentObject(context, bitstream); - sourceDocument = sourceDocument.load(bitstreamService.retrieve(context, bitstream)); + final InputStream inputStream = bitstreamService.retrieve(context, bitstream); + try { + sourceDocument = sourceDocument.load(inputStream); + } finally { + inputStream.close(); + } PDPage coverPage = new PDPage(citationPageFormat); generateCoverPage(context, document, coverPage, item); addCoverPageToDocument(document, sourceDocument, coverPage); + + //We already have the full PDF in memory, so keep it there try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { document.save(out); - return out.toByteArray(); + + byte[] data = out.toByteArray(); + return Pair.of(new ByteArrayInputStream(data), Long.valueOf(data.length)); } + } finally { sourceDocument.close(); document.close(); diff --git a/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java b/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java index a116ccf0b1..4a59de3f5f 100644 --- a/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java +++ b/dspace-api/src/main/java/org/dspace/disseminate/service/CitationDocumentService.java @@ -11,6 +11,7 @@ import java.io.IOException; import java.io.InputStream; import java.sql.SQLException; +import org.apache.commons.lang3.tuple.Pair; import org.apache.pdfbox.pdmodel.PDPage; import org.apache.pdfbox.pdmodel.PDPageContentStream; import org.apache.pdfbox.pdmodel.font.PDFont; @@ -83,12 +84,8 @@ public interface CitationDocumentService { * @throws SQLException if database error * @throws AuthorizeException if authorization error */ - public InputStream getCitedDocument(Context context, Bitstream bitstream) - throws SQLException, AuthorizeException, IOException; - - public Long getCitedDocumentLength(Context context, Bitstream bitstream) - throws SQLException, AuthorizeException, IOException; - + public Pair makeCitedDocument(Context context, Bitstream bitstream) + throws IOException, SQLException, AuthorizeException; /** * @param page page diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java index ab227682b4..75d3c9886c 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/BitstreamRestController.java @@ -12,6 +12,7 @@ import static org.dspace.app.rest.utils.RegexUtils.REGEX_REQUESTMAPPING_IDENTIFI import static org.springframework.web.bind.annotation.RequestMethod.PUT; import java.io.IOException; +import java.io.InputStream; import java.sql.SQLException; import java.util.List; import java.util.UUID; @@ -21,6 +22,7 @@ import javax.ws.rs.core.Response; import org.apache.catalina.connector.ClientAbortException; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.tuple.Pair; import org.apache.logging.log4j.Logger; import org.dspace.app.rest.converter.ConverterService; import org.dspace.app.rest.exception.DSpaceBadRequestException; @@ -133,7 +135,9 @@ public class BitstreamRestController { try { long filesize; if (citationDocumentService.isCitationEnabledForBitstream(bit, context)) { - filesize = citationDocumentService.getCitedDocumentLength(context, bit); + final Pair citedDocument = citationDocumentService.makeCitedDocument(context, bit); + filesize = citedDocument.getRight(); + citedDocument.getLeft().close(); } else { filesize = bit.getSizeBytes(); } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java index 57ea2bcffa..5b8c54629c 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/utils/BitstreamResource.java @@ -66,7 +66,7 @@ public class BitstreamResource extends AbstractResource { InputStream out; if (citationDocumentService.isCitationEnabledForBitstream(bitstream, context)) { - out = citationDocumentService.getCitedDocument(context, bitstream); + out = citationDocumentService.makeCitedDocument(context, bitstream).getLeft(); } else { out = bitstreamService.retrieve(context, bitstream); } From e593e90dfb564ef3a963d8afb203b30626346a36 Mon Sep 17 00:00:00 2001 From: Hardy Pottinger Date: Fri, 28 Jan 2022 12:20:13 -0600 Subject: [PATCH 18/19] 8134 - Fix the example requestiem helpdesk strategy example - Add autowire-candidate='true' to the example in requestitem config, as per suggestion from @bbranan. fixes #8134 --- dspace/config/spring/api/requestitem.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dspace/config/spring/api/requestitem.xml b/dspace/config/spring/api/requestitem.xml index 1288753c85..cc18c7916f 100644 --- a/dspace/config/spring/api/requestitem.xml +++ b/dspace/config/spring/api/requestitem.xml @@ -26,6 +26,7 @@ + id="org.dspace.app.requestitem.RequestItemAuthorExtractor" + autowire-candidate='true'>--> From 372ee07b2ff266cdca3bab1ef2d649d05af36ef9 Mon Sep 17 00:00:00 2001 From: Hardy Pottinger Date: Fri, 28 Jan 2022 13:00:32 -0600 Subject: [PATCH 19/19] 8138 Fix SQL/HQL in itemDAOImpl.java --- .../src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java b/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java index fd404e2f3e..c4125696a8 100644 --- a/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/dao/impl/ItemDAOImpl.java @@ -392,7 +392,7 @@ public class ItemDAOImpl extends AbstractHibernateDSODAO implements ItemDA public Iterator findByLastModifiedSince(Context context, Date since) throws SQLException { Query query = createQuery(context, - "SELECT i FROM item i WHERE last_modified > :last_modified ORDER BY id"); + "SELECT i FROM Item i WHERE last_modified > :last_modified ORDER BY id"); query.setParameter("last_modified", since, TemporalType.TIMESTAMP); return iterate(query); }