inputSet = reader.getInputsByCollection(collection);
for (DCInputSet inputs : inputSet) {
for (DCInput[] row : inputs.getFields()) {
for (DCInput input : row) {
diff --git a/dspace-api/src/main/java/org/dspace/curate/Curation.java b/dspace-api/src/main/java/org/dspace/curate/Curation.java
index b3af072a32cd..4d70286e79e0 100644
--- a/dspace-api/src/main/java/org/dspace/curate/Curation.java
+++ b/dspace-api/src/main/java/org/dspace/curate/Curation.java
@@ -152,17 +152,10 @@ private long runQueue(TaskQueue queue, Curator curator) throws SQLException, Aut
super.handler.logInfo("Curating id: " + entry.getObjectId());
}
curator.clear();
- // does entry relate to a DSO or workflow object?
- if (entry.getObjectId().indexOf('/') > 0) {
- for (String taskName : entry.getTaskNames()) {
- curator.addTask(taskName);
- }
- curator.curate(context, entry.getObjectId());
- } else {
- // TODO: Remove this exception once curation tasks are supported by configurable workflow
- // e.g. see https://github.com/DSpace/DSpace/pull/3157
- throw new IllegalArgumentException("curation for workflow items is no longer supported");
+ for (String taskName : entry.getTaskNames()) {
+ curator.addTask(taskName);
}
+ curator.curate(context, entry.getObjectId());
}
queue.release(this.queue, ticket, true);
return ticket;
diff --git a/dspace-api/src/main/java/org/dspace/curate/XmlWorkflowCuratorServiceImpl.java b/dspace-api/src/main/java/org/dspace/curate/XmlWorkflowCuratorServiceImpl.java
index 05c7a8d99930..00e91ee1fb40 100644
--- a/dspace-api/src/main/java/org/dspace/curate/XmlWorkflowCuratorServiceImpl.java
+++ b/dspace-api/src/main/java/org/dspace/curate/XmlWorkflowCuratorServiceImpl.java
@@ -13,6 +13,8 @@
import java.util.ArrayList;
import java.util.List;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.dspace.authorize.AuthorizeException;
import org.dspace.content.Collection;
@@ -30,6 +32,7 @@
import org.dspace.workflow.FlowStep;
import org.dspace.workflow.Task;
import org.dspace.workflow.TaskSet;
+import org.dspace.xmlworkflow.Role;
import org.dspace.xmlworkflow.RoleMembers;
import org.dspace.xmlworkflow.WorkflowConfigurationException;
import org.dspace.xmlworkflow.factory.XmlWorkflowFactory;
@@ -47,14 +50,17 @@
* Manage interactions between curation and workflow. A curation task can be
* attached to a workflow step, to be executed during the step.
*
+ *
+ * NOTE: when run in workflow, curation tasks run with
+ * authorization disabled.
+ *
* @see CurationTaskConfig
* @author mwood
*/
@Service
public class XmlWorkflowCuratorServiceImpl
implements XmlWorkflowCuratorService {
- private static final Logger LOG
- = org.apache.logging.log4j.LogManager.getLogger();
+ private static final Logger LOG = LogManager.getLogger();
@Autowired(required = true)
protected XmlWorkflowFactory workflowFactory;
@@ -97,7 +103,18 @@ public boolean doCuration(Context c, XmlWorkflowItem wfi)
throws AuthorizeException, IOException, SQLException {
Curator curator = new Curator();
curator.setReporter(reporter);
- return curate(curator, c, wfi);
+ c.turnOffAuthorisationSystem();
+ boolean wasAnonymous = false;
+ if (null == c.getCurrentUser()) { // We need someone to email
+ wasAnonymous = true;
+ c.setCurrentUser(ePersonService.getSystemEPerson(c));
+ }
+ boolean failedP = curate(curator, c, wfi);
+ if (wasAnonymous) {
+ c.setCurrentUser(null);
+ }
+ c.restoreAuthSystemState();
+ return failedP;
}
@Override
@@ -123,40 +140,47 @@ public boolean curate(Curator curator, Context c, XmlWorkflowItem wfi)
item.setOwningCollection(wfi.getCollection());
for (Task task : step.tasks) {
curator.addTask(task.name);
- curator.curate(item);
- int status = curator.getStatus(task.name);
- String result = curator.getResult(task.name);
- String action = "none";
- switch (status) {
- case Curator.CURATE_FAIL:
- // task failed - notify any contacts the task has assigned
- if (task.powers.contains("reject")) {
- action = "reject";
- }
- notifyContacts(c, wfi, task, "fail", action, result);
- // if task so empowered, reject submission and terminate
- if ("reject".equals(action)) {
- workflowService.sendWorkflowItemBackSubmission(c, wfi,
- c.getCurrentUser(), null,
- task.name + ": " + result);
- return false;
- }
- break;
- case Curator.CURATE_SUCCESS:
- if (task.powers.contains("approve")) {
- action = "approve";
- }
- notifyContacts(c, wfi, task, "success", action, result);
- if ("approve".equals(action)) {
- // cease further task processing and advance submission
- return true;
- }
- break;
- case Curator.CURATE_ERROR:
- notifyContacts(c, wfi, task, "error", action, result);
- break;
- default:
- break;
+ // Check whether the task is configured to be queued rather than automatically run
+ if (StringUtils.isNotEmpty(step.queue)) {
+ // queue attribute has been set in the FlowStep configuration: add task to configured queue
+ curator.queue(c, item.getID().toString(), step.queue);
+ } else {
+ // Task is configured to be run automatically
+ curator.curate(c, item);
+ int status = curator.getStatus(task.name);
+ String result = curator.getResult(task.name);
+ String action = "none";
+ switch (status) {
+ case Curator.CURATE_FAIL:
+ // task failed - notify any contacts the task has assigned
+ if (task.powers.contains("reject")) {
+ action = "reject";
+ }
+ notifyContacts(c, wfi, task, "fail", action, result);
+ // if task so empowered, reject submission and terminate
+ if ("reject".equals(action)) {
+ workflowService.sendWorkflowItemBackSubmission(c, wfi,
+ c.getCurrentUser(), null,
+ task.name + ": " + result);
+ return false;
+ }
+ break;
+ case Curator.CURATE_SUCCESS:
+ if (task.powers.contains("approve")) {
+ action = "approve";
+ }
+ notifyContacts(c, wfi, task, "success", action, result);
+ if ("approve".equals(action)) {
+ // cease further task processing and advance submission
+ return true;
+ }
+ break;
+ case Curator.CURATE_ERROR:
+ notifyContacts(c, wfi, task, "error", action, result);
+ break;
+ default:
+ break;
+ }
}
curator.clear();
}
@@ -223,8 +247,12 @@ protected void notifyContacts(Context c, XmlWorkflowItem wfi,
String status, String action, String message)
throws AuthorizeException, IOException, SQLException {
List epa = resolveContacts(c, task.getContacts(status), wfi);
- if (epa.size() > 0) {
+ if (!epa.isEmpty()) {
workflowService.notifyOfCuration(c, wfi, epa, task.name, action, message);
+ } else {
+ LOG.warn("No contacts were found for workflow item {}: "
+ + "task {} returned action {} with message {}",
+ wfi.getID(), task.name, action, message);
}
}
@@ -247,8 +275,7 @@ protected List resolveContacts(Context c, List contacts,
// decode contacts
if ("$flowgroup".equals(contact)) {
// special literal for current flowgoup
- ClaimedTask claimedTask = claimedTaskService.findByWorkflowIdAndEPerson(c, wfi, c.getCurrentUser());
- String stepID = claimedTask.getStepID();
+ String stepID = getFlowStep(c, wfi).step;
Step step;
try {
Workflow workflow = workflowFactory.getWorkflow(wfi.getCollection());
@@ -258,19 +285,26 @@ protected List resolveContacts(Context c, List contacts,
String.valueOf(wfi.getID()), e);
return epList;
}
- RoleMembers roleMembers = step.getRole().getMembers(c, wfi);
- for (EPerson ep : roleMembers.getEPersons()) {
- epList.add(ep);
- }
- for (Group group : roleMembers.getGroups()) {
- epList.addAll(group.getMembers());
+ Role role = step.getRole();
+ if (null != role) {
+ RoleMembers roleMembers = role.getMembers(c, wfi);
+ for (EPerson ep : roleMembers.getEPersons()) {
+ epList.add(ep);
+ }
+ for (Group group : roleMembers.getGroups()) {
+ epList.addAll(group.getMembers());
+ }
+ } else {
+ epList.add(ePersonService.getSystemEPerson(c));
}
} else if ("$colladmin".equals(contact)) {
+ // special literal for collection administrators
Group adGroup = wfi.getCollection().getAdministrators();
if (adGroup != null) {
epList.addAll(groupService.allMembers(c, adGroup));
}
} else if ("$siteadmin".equals(contact)) {
+ // special literal for site administrator
EPerson siteEp = ePersonService.findByEmail(c,
configurationService.getProperty("mail.admin"));
if (siteEp != null) {
diff --git a/dspace-api/src/main/java/org/dspace/curate/service/XmlWorkflowCuratorService.java b/dspace-api/src/main/java/org/dspace/curate/service/XmlWorkflowCuratorService.java
index 2ad1eac12904..778b779cfe03 100644
--- a/dspace-api/src/main/java/org/dspace/curate/service/XmlWorkflowCuratorService.java
+++ b/dspace-api/src/main/java/org/dspace/curate/service/XmlWorkflowCuratorService.java
@@ -42,9 +42,9 @@ public boolean needsCuration(Context c, XmlWorkflowItem wfi)
*
* @param c the context
* @param wfi the workflow item
- * @return true if curation was completed or not required,
+ * @return true if curation was completed or not required;
* false if tasks were queued for later completion,
- * or item was rejected
+ * or item was rejected.
* @throws AuthorizeException if authorization error
* @throws IOException if IO error
* @throws SQLException if database error
@@ -58,7 +58,9 @@ public boolean doCuration(Context c, XmlWorkflowItem wfi)
* @param curator the curation context
* @param c the user context
* @param wfId the workflow item's ID
- * @return true if curation failed.
+ * @return true if curation curation was completed or not required;
+ * false if tasks were queued for later completion,
+ * or item was rejected.
* @throws AuthorizeException if authorization error
* @throws IOException if IO error
* @throws SQLException if database error
@@ -72,7 +74,9 @@ public boolean curate(Curator curator, Context c, String wfId)
* @param curator the curation context
* @param c the user context
* @param wfi the workflow item
- * @return true if curation failed.
+ * @return true if workflow curation was completed or not required;
+ * false if tasks were queued for later completion,
+ * or item was rejected.
* @throws AuthorizeException if authorization error
* @throws IOException if IO error
* @throws SQLException if database error
diff --git a/dspace-api/src/main/java/org/dspace/discovery/IndexClient.java b/dspace-api/src/main/java/org/dspace/discovery/IndexClient.java
index 661c48d91cfc..b70e9162f7a1 100644
--- a/dspace-api/src/main/java/org/dspace/discovery/IndexClient.java
+++ b/dspace-api/src/main/java/org/dspace/discovery/IndexClient.java
@@ -7,14 +7,20 @@
*/
package org.dspace.discovery;
+import static org.dspace.discovery.IndexClientOptions.TYPE_OPTION;
+
import java.io.IOException;
import java.sql.SQLException;
+import java.util.Arrays;
import java.util.Iterator;
+import java.util.List;
import java.util.Optional;
import java.util.UUID;
+import java.util.stream.Collectors;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.ParseException;
+import org.apache.commons.lang3.StringUtils;
import org.dspace.content.Collection;
import org.dspace.content.Community;
import org.dspace.content.DSpaceObject;
@@ -51,6 +57,17 @@ public void internalRun() throws Exception {
return;
}
+ String type = null;
+ if (commandLine.hasOption(TYPE_OPTION)) {
+ List indexableObjectTypes = IndexObjectFactoryFactory.getInstance().getIndexFactories().stream()
+ .map((indexFactory -> indexFactory.getType())).collect(Collectors.toList());
+ type = commandLine.getOptionValue(TYPE_OPTION);
+ if (!indexableObjectTypes.contains(type)) {
+ handler.handleException(String.format("%s is not a valid indexable object type, options: %s",
+ type, Arrays.toString(indexableObjectTypes.toArray())));
+ }
+ }
+
/** Acquire from dspace-services in future */
/**
* new DSpace.getServiceManager().getServiceByName("org.dspace.discovery.SolrIndexer");
@@ -113,6 +130,10 @@ public void internalRun() throws Exception {
} else if (indexClientOptions == IndexClientOptions.BUILD ||
indexClientOptions == IndexClientOptions.BUILDANDSPELLCHECK) {
handler.logInfo("(Re)building index from scratch.");
+ if (StringUtils.isNotBlank(type)) {
+ handler.logWarning(String.format("Type option, %s, not applicable for entire index rebuild option, b" +
+ ", type will be ignored", TYPE_OPTION));
+ }
indexer.deleteIndex();
indexer.createIndex(context);
if (indexClientOptions == IndexClientOptions.BUILDANDSPELLCHECK) {
@@ -133,14 +154,14 @@ public void internalRun() throws Exception {
} else if (indexClientOptions == IndexClientOptions.UPDATE ||
indexClientOptions == IndexClientOptions.UPDATEANDSPELLCHECK) {
handler.logInfo("Updating Index");
- indexer.updateIndex(context, false);
+ indexer.updateIndex(context, false, type);
if (indexClientOptions == IndexClientOptions.UPDATEANDSPELLCHECK) {
checkRebuildSpellCheck(commandLine, indexer);
}
} else if (indexClientOptions == IndexClientOptions.FORCEUPDATE ||
indexClientOptions == IndexClientOptions.FORCEUPDATEANDSPELLCHECK) {
handler.logInfo("Updating Index");
- indexer.updateIndex(context, true);
+ indexer.updateIndex(context, true, type);
if (indexClientOptions == IndexClientOptions.FORCEUPDATEANDSPELLCHECK) {
checkRebuildSpellCheck(commandLine, indexer);
}
diff --git a/dspace-api/src/main/java/org/dspace/discovery/IndexClientOptions.java b/dspace-api/src/main/java/org/dspace/discovery/IndexClientOptions.java
index 74d9ba0c3a56..0de5b22d0655 100644
--- a/dspace-api/src/main/java/org/dspace/discovery/IndexClientOptions.java
+++ b/dspace-api/src/main/java/org/dspace/discovery/IndexClientOptions.java
@@ -8,8 +8,13 @@
package org.dspace.discovery;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Options;
+import org.dspace.discovery.indexobject.factory.IndexObjectFactoryFactory;
/**
* This Enum holds all the possible options and combinations for the Index discovery script
@@ -29,6 +34,8 @@ public enum IndexClientOptions {
FORCEUPDATEANDSPELLCHECK,
HELP;
+ public static final String TYPE_OPTION = "t";
+
/**
* This method resolves the CommandLine parameters to figure out which action the index-discovery script should
* perform
@@ -71,11 +78,15 @@ protected static IndexClientOptions getIndexClientOption(CommandLine commandLine
protected static Options constructOptions() {
Options options = new Options();
+ List indexableObjectTypes = IndexObjectFactoryFactory.getInstance().getIndexFactories().stream()
+ .map((indexFactory -> indexFactory.getType())).collect(Collectors.toList());
options
.addOption("r", "remove", true, "remove an Item, Collection or Community from index based on its handle");
options.addOption("i", "index", true,
"add or update an Item, Collection or Community based on its handle or uuid");
+ options.addOption(TYPE_OPTION, "type", true, "reindex only specific type of " +
+ "(re)indexable objects; options: " + Arrays.toString(indexableObjectTypes.toArray()));
options.addOption("c", "clean", false,
"clean existing index removing any documents that no longer exist in the db");
options.addOption("d", "delete", false,
diff --git a/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java b/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java
index 4ff1f3134484..80602ac80459 100644
--- a/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java
+++ b/dspace-api/src/main/java/org/dspace/discovery/IndexEventConsumer.java
@@ -154,7 +154,11 @@ public void consume(Context ctx, Event event) throws Exception {
case Event.REMOVE:
case Event.ADD:
- if (object == null) {
+ // At this time, ADD and REMOVE actions are ignored on SITE object. They are only triggered for
+ // top-level communities. No action is necessary as Community itself is indexed (or deleted) separately.
+ if (event.getSubjectType() == Constants.SITE) {
+ log.debug(event.getEventTypeAsString() + " event triggered for Site object. Skipping it.");
+ } else if (object == null) {
log.warn(event.getEventTypeAsString() + " event, could not get object for "
+ event.getObjectTypeAsString() + " id="
+ event.getObjectID()
@@ -201,6 +205,10 @@ public void consume(Context ctx, Event event) throws Exception {
@Override
public void end(Context ctx) throws Exception {
+ // Change the mode to readonly to improve performance
+ Context.Mode originalMode = ctx.getCurrentMode();
+ ctx.setMode(Context.Mode.READ_ONLY);
+
try {
for (String uid : uniqueIdsToDelete) {
try {
@@ -230,6 +238,8 @@ public void end(Context ctx) throws Exception {
uniqueIdsToDelete.clear();
createdItemsToUpdate.clear();
}
+
+ ctx.setMode(originalMode);
}
}
diff --git a/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java b/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java
index 0cf2aa50af67..cd3797e3e34e 100644
--- a/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java
+++ b/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java
@@ -1031,9 +1031,8 @@ protected DiscoverResult retrieveResult(Context context, DiscoverQuery query)
// Add information about our search fields
for (String field : searchFields) {
List valuesAsString = new ArrayList<>();
- for (Object o : doc.getFieldValues(field)) {
- valuesAsString.add(String.valueOf(o));
- }
+ Optional.ofNullable(doc.getFieldValues(field))
+ .ifPresent(l -> l.forEach(o -> valuesAsString.add(String.valueOf(o))));
resultDoc.addSearchField(field, valuesAsString.toArray(new String[valuesAsString.size()]));
}
result.addSearchDocument(indexableObject, resultDoc);
diff --git a/dspace-api/src/main/java/org/dspace/discovery/indexobject/IndexFactoryImpl.java b/dspace-api/src/main/java/org/dspace/discovery/indexobject/IndexFactoryImpl.java
index 55c99b168e7a..f1ae137b9163 100644
--- a/dspace-api/src/main/java/org/dspace/discovery/indexobject/IndexFactoryImpl.java
+++ b/dspace-api/src/main/java/org/dspace/discovery/indexobject/IndexFactoryImpl.java
@@ -64,7 +64,14 @@ public SolrInputDocument buildDocument(Context context, T indexableObject) throw
//Do any additional indexing, depends on the plugins
for (SolrServiceIndexPlugin solrServiceIndexPlugin : ListUtils.emptyIfNull(solrServiceIndexPlugins)) {
- solrServiceIndexPlugin.additionalIndex(context, indexableObject, doc);
+ try {
+ solrServiceIndexPlugin.additionalIndex(context, indexableObject, doc);
+ } catch (Exception e) {
+ log.error("An error occurred while indexing additional fields. " +
+ "Could not fully index item with UUID: {}. Plugin: {}",
+ indexableObject.getUniqueIndexID(), solrServiceIndexPlugin.getClass().getSimpleName());
+
+ }
}
return doc;
@@ -82,7 +89,7 @@ public void writeDocument(Context context, T indexableObject, SolrInputDocument
writeDocument(solrInputDocument, null);
} catch (Exception e) {
log.error("Error occurred while writing SOLR document for {} object {}",
- indexableObject.getType(), indexableObject.getID(), e);
+ indexableObject.getType(), indexableObject.getID(), e);
}
}
@@ -101,8 +108,8 @@ protected void writeDocument(SolrInputDocument doc, FullTextContentStreams strea
if (streams != null && !streams.isEmpty()) {
// limit full text indexing to first 100,000 characters unless configured otherwise
final int charLimit = DSpaceServicesFactory.getInstance().getConfigurationService()
- .getIntProperty("discovery.solr.fulltext.charLimit",
- 100000);
+ .getIntProperty("discovery.solr.fulltext.charLimit",
+ 100000);
// Use Tika's Text parser as the streams are always from the TEXT bundle (i.e. already extracted text)
TextAndCSVParser tikaParser = new TextAndCSVParser();
@@ -113,6 +120,18 @@ protected void writeDocument(SolrInputDocument doc, FullTextContentStreams strea
// Use Apache Tika to parse the full text stream(s)
try (InputStream fullTextStreams = streams.getStream()) {
tikaParser.parse(fullTextStreams, tikaHandler, tikaMetadata, tikaContext);
+
+ // Write Tika metadata to "tika_meta_*" fields.
+ // This metadata is not very useful right now,
+ // but we'll keep it just in case it becomes more useful.
+ for (String name : tikaMetadata.names()) {
+ for (String value : tikaMetadata.getValues(name)) {
+ doc.addField("tika_meta_" + name, value);
+ }
+ }
+
+ // Save (parsed) full text to "fulltext" field
+ doc.addField("fulltext", tikaHandler.toString());
} catch (SAXException saxe) {
// Check if this SAXException is just a notice that this file was longer than the character limit.
// Unfortunately there is not a unique, public exception type to catch here. This error is thrown
@@ -121,30 +140,23 @@ protected void writeDocument(SolrInputDocument doc, FullTextContentStreams strea
if (saxe.getMessage().contains("limit has been reached")) {
// log that we only indexed up to that configured limit
log.info("Full text is larger than the configured limit (discovery.solr.fulltext.charLimit)."
- + " Only the first {} characters were indexed.", charLimit);
+ + " Only the first {} characters were indexed.", charLimit);
} else {
log.error("Tika parsing error. Could not index full text.", saxe);
throw new IOException("Tika parsing error. Could not index full text.", saxe);
}
- } catch (TikaException ex) {
+ } catch (TikaException | IOException ex) {
log.error("Tika parsing error. Could not index full text.", ex);
throw new IOException("Tika parsing error. Could not index full text.", ex);
+ } finally {
+ // Add document to index
+ solr.add(doc);
}
-
- // Write Tika metadata to "tika_meta_*" fields.
- // This metadata is not very useful right now, but we'll keep it just in case it becomes more useful.
- for (String name : tikaMetadata.names()) {
- for (String value : tikaMetadata.getValues(name)) {
- doc.addField("tika_meta_" + name, value);
- }
- }
-
- // Save (parsed) full text to "fulltext" field
- doc.addField("fulltext", tikaHandler.toString());
+ return;
}
-
// Add document to index
solr.add(doc);
+
}
}
diff --git a/dspace-api/src/main/java/org/dspace/eperson/EPersonServiceImpl.java b/dspace-api/src/main/java/org/dspace/eperson/EPersonServiceImpl.java
index 61477995c7ed..b9ac740685bd 100644
--- a/dspace-api/src/main/java/org/dspace/eperson/EPersonServiceImpl.java
+++ b/dspace-api/src/main/java/org/dspace/eperson/EPersonServiceImpl.java
@@ -33,6 +33,7 @@
import org.dspace.content.Item;
import org.dspace.content.MetadataField;
import org.dspace.content.MetadataValue;
+import org.dspace.content.QAEventProcessed;
import org.dspace.content.WorkspaceItem;
import org.dspace.content.factory.ContentServiceFactory;
import org.dspace.content.service.ItemService;
@@ -47,6 +48,8 @@
import org.dspace.eperson.service.SubscribeService;
import org.dspace.event.Event;
import org.dspace.orcid.service.OrcidTokenService;
+import org.dspace.qaevent.dao.QAEventsDAO;
+import org.dspace.services.ConfigurationService;
import org.dspace.util.UUIDUtils;
import org.dspace.versioning.Version;
import org.dspace.versioning.VersionHistory;
@@ -101,8 +104,12 @@ public class EPersonServiceImpl extends DSpaceObjectServiceImpl impleme
protected VersionDAO versionDAO;
@Autowired(required = true)
protected ClaimedTaskService claimedTaskService;
+ @Autowired(required = true)
+ protected ConfigurationService configurationService;
@Autowired
protected OrcidTokenService orcidTokenService;
+ @Autowired
+ protected QAEventsDAO qaEventsDao;
protected EPersonServiceImpl() {
super();
@@ -113,12 +120,41 @@ public EPerson find(Context context, UUID id) throws SQLException {
return ePersonDAO.findByID(context, EPerson.class, id);
}
+ /**
+ * Create a fake EPerson which can receive email. Its address will be the
+ * value of "mail.admin", or "postmaster" if all else fails.
+ * @param c
+ * @return
+ * @throws SQLException
+ */
+ @Override
+ public EPerson getSystemEPerson(Context c)
+ throws SQLException {
+ String adminEmail = configurationService.getProperty("mail.admin");
+ if (null == adminEmail) {
+ adminEmail = "postmaster"; // Last-ditch attempt to send *somewhere*
+ }
+ EPerson systemEPerson = findByEmail(c, adminEmail);
+
+ if (null == systemEPerson) {
+ systemEPerson = new EPerson();
+ systemEPerson.setEmail(adminEmail);
+ }
+
+ return systemEPerson;
+ }
+
@Override
public EPerson findByIdOrLegacyId(Context context, String id) throws SQLException {
- if (StringUtils.isNumeric(id)) {
- return findByLegacyId(context, Integer.parseInt(id));
- } else {
- return find(context, UUID.fromString(id));
+ try {
+ if (StringUtils.isNumeric(id)) {
+ return findByLegacyId(context, Integer.parseInt(id));
+ } else {
+ return find(context, UUID.fromString(id));
+ }
+ } catch (IllegalArgumentException e) {
+ // Not a valid legacy ID or valid UUID
+ return null;
}
}
@@ -157,32 +193,98 @@ public List search(Context context, String query) throws SQLException {
@Override
public List search(Context context, String query, int offset, int limit) throws SQLException {
- try {
- List ePerson = new ArrayList<>();
- EPerson person = find(context, UUID.fromString(query));
+ List ePersons = new ArrayList<>();
+ UUID uuid = UUIDUtils.fromString(query);
+ if (uuid == null) {
+ // Search by firstname & lastname (NOTE: email will also be included automatically)
+ MetadataField firstNameField = metadataFieldService.findByElement(context, "eperson", "firstname", null);
+ MetadataField lastNameField = metadataFieldService.findByElement(context, "eperson", "lastname", null);
+ if (StringUtils.isBlank(query)) {
+ query = null;
+ }
+ ePersons = ePersonDAO.search(context, query, Arrays.asList(firstNameField, lastNameField),
+ Arrays.asList(firstNameField, lastNameField), offset, limit);
+ } else {
+ // Search by UUID
+ EPerson person = find(context, uuid);
if (person != null) {
- ePerson.add(person);
+ ePersons.add(person);
}
- return ePerson;
- } catch (IllegalArgumentException e) {
+ }
+ return ePersons;
+ }
+
+ @Override
+ public int searchResultCount(Context context, String query) throws SQLException {
+ int result = 0;
+ UUID uuid = UUIDUtils.fromString(query);
+ if (uuid == null) {
+ // Count results found by firstname & lastname (email is also included automatically)
+ MetadataField firstNameField = metadataFieldService.findByElement(context, "eperson", "firstname", null);
+ MetadataField lastNameField = metadataFieldService.findByElement(context, "eperson", "lastname", null);
+ if (StringUtils.isBlank(query)) {
+ query = null;
+ }
+ result = ePersonDAO.searchResultCount(context, query, Arrays.asList(firstNameField, lastNameField));
+ } else {
+ // Search by UUID
+ EPerson person = find(context, uuid);
+ if (person != null) {
+ result = 1;
+ }
+ }
+ return result;
+ }
+
+ @Override
+ public List searchNonMembers(Context context, String query, Group excludeGroup, int offset, int limit)
+ throws SQLException {
+ List ePersons = new ArrayList<>();
+ UUID uuid = UUIDUtils.fromString(query);
+ if (uuid == null) {
+ // Search by firstname & lastname (NOTE: email will also be included automatically)
MetadataField firstNameField = metadataFieldService.findByElement(context, "eperson", "firstname", null);
MetadataField lastNameField = metadataFieldService.findByElement(context, "eperson", "lastname", null);
if (StringUtils.isBlank(query)) {
query = null;
}
- return ePersonDAO.search(context, query, Arrays.asList(firstNameField, lastNameField),
- Arrays.asList(firstNameField, lastNameField), offset, limit);
+ ePersons = ePersonDAO.searchNotMember(context, query, Arrays.asList(firstNameField, lastNameField),
+ excludeGroup, Arrays.asList(firstNameField, lastNameField),
+ offset, limit);
+ } else {
+ // Search by UUID
+ EPerson person = find(context, uuid);
+ // Verify EPerson is NOT a member of the given excludeGroup before adding
+ if (person != null && !groupService.isDirectMember(excludeGroup, person)) {
+ ePersons.add(person);
+ }
}
+
+ return ePersons;
}
@Override
- public int searchResultCount(Context context, String query) throws SQLException {
- MetadataField firstNameField = metadataFieldService.findByElement(context, "eperson", "firstname", null);
- MetadataField lastNameField = metadataFieldService.findByElement(context, "eperson", "lastname", null);
- if (StringUtils.isBlank(query)) {
- query = null;
+ public int searchNonMembersCount(Context context, String query, Group excludeGroup) throws SQLException {
+ int result = 0;
+ UUID uuid = UUIDUtils.fromString(query);
+ if (uuid == null) {
+ // Count results found by firstname & lastname (email is also included automatically)
+ MetadataField firstNameField = metadataFieldService.findByElement(context, "eperson", "firstname", null);
+ MetadataField lastNameField = metadataFieldService.findByElement(context, "eperson", "lastname", null);
+ if (StringUtils.isBlank(query)) {
+ query = null;
+ }
+ result = ePersonDAO.searchNotMemberCount(context, query, Arrays.asList(firstNameField, lastNameField),
+ excludeGroup);
+ } else {
+ // Search by UUID
+ EPerson person = find(context, uuid);
+ // Verify EPerson is NOT a member of the given excludeGroup before counting
+ if (person != null && !groupService.isDirectMember(excludeGroup, person)) {
+ result = 1;
+ }
}
- return ePersonDAO.searchResultCount(context, query, Arrays.asList(firstNameField, lastNameField));
+ return result;
}
@Override
@@ -278,10 +380,13 @@ public void delete(Context context, EPerson ePerson, boolean cascade)
throw new AuthorizeException(
"You must be an admin to delete an EPerson");
}
+ // Get all workflow-related groups that the current EPerson belongs to
Set workFlowGroups = getAllWorkFlowGroups(context, ePerson);
for (Group group: workFlowGroups) {
- List ePeople = groupService.allMembers(context, group);
- if (ePeople.size() == 1 && ePeople.contains(ePerson)) {
+ // Get total number of unique EPerson objs who are a member of this group (or subgroup)
+ int totalMembers = groupService.countAllMembers(context, group);
+ // If only one EPerson is a member, then we cannot delete the last member of this group.
+ if (totalMembers == 1) {
throw new EmptyWorkflowGroupException(ePerson.getID(), group.getID());
}
}
@@ -391,6 +496,11 @@ public void delete(Context context, EPerson ePerson, boolean cascade)
// Remove any subscriptions
subscribeService.deleteByEPerson(context, ePerson);
+ List qaEvents = qaEventsDao.findByEPerson(context, ePerson);
+ for (QAEventProcessed qaEvent : qaEvents) {
+ qaEventsDao.delete(context, qaEvent);
+ }
+
// Remove ourself
ePersonDAO.delete(context, ePerson);
@@ -540,14 +650,29 @@ public List getDeleteConstraints(Context context, EPerson ePerson) throw
@Override
public List findByGroups(Context c, Set groups) throws SQLException {
+ return findByGroups(c, groups, -1, -1);
+ }
+
+ @Override
+ public List findByGroups(Context c, Set groups, int pageSize, int offset) throws SQLException {
//Make sure we at least have one group, if not don't even bother searching.
if (CollectionUtils.isNotEmpty(groups)) {
- return ePersonDAO.findByGroups(c, groups);
+ return ePersonDAO.findByGroups(c, groups, pageSize, offset);
} else {
return new ArrayList<>();
}
}
+ @Override
+ public int countByGroups(Context c, Set groups) throws SQLException {
+ //Make sure we at least have one group, if not don't even bother counting.
+ if (CollectionUtils.isNotEmpty(groups)) {
+ return ePersonDAO.countByGroups(c, groups);
+ } else {
+ return 0;
+ }
+ }
+
@Override
public List findEPeopleWithSubscription(Context context) throws SQLException {
return ePersonDAO.findAllSubscribers(context);
diff --git a/dspace-api/src/main/java/org/dspace/eperson/Group.java b/dspace-api/src/main/java/org/dspace/eperson/Group.java
index 6cb534146b25..67655e0e0aaf 100644
--- a/dspace-api/src/main/java/org/dspace/eperson/Group.java
+++ b/dspace-api/src/main/java/org/dspace/eperson/Group.java
@@ -98,7 +98,11 @@ void addMember(EPerson e) {
}
/**
- * Return EPerson members of a Group
+ * Return EPerson members of a Group.
+ *
+ * WARNING: This method may have bad performance for Groups with large numbers of EPerson members.
+ * Therefore, only use this when you need to access every EPerson member. Instead, consider using
+ * EPersonService.findByGroups() for a paginated list of EPersons.
*
* @return list of EPersons
*/
@@ -143,9 +147,13 @@ List getParentGroups() {
}
/**
- * Return Group members of a Group.
+ * Return Group members (i.e. direct subgroups) of a Group.
+ *
+ * WARNING: This method may have bad performance for Groups with large numbers of Subgroups.
+ * Therefore, only use this when you need to access every Subgroup. Instead, consider using
+ * GroupService.findByParent() for a paginated list of Subgroups.
*
- * @return list of groups
+ * @return list of subgroups
*/
public List getMemberGroups() {
return groups;
diff --git a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java
index 607e57af0b2c..730053e42ce2 100644
--- a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java
+++ b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java
@@ -179,8 +179,13 @@ public void removeMember(Context context, Group group, EPerson ePerson) throws S
for (CollectionRole collectionRole : collectionRoles) {
if (StringUtils.equals(collectionRole.getRoleId(), role.getId())
&& claimedTask.getWorkflowItem().getCollection() == collectionRole.getCollection()) {
- List ePeople = allMembers(context, group);
- if (ePeople.size() == 1 && ePeople.contains(ePerson)) {
+ // Count number of EPersons who are *direct* members of this group
+ int totalDirectEPersons = ePersonService.countByGroups(context, Set.of(group));
+ // Count number of Groups which have this groupParent as a direct parent
+ int totalChildGroups = countByParent(context, group);
+ // If this group has only one direct EPerson and *zero* child groups, then we cannot delete the
+ // EPerson or we will leave this group empty.
+ if (totalDirectEPersons == 1 && totalChildGroups == 0) {
throw new IllegalStateException(
"Refused to remove user " + ePerson
.getID() + " from workflow group because the group " + group
@@ -191,8 +196,13 @@ public void removeMember(Context context, Group group, EPerson ePerson) throws S
}
}
if (!poolTasks.isEmpty()) {
- List ePeople = allMembers(context, group);
- if (ePeople.size() == 1 && ePeople.contains(ePerson)) {
+ // Count number of EPersons who are *direct* members of this group
+ int totalDirectEPersons = ePersonService.countByGroups(context, Set.of(group));
+ // Count number of Groups which have this groupParent as a direct parent
+ int totalChildGroups = countByParent(context, group);
+ // If this group has only one direct EPerson and *zero* child groups, then we cannot delete the
+ // EPerson or we will leave this group empty.
+ if (totalDirectEPersons == 1 && totalChildGroups == 0) {
throw new IllegalStateException(
"Refused to remove user " + ePerson
.getID() + " from workflow group because the group " + group
@@ -212,9 +222,13 @@ public void removeMember(Context context, Group groupParent, Group childGroup) t
if (!collectionRoles.isEmpty()) {
List poolTasks = poolTaskService.findByGroup(context, groupParent);
if (!poolTasks.isEmpty()) {
- List parentPeople = allMembers(context, groupParent);
- List childPeople = allMembers(context, childGroup);
- if (childPeople.containsAll(parentPeople)) {
+ // Count number of Groups which have this groupParent as a direct parent
+ int totalChildGroups = countByParent(context, groupParent);
+ // Count number of EPersons who are *direct* members of this group
+ int totalDirectEPersons = ePersonService.countByGroups(context, Set.of(groupParent));
+ // If this group has only one childGroup and *zero* direct EPersons, then we cannot delete the
+ // childGroup or we will leave this group empty.
+ if (totalChildGroups == 1 && totalDirectEPersons == 0) {
throw new IllegalStateException(
"Refused to remove sub group " + childGroup
.getID() + " from workflow group because the group " + groupParent
@@ -368,7 +382,8 @@ public List allMembers(Context c, Group g) throws SQLException {
// Get all groups which are a member of this group
List group2GroupCaches = group2GroupCacheDAO.findByParent(c, g);
- Set groups = new HashSet<>();
+ // Initialize HashSet based on List size to avoid Set resizing. See https://stackoverflow.com/a/21822273
+ Set groups = new HashSet<>((int) (group2GroupCaches.size() / 0.75 + 1));
for (Group2GroupCache group2GroupCache : group2GroupCaches) {
groups.add(group2GroupCache.getChild());
}
@@ -381,6 +396,23 @@ public List allMembers(Context c, Group g) throws SQLException {
return new ArrayList<>(childGroupChildren);
}
+ @Override
+ public int countAllMembers(Context context, Group group) throws SQLException {
+ // Get all groups which are a member of this group
+ List group2GroupCaches = group2GroupCacheDAO.findByParent(context, group);
+ // Initialize HashSet based on List size + current 'group' to avoid Set resizing.
+ // See https://stackoverflow.com/a/21822273
+ Set