From d585b3a9f1a34e9e4e6240a803114d2ce9e365da Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Wed, 4 Oct 2017 15:19:57 -0400 Subject: [PATCH 01/97] [DS-2670] First attempt --- .../general/CreateMissingIdentifiers.java | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java diff --git a/dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java b/dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java new file mode 100644 index 0000000000..e5afd3afe1 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java @@ -0,0 +1,78 @@ +/** + * 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.ctask.general; + +import java.io.IOException; +import java.sql.SQLException; +import org.dspace.authorize.AuthorizeException; +import org.dspace.content.DSpaceObject; +import org.dspace.content.Item; +import org.dspace.core.Constants; +import org.dspace.core.Context; +import org.dspace.curate.AbstractCurationTask; +import org.dspace.curate.Curator; +import org.dspace.identifier.IdentifierException; +import org.dspace.identifier.service.IdentifierService; +import org.dspace.utils.DSpace; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Ensure that an object has all of the identifiers that it should, minting them + * as necessary. + * + * @author Mark H. Wood {@literal } + */ +public class CreateMissingIdentifiers + extends AbstractCurationTask +{ + private static final Logger LOG + = LoggerFactory.getLogger(CreateMissingIdentifiers.class); + + @Override + public int perform(DSpaceObject dso) + throws IOException + { + // Only some kinds of model objects get identifiers + if (!(dso instanceof Item)) + return Curator.CURATE_SKIP; + + String typeText = Constants.typeText[dso.getType()]; + + // Get a Context + Context context; + try { + context = Curator.curationContext(); + } catch (SQLException ex) { + report("Could not get the curation Context: " + ex.getMessage()); + return Curator.CURATE_ERROR; + } + + // Find the IdentifierService implementation + IdentifierService identifierService = new DSpace() + .getServiceManager() + .getServiceByName(null, IdentifierService.class); + + // Register any missing identifiers. + try { + identifierService.register(context, dso); + } catch (AuthorizeException | IdentifierException | SQLException ex) { + String message = ex.getMessage(); + report(String.format("Identifier(s) not minted for %s %s: %s%n", + typeText, dso.getID().toString(), message)); + LOG.error("Identifier(s) not minted: {}", message); + return Curator.CURATE_ERROR; + } + + // Success! + report(String.format("%s %s registered.%n", + typeText, dso.getID().toString())); + return Curator.CURATE_SUCCESS; + } +} From d6a8002a7cefea503c607e24614d4af45a8910a6 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Thu, 12 Jul 2018 11:46:27 -0400 Subject: [PATCH 02/97] [DS-3951] Change request-copy recipient to a List; add strategies to list collection administrators and to combine other strategies. --- ...tionAdministratorsRequestItemStrategy.java | 46 +++++++++++++++ .../CombiningRequestItemStrategy.java | 56 +++++++++++++++++++ .../dspace/app/requestitem/RequestItem.java | 18 +++++- .../app/requestitem/RequestItemAuthor.java | 32 ++++++++++- .../RequestItemAuthorExtractor.java | 3 +- .../RequestItemHelpdeskStrategy.java | 24 ++++---- .../RequestItemMetadataStrategy.java | 38 +++++++------ .../RequestItemSubmitterStrategy.java | 8 ++- dspace/config/spring/api/requestitem.xml | 47 ++++++++++++---- 9 files changed, 229 insertions(+), 43 deletions(-) create mode 100644 dspace-api/src/main/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategy.java create mode 100644 dspace-api/src/main/java/org/dspace/app/requestitem/CombiningRequestItemStrategy.java diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategy.java b/dspace-api/src/main/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategy.java new file mode 100644 index 0000000000..0871983969 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategy.java @@ -0,0 +1,46 @@ +/** + * 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.requestitem; + +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; + +import org.dspace.content.Collection; +import org.dspace.content.Item; +import org.dspace.core.Context; +import org.dspace.eperson.EPerson; + +/** + * Derive request recipients from groups of all collections which hold an Item. + * The list will include all members of the administrators group. If the + * resulting list is empty, delegates to {@link RequestItemHelpdeskStrategy}. + * + * @author Mark H. Wood + */ +public class CollectionAdministratorsRequestItemStrategy + implements RequestItemAuthorExtractor { + @Override + public List getRequestItemAuthor(Context context, + Item item) + throws SQLException { + List recipients = new ArrayList<>(); + for (Collection collection : item.getCollections()) { + for (EPerson admin : collection.getAdministrators().getMembers()) { + recipients.add(new RequestItemAuthor(admin)); + } + } + if (recipients.isEmpty()) { + return new RequestItemHelpdeskStrategy() + .getRequestItemAuthor(context, item); + } else { + return recipients; + } + } +} diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/CombiningRequestItemStrategy.java b/dspace-api/src/main/java/org/dspace/app/requestitem/CombiningRequestItemStrategy.java new file mode 100644 index 0000000000..7623383d50 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/CombiningRequestItemStrategy.java @@ -0,0 +1,56 @@ +/** + * 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.requestitem; + +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import javax.inject.Named; + +import org.dspace.content.Item; +import org.dspace.core.Context; + +/** + * Assemble a list of recipients from the results of other strategies. + * The list of strategy classes is injected as the property {@code strategies}. + * If the property is not configured, returns an empty List. + * + * @author Mark H. Wood + */ +@Named +public class CombiningRequestItemStrategy + implements RequestItemAuthorExtractor { + /** The strategies to combine. */ + private final List strategies; + + public CombiningRequestItemStrategy(List strategies) { + this.strategies = strategies; + } + + /** + * Do not call. + * @throws IllegalArgumentException always + */ + private CombiningRequestItemStrategy() { + throw new IllegalArgumentException(); + } + + @Override + public List getRequestItemAuthor(Context context, Item item) + throws SQLException { + List recipients = new ArrayList<>(); + + if (null != strategies) { + for (RequestItemAuthorExtractor strategy : strategies) { + recipients.addAll(strategy.getRequestItemAuthor(context, item)); + } + } + + return recipients; + } +} diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItem.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItem.java index d96cbbb5a4..d86b5503d6 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItem.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItem.java @@ -27,7 +27,7 @@ import org.dspace.core.Context; import org.dspace.core.ReloadableEntity; /** - * Object representing an Item Request + * Object representing an Item Request. */ @Entity @Table(name = "requestitem") @@ -88,6 +88,7 @@ public class RequestItem implements ReloadableEntity { protected RequestItem() { } + @Override public Integer getID() { return requestitem_id; } @@ -96,6 +97,9 @@ public class RequestItem implements ReloadableEntity { this.allfiles = allfiles; } + /** + * @return {@code true} if all of the Item's files are requested. + */ public boolean isAllfiles() { return allfiles; } @@ -104,6 +108,9 @@ public class RequestItem implements ReloadableEntity { this.reqMessage = reqMessage; } + /** + * @return a message from the requester. + */ public String getReqMessage() { return reqMessage; } @@ -112,6 +119,9 @@ public class RequestItem implements ReloadableEntity { this.reqName = reqName; } + /** + * @return Human-readable name of the user requesting access. + */ public String getReqName() { return reqName; } @@ -120,6 +130,9 @@ public class RequestItem implements ReloadableEntity { this.reqEmail = reqEmail; } + /** + * @return address of the user requesting access. + */ public String getReqEmail() { return reqEmail; } @@ -128,6 +141,9 @@ public class RequestItem implements ReloadableEntity { this.token = token; } + /** + * @return a unique request identifier which can be emailed. + */ public String getToken() { return token; } diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthor.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthor.java index 49e26fe00b..9731a96ae6 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthor.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthor.java @@ -7,6 +7,9 @@ */ package org.dspace.app.requestitem; +import java.util.ArrayList; +import java.util.List; + import org.dspace.eperson.EPerson; /** @@ -16,8 +19,8 @@ import org.dspace.eperson.EPerson; * @author Andrea Bollini */ public class RequestItemAuthor { - private String fullName; - private String email; + private final String fullName; + private final String email; public RequestItemAuthor(String fullName, String email) { super(); @@ -38,4 +41,29 @@ public class RequestItemAuthor { public String getFullName() { return fullName; } + /** + * Build a comma-list of addresses from a list of request recipients. + * @param recipients those to receive the request. + * @return addresses of the recipients, separated by ", ". + */ + public static String listAddresses(List recipients) { + List addresses = new ArrayList(recipients.size()); + for (RequestItemAuthor recipient : recipients) { + addresses.add(recipient.getEmail()); + } + return String.join(", ", addresses); + } + + /** + * Build a comma-list of full names from a list of request recipients. + * @param recipients those to receive the request. + * @return names of the recipients, separated by ", ". + */ + public static String listNames(List recipients) { + List names = new ArrayList(recipients.size()); + for (RequestItemAuthor recipient : recipients) { + names.add(recipient.getFullName()); + } + return String.join(", ", names); + } } diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java index bba0913193..ba75faa854 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java @@ -8,6 +8,7 @@ package org.dspace.app.requestitem; import java.sql.SQLException; +import java.util.List; import org.dspace.content.Item; import org.dspace.core.Context; @@ -19,6 +20,6 @@ import org.dspace.core.Context; * @author Andrea Bollini */ public interface RequestItemAuthorExtractor { - public RequestItemAuthor getRequestItemAuthor(Context context, Item item) + public List getRequestItemAuthor(Context context, Item item) throws SQLException; } diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java index a5f7341039..452a368a45 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java @@ -8,19 +8,20 @@ package org.dspace.app.requestitem; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; import org.apache.commons.lang3.StringUtils; -import org.apache.logging.log4j.Logger; import org.dspace.content.Item; -import org.dspace.core.ConfigurationManager; import org.dspace.core.Context; import org.dspace.core.I18nUtil; import org.dspace.eperson.EPerson; import org.dspace.eperson.service.EPersonService; +import org.dspace.services.ConfigurationService; import org.springframework.beans.factory.annotation.Autowired; /** - * RequestItem strategy to allow DSpace support team's helpdesk to receive requestItem request + * RequestItem strategy to allow DSpace support team's helpdesk to receive requestItem request. * With this enabled, then the Item author/submitter doesn't receive the request, but the helpdesk instead does. * * Failover to the RequestItemSubmitterStrategy, which means the submitter would get the request if there is no @@ -30,23 +31,26 @@ import org.springframework.beans.factory.annotation.Autowired; * @author Peter Dietz */ public class RequestItemHelpdeskStrategy extends RequestItemSubmitterStrategy { - - private Logger log = org.apache.logging.log4j.LogManager.getLogger(RequestItemHelpdeskStrategy.class); - @Autowired(required = true) protected EPersonService ePersonService; + @Autowired(required = true) + private ConfigurationService configuration; + public RequestItemHelpdeskStrategy() { } @Override - public RequestItemAuthor getRequestItemAuthor(Context context, Item item) throws SQLException { - boolean helpdeskOverridesSubmitter = ConfigurationManager + public List getRequestItemAuthor(Context context, Item item) + throws SQLException { + boolean helpdeskOverridesSubmitter = configuration .getBooleanProperty("request.item.helpdesk.override", false); - String helpDeskEmail = ConfigurationManager.getProperty("mail.helpdesk"); + String helpDeskEmail = configuration.getProperty("mail.helpdesk"); if (helpdeskOverridesSubmitter && StringUtils.isNotBlank(helpDeskEmail)) { - return getHelpDeskPerson(context, helpDeskEmail); + List authors = new ArrayList<>(1); + authors.add(getHelpDeskPerson(context, helpDeskEmail)); + return authors; } else { //Fallback to default logic (author of Item) if helpdesk isn't fully enabled or setup return super.getRequestItemAuthor(context, item); diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemMetadataStrategy.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemMetadataStrategy.java index 4d2f78408a..990b19d104 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemMetadataStrategy.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemMetadataStrategy.java @@ -8,6 +8,7 @@ package org.dspace.app.requestitem; import java.sql.SQLException; +import java.util.ArrayList; import java.util.List; import org.apache.commons.lang3.StringUtils; @@ -20,7 +21,7 @@ import org.springframework.beans.factory.annotation.Autowired; /** * Try to look to an item metadata for the corresponding author name and email. - * Failover to the RequestItemSubmitterStrategy + * Failover to the RequestItemSubmitterStrategy. * * @author Andrea Bollini */ @@ -36,29 +37,32 @@ public class RequestItemMetadataStrategy extends RequestItemSubmitterStrategy { } @Override - public RequestItemAuthor getRequestItemAuthor(Context context, Item item) + public List getRequestItemAuthor(Context context, Item item) throws SQLException { if (emailMetadata != null) { List vals = itemService.getMetadataByMetadataString(item, emailMetadata); if (vals.size() > 0) { - String email = vals.iterator().next().getValue(); - String fullname = null; - if (fullNameMetadata != null) { - List nameVals = itemService.getMetadataByMetadataString(item, fullNameMetadata); - if (nameVals.size() > 0) { - fullname = nameVals.iterator().next().getValue(); + List authors = new ArrayList<>(vals.size()); + for (MetadataValue datum : vals) { + String email = datum.getValue(); + String fullname = null; + if (fullNameMetadata != null) { + List nameVals = itemService.getMetadataByMetadataString(item, fullNameMetadata); + if (!nameVals.isEmpty()) { + fullname = nameVals.get(0).getValue(); + } } - } - if (StringUtils.isBlank(fullname)) { - fullname = I18nUtil - .getMessage( - "org.dspace.app.requestitem.RequestItemMetadataStrategy.unnamed", - context); + if (StringUtils.isBlank(fullname)) { + fullname = I18nUtil.getMessage( + "org.dspace.app.requestitem.RequestItemMetadataStrategy.unnamed", + context); + } + RequestItemAuthor author = new RequestItemAuthor( + fullname, email); + authors.add(author); } - RequestItemAuthor author = new RequestItemAuthor( - fullname, email); - return author; + return authors; } } return super.getRequestItemAuthor(context, item); diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java index 8ed6238a8c..072451ff80 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java @@ -8,6 +8,8 @@ package org.dspace.app.requestitem; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; import org.dspace.content.Item; import org.dspace.core.Context; @@ -24,12 +26,14 @@ public class RequestItemSubmitterStrategy implements RequestItemAuthorExtractor } @Override - public RequestItemAuthor getRequestItemAuthor(Context context, Item item) + public List getRequestItemAuthor(Context context, Item item) throws SQLException { EPerson submitter = item.getSubmitter(); + List authors = new ArrayList<>(1); RequestItemAuthor author = new RequestItemAuthor( submitter.getFullName(), submitter.getEmail()); - return author; + authors.add(author); + return authors; } } diff --git a/dspace/config/spring/api/requestitem.xml b/dspace/config/spring/api/requestitem.xml index cd18add16d..8b28b8b4a9 100644 --- a/dspace/config/spring/api/requestitem.xml +++ b/dspace/config/spring/api/requestitem.xml @@ -8,22 +8,49 @@ http://www.springframework.org/schema/context/spring-context-2.5.xsd" default-autowire-candidates="*Service,*DAO,javax.sql.DataSource"> + Strategies for determining who receives Request Copy emails. + - + + + + + - + + + + + + + + + A list of references to RequestItemAuthorExtractor beans + + + + + + From d28c15dae31054377acdf2f8a6fc5a36f52b0506 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Tue, 2 Jul 2019 14:51:25 -0400 Subject: [PATCH 03/97] [DS-4289] use a new method to write the 'contents' file. --- .../app/itemexport/ItemExportServiceImpl.java | 48 +++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java b/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java index 73bd16c7d7..ceceb6fc3d 100644 --- a/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/app/itemexport/ItemExportServiceImpl.java @@ -63,17 +63,21 @@ import org.springframework.beans.factory.annotation.Autowired; * Item exporter to create simple AIPs for DSpace content. Currently exports * individual items, or entire collections. For instructions on use, see * printUsage() method. - *

+ *

* ItemExport creates the simple AIP package that the importer also uses. It * consists of: - *

- * /exportdir/42/ (one directory per item) / dublin_core.xml - qualified dublin - * core in RDF schema / contents - text file, listing one file per line / file1 - * - files contained in the item / file2 / ... - *

+ * 

{@code
+ * /exportdir/42/ (one directory per item)
+ *              / dublin_core.xml - qualified dublin core in RDF schema
+ *              / contents - text file, listing one file per line
+ *              / file1 - files contained in the item
+ *              / file2
+ *              / ...
+ * }
+ *

* issues -doesn't handle special characters in metadata (needs to turn {@code &'s} into * {@code &}, etc.) - *

+ *

* Modified by David Little, UCSD Libraries 12/21/04 to allow the registration * of files (bitstreams) into DSpace. * @@ -98,7 +102,7 @@ public class ItemExportServiceImpl implements ItemExportService { /** * log4j logger */ - private Logger log = org.apache.logging.log4j.LogManager.getLogger(ItemExportServiceImpl.class); + private final Logger log = org.apache.logging.log4j.LogManager.getLogger(); protected ItemExportServiceImpl() { @@ -165,6 +169,7 @@ public class ItemExportServiceImpl implements ItemExportService { // make it this far, now start exporting writeMetadata(c, myItem, itemDir, migrate); writeBitstreams(c, myItem, itemDir, excludeBitstreams); + writeCollections(myItem, itemDir); if (!migrate) { writeHandle(c, myItem, itemDir); } @@ -340,6 +345,33 @@ public class ItemExportServiceImpl implements ItemExportService { } } + /** + * Create the 'collections' file. List handles of all Collections which + * contain this Item. The "owning" Collection is listed first. + * + * @param item list collections holding this Item. + * @param destDir write the file here. + * @throws IOException if the file cannot be created or written. + */ + protected void writeCollections(Item item, File destDir) + throws IOException { + File outFile = new File(destDir, "collections"); + if (outFile.createNewFile()) { + try (PrintWriter out = new PrintWriter(new FileWriter(outFile))) { + String ownerHandle = item.getOwningCollection().getHandle(); + out.println(ownerHandle); + for (Collection collection : item.getCollections()) { + String collectionHandle = collection.getHandle(); + if (!collectionHandle.equals(ownerHandle)) { + out.println(collectionHandle); + } + } + } + } else { + throw new IOException("Cannot create 'collections' in " + destDir); + } + } + /** * Create both the bitstreams and the contents file. Any bitstreams that * were originally registered will be marked in the contents file as such. From e02d91122f9b96aea1441cd82a9e4c45f0290937 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Mon, 15 Jul 2019 12:41:13 -0400 Subject: [PATCH 04/97] [DS-4300] Add option to use provided Handles if any. --- .../org/dspace/administer/StructBuilder.java | 65 +++++++++++++++---- 1 file changed, 51 insertions(+), 14 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java b/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java index d512358be0..841b153513 100644 --- a/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java +++ b/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java @@ -30,6 +30,7 @@ import org.apache.commons.cli.HelpFormatter; import org.apache.commons.cli.Option; import org.apache.commons.cli.Options; import org.apache.commons.cli.ParseException; +import org.apache.commons.lang3.StringUtils; import org.apache.xpath.XPathAPI; import org.dspace.authorize.AuthorizeException; import org.dspace.content.Collection; @@ -43,6 +44,8 @@ import org.dspace.content.service.CommunityService; import org.dspace.core.Context; import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.service.EPersonService; +import org.dspace.handle.factory.HandleServiceFactory; +import org.dspace.handle.service.HandleService; import org.jdom.Element; import org.jdom.output.Format; import org.jdom.output.XMLOutputter; @@ -67,6 +70,7 @@ import org.xml.sax.SAXException; * * * } + * *

* It can be arbitrarily deep, and supports all the metadata elements * that make up the community and collection metadata. See the system @@ -95,12 +99,16 @@ public class StructBuilder { */ private static final Map communityMap = new HashMap<>(); - protected static CommunityService communityService + protected static final CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); - protected static CollectionService collectionService + protected static final CollectionService collectionService = ContentServiceFactory.getInstance().getCollectionService(); - protected static EPersonService ePersonService + protected static final EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); + protected static final HandleService handleService + = HandleServiceFactory.getInstance().getHandleService(); + + private static boolean keepHandles; /** * Default constructor @@ -136,6 +144,7 @@ public class StructBuilder { options.addOption("h", "help", false, "Print this help message."); options.addOption("?", "help"); options.addOption("x", "export", false, "Export the current structure as XML."); + options.addOption("k", "keep-handles", false, "Apply Handles from input document."); options.addOption(Option.builder("e").longOpt("eperson") .desc("User who is manipulating the repository's structure.") @@ -211,6 +220,7 @@ public class StructBuilder { inputStream = new FileInputStream(input); } + keepHandles = options.hasOption("k"); importStructure(context, inputStream, outputStream); } System.exit(0); @@ -228,8 +238,10 @@ public class StructBuilder { * @throws TransformerException * @throws SQLException */ - static void importStructure(Context context, InputStream input, OutputStream output) - throws IOException, ParserConfigurationException, SQLException, TransformerException { + static void importStructure(Context context, InputStream input, + OutputStream output) + throws IOException, ParserConfigurationException, SQLException, + TransformerException { // load the XML Document document = null; @@ -255,7 +267,19 @@ public class StructBuilder { // Check for 'identifier' attributes -- possibly output by this class. NodeList identifierNodes = XPathAPI.selectNodeList(document, "//*[@identifier]"); if (identifierNodes.getLength() > 0) { - System.err.println("The input document has 'identifier' attributes, which will be ignored."); + if (!keepHandles) { + System.err.println("The input document has 'identifier' attributes, which will be ignored."); + } else { + for (int i = 0; i < identifierNodes.getLength() ; i++) { + String identifier = identifierNodes.item(i).getAttributes().item(0).getTextContent(); + if (handleService.resolveToURL(context, identifier) != null) { + System.err.printf("The input document contains handle %s," + + " which is in use already. Aborting...%n", + identifier); + System.exit(1); + } + } + } } // load the mappings into the member variable hashmaps @@ -608,21 +632,23 @@ public class StructBuilder { Element[] elements = new Element[communities.getLength()]; for (int i = 0; i < communities.getLength(); i++) { - Community community; - Element element = new Element("community"); + Node tn = communities.item(i); + Node identifier = tn.getAttributes().getNamedItem("identifier"); // create the community or sub community - if (parent != null) { + Community community; + if (null == identifier + || StringUtils.isBlank(identifier.getNodeValue()) + || !keepHandles) { community = communityService.create(parent, context); } else { - community = communityService.create(null, context); + community = communityService.create(parent, context, identifier.getNodeValue()); } // default the short description to be an empty string communityService.setMetadata(context, community, "short_description", " "); // now update the metadata - Node tn = communities.item(i); for (Map.Entry entry : communityMap.entrySet()) { NodeList nl = XPathAPI.selectNodeList(tn, entry.getKey()); if (nl.getLength() == 1) { @@ -647,6 +673,7 @@ public class StructBuilder { // but it's here to keep it separate from the create process in // case // we want to move it or make it switchable later + Element element = new Element("community"); element.setAttribute("identifier", community.getHandle()); Element nameElement = new Element("name"); @@ -713,14 +740,23 @@ public class StructBuilder { Element[] elements = new Element[collections.getLength()]; for (int i = 0; i < collections.getLength(); i++) { - Element element = new Element("collection"); - Collection collection = collectionService.create(context, parent); + Node tn = collections.item(i); + Node identifier = tn.getAttributes().getNamedItem("identifier"); + + // Create the Collection. + Collection collection; + if (null == identifier + || StringUtils.isBlank(identifier.getNodeValue()) + || !keepHandles) { + collection = collectionService.create(context, parent); + } else { + collection = collectionService.create(context, parent, identifier.getNodeValue()); + } // default the short description to the empty string collectionService.setMetadata(context, collection, "short_description", " "); // import the rest of the metadata - Node tn = collections.item(i); for (Map.Entry entry : collectionMap.entrySet()) { NodeList nl = XPathAPI.selectNodeList(tn, entry.getKey()); if (nl.getLength() == 1) { @@ -730,6 +766,7 @@ public class StructBuilder { collectionService.update(context, collection); + Element element = new Element("collection"); element.setAttribute("identifier", collection.getHandle()); Element nameElement = new Element("name"); From d21b019c71488def1e2bca9249fd7ca30a9bd9db Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Tue, 4 Aug 2020 15:21:27 -0400 Subject: [PATCH 05/97] [DS-2670] Fix concompliance with new format rules. --- .../dspace/ctask/general/CreateMissingIdentifiers.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java b/dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java index e5afd3afe1..8eba916971 100644 --- a/dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java +++ b/dspace-api/src/main/java/org/dspace/ctask/general/CreateMissingIdentifiers.java @@ -10,6 +10,7 @@ package org.dspace.ctask.general; import java.io.IOException; import java.sql.SQLException; + import org.dspace.authorize.AuthorizeException; import org.dspace.content.DSpaceObject; import org.dspace.content.Item; @@ -30,18 +31,17 @@ import org.slf4j.LoggerFactory; * @author Mark H. Wood {@literal } */ public class CreateMissingIdentifiers - extends AbstractCurationTask -{ + extends AbstractCurationTask { private static final Logger LOG = LoggerFactory.getLogger(CreateMissingIdentifiers.class); @Override public int perform(DSpaceObject dso) - throws IOException - { + throws IOException { // Only some kinds of model objects get identifiers - if (!(dso instanceof Item)) + if (!(dso instanceof Item)) { return Curator.CURATE_SKIP; + } String typeText = Constants.typeText[dso.getType()]; From 5c6cd3eca3226b7a4c4cd88ecf79e0570923060d Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Sat, 13 Mar 2021 20:22:05 -0500 Subject: [PATCH 06/97] [DS-3951] Update a newer test to cope with lists of recipients. --- .../app/rest/eperson/DeleteEPersonSubmitterIT.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/eperson/DeleteEPersonSubmitterIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/eperson/DeleteEPersonSubmitterIT.java index e280bffdef..5a41c6c631 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/eperson/DeleteEPersonSubmitterIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/eperson/DeleteEPersonSubmitterIT.java @@ -142,10 +142,10 @@ public class DeleteEPersonSubmitterIT extends AbstractControllerIntegrationTest Item item = itemService.find(context, installItem.getID()); - RequestItemAuthor requestItemAuthor = requestItemAuthorExtractor.getRequestItemAuthor(context, item); + List requestItemAuthor = requestItemAuthorExtractor.getRequestItemAuthor(context, item); - assertEquals("Help Desk", requestItemAuthor.getFullName()); - assertEquals("dspace-help@myu.edu", requestItemAuthor.getEmail()); + assertEquals("Help Desk", requestItemAuthor.get(0).getFullName()); + assertEquals("dspace-help@myu.edu", requestItemAuthor.get(0).getEmail()); } /** @@ -171,7 +171,7 @@ public class DeleteEPersonSubmitterIT extends AbstractControllerIntegrationTest Item item = installItemService.installItem(context, wsi); - List opsToWithDraw = new ArrayList(); + List opsToWithDraw = new ArrayList<>(); ReplaceOperation replaceOperationToWithDraw = new ReplaceOperation("/withdrawn", true); opsToWithDraw.add(replaceOperationToWithDraw); String patchBodyToWithdraw = getPatchContent(opsToWithDraw); @@ -191,7 +191,7 @@ public class DeleteEPersonSubmitterIT extends AbstractControllerIntegrationTest assertNull(retrieveItemSubmitter(item.getID())); - List opsToReinstate = new ArrayList(); + List opsToReinstate = new ArrayList<>(); ReplaceOperation replaceOperationToReinstate = new ReplaceOperation("/withdrawn", false); opsToReinstate.add(replaceOperationToReinstate); String patchBodyToReinstate = getPatchContent(opsToReinstate); From 81a72354325954191500353a817f932da4f92cd6 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Wed, 25 Aug 2021 10:33:50 -0400 Subject: [PATCH 07/97] Take up many IDE suggestions. --- .../dspace/app/statistics/LogAnalyser.java | 105 +++++++++--------- 1 file changed, 54 insertions(+), 51 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java b/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java index 0ece47d32d..fbb06acb67 100644 --- a/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java +++ b/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java @@ -236,12 +236,12 @@ public class LogAnalyser { /** * pattern to match commented out lines from the config file */ - private static final Pattern comment = Pattern.compile("^#"); + private static final Pattern COMMENT = Pattern.compile("^#"); /** * pattern to match genuine lines from the config file */ - private static final Pattern real = Pattern.compile("^(.+)=(.+)"); + private static final Pattern REAL = Pattern.compile("^(.+)=(.+)"); /** * pattern to match all search types @@ -406,18 +406,18 @@ public class LogAnalyser { startTime = new GregorianCalendar(); //instantiate aggregators - actionAggregator = new HashMap(); - searchAggregator = new HashMap(); - userAggregator = new HashMap(); - itemAggregator = new HashMap(); - archiveStats = new HashMap(); + actionAggregator = new HashMap<>(); + searchAggregator = new HashMap<>(); + userAggregator = new HashMap<>(); + itemAggregator = new HashMap<>(); + archiveStats = new HashMap<>(); //instantiate lists - generalSummary = new ArrayList(); - excludeWords = new ArrayList(); - excludeTypes = new ArrayList(); - excludeChars = new ArrayList(); - itemTypes = new ArrayList(); + generalSummary = new ArrayList<>(); + excludeWords = new ArrayList<>(); + excludeTypes = new ArrayList<>(); + excludeChars = new ArrayList<>(); + itemTypes = new ArrayList<>(); // set the parameters for this analysis setParameters(myLogDir, myFileTemplate, myConfigFile, myOutFile, myStartDate, myEndDate, myLookUp); @@ -529,10 +529,11 @@ public class LogAnalyser { // for each search word add to the aggregator or // increment the aggregator's counter - for (int j = 0; j < words.length; j++) { + for (String word : words) { // FIXME: perhaps aggregators ought to be objects // themselves - searchAggregator.put(words[j], increment(searchAggregator, words[j])); + searchAggregator.put(word, + increment(searchAggregator, word)); } } @@ -591,7 +592,7 @@ public class LogAnalyser { } // do the average views analysis - if ((archiveStats.get("All Items")).intValue() != 0) { + if ((archiveStats.get("All Items")) != 0) { // FIXME: this is dependent on their being a query on the db, which // there might not always be if it becomes configurable Double avg = Math.ceil( @@ -672,55 +673,55 @@ public class LogAnalyser { Iterator keys = null; // output the number of lines parsed - summary.append("log_lines=" + Integer.toString(lineCount) + "\n"); + summary.append("log_lines=").append(Integer.toString(lineCount)).append("\n"); // output the number of warnings encountered - summary.append("warnings=" + Integer.toString(warnCount) + "\n"); - summary.append("exceptions=" + Integer.toString(excCount) + "\n"); + summary.append("warnings=").append(Integer.toString(warnCount)).append("\n"); + summary.append("exceptions=").append(Integer.toString(excCount)).append("\n"); // set the general summary config up in the aggregator file for (int i = 0; i < generalSummary.size(); i++) { - summary.append("general_summary=" + generalSummary.get(i) + "\n"); + summary.append("general_summary=").append(generalSummary.get(i)).append("\n"); } // output the host name - summary.append("server_name=" + hostName + "\n"); + summary.append("server_name=").append(hostName).append("\n"); // output the service name - summary.append("service_name=" + name + "\n"); + summary.append("service_name=").append(name).append("\n"); // output the date information if necessary SimpleDateFormat sdf = new SimpleDateFormat("dd'/'MM'/'yyyy"); if (startDate != null) { - summary.append("start_date=" + sdf.format(startDate) + "\n"); + summary.append("start_date=").append(sdf.format(startDate)).append("\n"); } else if (logStartDate != null) { - summary.append("start_date=" + sdf.format(logStartDate) + "\n"); + summary.append("start_date=").append(sdf.format(logStartDate)).append("\n"); } if (endDate != null) { - summary.append("end_date=" + sdf.format(endDate) + "\n"); + summary.append("end_date=").append(sdf.format(endDate)).append("\n"); } else if (logEndDate != null) { - summary.append("end_date=" + sdf.format(logEndDate) + "\n"); + summary.append("end_date=").append(sdf.format(logEndDate)).append("\n"); } // write out the archive stats keys = archiveStats.keySet().iterator(); while (keys.hasNext()) { String key = keys.next(); - summary.append("archive." + key + "=" + archiveStats.get(key) + "\n"); + summary.append("archive.").append(key).append("=").append(archiveStats.get(key)).append("\n"); } // write out the action aggregation results keys = actionAggregator.keySet().iterator(); while (keys.hasNext()) { String key = keys.next(); - summary.append("action." + key + "=" + actionAggregator.get(key) + "\n"); + summary.append("action.").append(key).append("=").append(actionAggregator.get(key)).append("\n"); } // depending on the config settings for reporting on emails output the // login information - summary.append("user_email=" + userEmail + "\n"); + summary.append("user_email=").append(userEmail).append("\n"); int address = 1; keys = userAggregator.keySet().iterator(); @@ -731,9 +732,10 @@ public class LogAnalyser { String key = keys.next(); summary.append("user."); if (userEmail.equals("on")) { - summary.append(key + "=" + userAggregator.get(key) + "\n"); + summary.append(key).append("=").append(userAggregator.get(key)).append("\n"); } else if (userEmail.equals("alias")) { - summary.append("Address " + Integer.toString(address++) + "=" + userAggregator.get(key) + "\n"); + summary.append("Address ").append(Integer.toString(address++)) + .append("=").append(userAggregator.get(key)).append("\n"); } } @@ -742,12 +744,13 @@ public class LogAnalyser { // the listing there are // output the search word information - summary.append("search_floor=" + searchFloor + "\n"); + summary.append("search_floor=").append(searchFloor).append("\n"); keys = searchAggregator.keySet().iterator(); while (keys.hasNext()) { String key = keys.next(); - if ((searchAggregator.get(key)).intValue() >= searchFloor) { - summary.append("search." + key + "=" + searchAggregator.get(key) + "\n"); + if ((searchAggregator.get(key)) >= searchFloor) { + summary.append("search.").append(key).append("=") + .append(searchAggregator.get(key)).append("\n"); } } @@ -759,35 +762,35 @@ public class LogAnalyser { // be the same thing. // item viewing information - summary.append("item_floor=" + itemFloor + "\n"); - summary.append("host_url=" + url + "\n"); - summary.append("item_lookup=" + itemLookup + "\n"); + summary.append("item_floor=").append(itemFloor).append("\n"); + summary.append("host_url=").append(url).append("\n"); + summary.append("item_lookup=").append(itemLookup).append("\n"); // write out the item access information keys = itemAggregator.keySet().iterator(); while (keys.hasNext()) { String key = keys.next(); - if ((itemAggregator.get(key)).intValue() >= itemFloor) { - summary.append("item." + key + "=" + itemAggregator.get(key) + "\n"); + if ((itemAggregator.get(key)) >= itemFloor) { + summary.append("item.").append(key).append("=") + .append(itemAggregator.get(key)).append("\n"); } } // output the average views per item if (views > 0) { - summary.append("avg_item_views=" + views + "\n"); + summary.append("avg_item_views=").append(views).append("\n"); } // insert the analysis processing time information Calendar endTime = new GregorianCalendar(); long timeInMillis = (endTime.getTimeInMillis() - startTime.getTimeInMillis()); - summary.append("analysis_process_time=" + Long.toString(timeInMillis / 1000) + "\n"); + summary.append("analysis_process_time=") + .append(Long.toString(timeInMillis / 1000)).append("\n"); // finally write the string into the output file - try { - BufferedWriter out = new BufferedWriter(new FileWriter(outFile)); + try (BufferedWriter out = new BufferedWriter(new FileWriter(outFile));) { out.write(summary.toString()); out.flush(); - out.close(); } catch (IOException e) { System.out.println("Unable to write to output file " + outFile); System.exit(0); @@ -891,11 +894,11 @@ public class LogAnalyser { if (i > 0) { wordRXString.append("|"); } - wordRXString.append(" " + excludeWords.get(i) + " "); + wordRXString.append(" ").append(excludeWords.get(i)).append(" "); wordRXString.append("|"); - wordRXString.append("^" + excludeWords.get(i) + " "); + wordRXString.append("^").append(excludeWords.get(i)).append(" "); wordRXString.append("|"); - wordRXString.append(" " + excludeWords.get(i) + "$"); + wordRXString.append(" ").append(excludeWords.get(i)).append("$"); } wordRXString.append(")"); wordRX = Pattern.compile(wordRXString.toString()); @@ -956,8 +959,8 @@ public class LogAnalyser { // read in the config file and set up our instance variables while ((record = br.readLine()) != null) { // check to see what kind of line we have - Matcher matchComment = comment.matcher(record); - Matcher matchReal = real.matcher(record); + Matcher matchComment = COMMENT.matcher(record); + Matcher matchReal = REAL.matcher(record); // if the line is not a comment and is real, read it in if (!matchComment.matches() && matchReal.matches()) { @@ -968,7 +971,7 @@ public class LogAnalyser { // read the config values into our instance variables (see // documentation for more info on config params) if (key.equals("general.summary")) { - actionAggregator.put(value, Integer.valueOf(0)); + actionAggregator.put(value, 0); generalSummary.add(value); } @@ -1022,9 +1025,9 @@ public class LogAnalyser { Integer newValue = null; if (map.containsKey(key)) { // FIXME: this seems like a ridiculous way to add Integers - newValue = Integer.valueOf((map.get(key)).intValue() + 1); + newValue = (map.get(key)) + 1; } else { - newValue = Integer.valueOf(1); + newValue = 1; } return newValue; } From 702a72ffab9c9b662f185fd54d70f570a7b4956d Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Wed, 25 Aug 2021 12:58:27 -0400 Subject: [PATCH 08/97] Replace handmade option parsing with Commons CLI to address LGTM array bounds complaints. --- .../dspace/app/statistics/LogAnalyser.java | 88 +++++++++++++------ .../app/statistics/ReportGenerator.java | 64 +++++++++----- 2 files changed, 103 insertions(+), 49 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java b/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java index fbb06acb67..6c6cefd9e4 100644 --- a/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java +++ b/dspace-api/src/main/java/org/dspace/app/statistics/LogAnalyser.java @@ -29,6 +29,10 @@ import java.util.TimeZone; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; import org.apache.commons.lang3.StringUtils; import org.dspace.core.Context; import org.dspace.core.LogManager; @@ -337,44 +341,72 @@ public class LogAnalyser { Date myEndDate = null; boolean myLookUp = false; - // read in our command line options - for (int i = 0; i < argv.length; i++) { - if (argv[i].equals("-log")) { - myLogDir = argv[i + 1]; - } + // Define command line options. + Options options = new Options(); + Option option; - if (argv[i].equals("-file")) { - myFileTemplate = argv[i + 1]; - } + option = Option.builder().longOpt("log").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-cfg")) { - myConfigFile = argv[i + 1]; - } + option = Option.builder().longOpt("file").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-out")) { - myOutFile = argv[i + 1]; - } + option = Option.builder().longOpt("cfg").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-help")) { - LogAnalyser.usage(); - System.exit(0); - } + option = Option.builder().longOpt("out").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-start")) { - myStartDate = parseDate(argv[i + 1]); - } + option = Option.builder().longOpt("help").build(); + options.addOption(option); - if (argv[i].equals("-end")) { - myEndDate = parseDate(argv[i + 1]); - } + option = Option.builder().longOpt("start").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-lookup")) { - myLookUp = true; - } + option = Option.builder().longOpt("end").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("lookup").build(); + + // Parse the command. + DefaultParser cmdParser = new DefaultParser(); + CommandLine cmd = cmdParser.parse(options, argv); + + // Analyze the command. + if (cmd.hasOption("help")) { + LogAnalyser.usage(); + System.exit(0); } + if (cmd.hasOption("log")) { + myLogDir = cmd.getOptionValue("log"); + } + + if (cmd.hasOption("file")) { + myFileTemplate = cmd.getOptionValue("file"); + } + + if (cmd.hasOption("cfg")) { + myConfigFile = cmd.getOptionValue("cfg"); + } + + if (cmd.hasOption("out")) { + myOutFile = cmd.getOptionValue("out"); + } + + if (cmd.hasOption("start")) { + myStartDate = parseDate(cmd.getOptionValue("start")); + } + + if (cmd.hasOption("end")) { + myEndDate = parseDate(cmd.getOptionValue("end")); + } + + myLookUp = cmd.hasOption("lookup"); + // now call the method which actually processes the logs - processLogs(context, myLogDir, myFileTemplate, myConfigFile, myOutFile, myStartDate, myEndDate, myLookUp); + processLogs(context, myLogDir, myFileTemplate, myConfigFile, myOutFile, + myStartDate, myEndDate, myLookUp); } /** diff --git a/dspace-api/src/main/java/org/dspace/app/statistics/ReportGenerator.java b/dspace-api/src/main/java/org/dspace/app/statistics/ReportGenerator.java index 25c6d8cb9c..c5fe0072f5 100644 --- a/dspace-api/src/main/java/org/dspace/app/statistics/ReportGenerator.java +++ b/dspace-api/src/main/java/org/dspace/app/statistics/ReportGenerator.java @@ -27,6 +27,10 @@ import java.util.StringTokenizer; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; import org.dspace.content.Item; import org.dspace.content.MetadataSchemaEnum; import org.dspace.content.MetadataValue; @@ -162,7 +166,7 @@ public class ReportGenerator { /** * pattern that matches an unqualified aggregator property */ - private static final Pattern real = Pattern.compile("^(.+)=(.+)"); + private static final Pattern REAL = Pattern.compile("^(.+)=(.+)"); ////////////////////////// // Miscellaneous variables @@ -221,28 +225,46 @@ public class ReportGenerator { String myOutput = null; String myMap = null; - // read in our command line options - for (int i = 0; i < argv.length; i++) { - if (argv[i].equals("-format")) { - myFormat = argv[i + 1].toLowerCase(); - } + Options options = new Options(); + Option option; - if (argv[i].equals("-in")) { - myInput = argv[i + 1]; - } + option = Option.builder().longOpt("format").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-out")) { - myOutput = argv[i + 1]; - } + option = Option.builder().longOpt("in").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-map")) { - myMap = argv[i + 1]; - } + option = Option.builder().longOpt("out").hasArg().build(); + options.addOption(option); - if (argv[i].equals("-help")) { - usage(); - System.exit(0); - } + option = Option.builder().longOpt("map").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("help").build(); + options.addOption(option); + + DefaultParser parser = new DefaultParser(); + CommandLine cmd = parser.parse(options, argv); + + if (cmd.hasOption("help")) { + usage(); + System.exit(0); + } + + if (cmd.hasOption("format")) { + myFormat = cmd.getOptionValue("format"); + } + + if (cmd.hasOption("in")) { + myInput = cmd.getOptionValue("in"); + } + + if (cmd.hasOption("out")) { + myOutput = cmd.getOptionValue("out"); + } + + if (cmd.hasOption("map")) { + myMap = cmd.getOptionValue("map"); } processReport(context, myFormat, myInput, myOutput, myMap); @@ -576,7 +598,7 @@ public class ReportGenerator { // loop through the map file and read in the values while ((record = br.readLine()) != null) { - Matcher matchReal = real.matcher(record); + Matcher matchReal = REAL.matcher(record); // if the line is real then read it in if (matchReal.matches()) { @@ -650,7 +672,7 @@ public class ReportGenerator { // loop through the aggregator file and read in the values while ((record = br.readLine()) != null) { // match real lines - Matcher matchReal = real.matcher(record); + Matcher matchReal = REAL.matcher(record); // pre-prepare our input strings String section = null; From 48d409d87d1afac34af643127034fa4e563e63de Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Wed, 25 Aug 2021 14:09:37 -0400 Subject: [PATCH 09/97] Formalize more option parsing. --- .../storage/bitstore/S3BitStoreService.java | 46 ++-- .../org/purl/sword/client/ClientOptions.java | 229 +++++++++--------- 2 files changed, 146 insertions(+), 129 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java b/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java index ce2b3b3f05..004a8bba04 100644 --- a/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java +++ b/dspace-api/src/main/java/org/dspace/storage/bitstore/S3BitStoreService.java @@ -25,6 +25,12 @@ import com.amazonaws.services.s3.model.ObjectMetadata; import com.amazonaws.services.s3.model.PutObjectRequest; import com.amazonaws.services.s3.model.PutObjectResult; import com.amazonaws.services.s3.model.S3Object; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.apache.http.HttpStatus; @@ -324,27 +330,37 @@ public class S3BitStoreService implements BitStoreService { * @throws Exception generic exception */ public static void main(String[] args) throws Exception { - //TODO use proper CLI, or refactor to be a unit test. Can't mock this without keys though. + //TODO Perhaps refactor to be a unit test. Can't mock this without keys though. // parse command line - String assetFile = null; - String accessKey = null; - String secretKey = null; + Options options = new Options(); + Option option; - for (int i = 0; i < args.length; i += 2) { - if (args[i].startsWith("-a")) { - accessKey = args[i + 1]; - } else if (args[i].startsWith("-s")) { - secretKey = args[i + 1]; - } else if (args[i].startsWith("-f")) { - assetFile = args[i + 1]; - } - } + option = Option.builder("a").desc("access key").hasArg().required().build(); + options.addOption(option); - if (accessKey == null || secretKey == null || assetFile == null) { - System.out.println("Missing arguments - exiting"); + option = Option.builder("s").desc("secret key").hasArg().required().build(); + options.addOption(option); + + option = Option.builder("f").desc("asset file name").hasArg().required().build(); + options.addOption(option); + + DefaultParser parser = new DefaultParser(); + + CommandLine command; + try { + command = parser.parse(options, args); + } catch (ParseException e) { + System.err.println(e.getMessage()); + new HelpFormatter().printHelp( + S3BitStoreService.class.getSimpleName() + "options", options); return; } + + String accessKey = command.getOptionValue("a"); + String secretKey = command.getOptionValue("s"); + String assetFile = command.getOptionValue("f"); + S3BitStoreService store = new S3BitStoreService(); AWSCredentials awsCredentials = new BasicAWSCredentials(accessKey, secretKey); diff --git a/dspace-sword/src/main/java/org/purl/sword/client/ClientOptions.java b/dspace-sword/src/main/java/org/purl/sword/client/ClientOptions.java index e085baeb78..688b61ad17 100644 --- a/dspace-sword/src/main/java/org/purl/sword/client/ClientOptions.java +++ b/dspace-sword/src/main/java/org/purl/sword/client/ClientOptions.java @@ -13,6 +13,12 @@ import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; +import org.apache.commons.cli.ParseException; +import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; /** @@ -72,7 +78,7 @@ public class ClientOptions { private String filename = null; /** - * Filetype. + * File type. */ private String filetype = null; @@ -117,17 +123,18 @@ public class ClientOptions { /** * Logger. */ - private static Logger log = org.apache.logging.log4j.LogManager.getLogger(ClientOptions.class); + private static final Logger log = LogManager.getLogger(); /** * List of multiple destination items. Used if the mode is set to multipost. */ - private List multiPost = new ArrayList(); + private final List multiPost = new ArrayList<>(); /** * Pattern string to extract the data from a destination parameter in multipost mode. */ - private static final Pattern multiPattern = Pattern.compile("(.*?)(\\[(.*?)\\]) {0,1}(:(.*)) {0,1}@(http://.*)"); + private static final Pattern MULTI_PATTERN + = Pattern.compile("(.*?)(\\[(.*?)\\]) {0,1}(:(.*)) {0,1}@(http://.*)"); /** * Flag that indicates if the GUI mode has been set. This is @@ -148,128 +155,122 @@ public class ClientOptions { * @return True if the options were parsed successfully. */ public boolean parseOptions(String[] args) { + Options options = new Options(); + options.addOption(Option.builder().longOpt("md5").build()) + .addOption(Option.builder().longOpt("noOp").build()) + .addOption(Option.builder().longOpt("verbose").build()) + .addOption(Option.builder().longOpt("cmd").build()) + .addOption(Option.builder().longOpt("gui").build()) + .addOption(Option.builder().longOpt("help").build()) + .addOption(Option.builder().longOpt("nocapture").build()); + + Option option; + + option = Option.builder().longOpt("host").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("port").hasArg().build(); + options.addOption(option); + + option = Option.builder("u").hasArg().build(); + options.addOption(option); + + option = Option.builder("p").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("href").hasArg().build(); + options.addOption(option); + + option = Option.builder("t").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("file").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("filetype").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("slug").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("onBehalfOf").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("formatNamespace").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("checksumError").hasArg().build(); + options.addOption(option); + + option = Option.builder().longOpt("dest").hasArg().build(); + options.addOption(option); + + DefaultParser parser = new DefaultParser(); + CommandLine command; try { - // iterate over the args - for (int i = 0; i < args.length; i++) { - if ("-md5".equals(args[i])) { - md5 = true; + command = parser.parse(options, args); + } catch (ParseException ex) { + log.error(ex.getMessage()); + return false; + } + + if (command.hasOption("help")) { + return false; // force the calling code to display the usage information. + } + md5 = command.hasOption("md5"); + noOp = command.hasOption("noOp"); + verbose = command.hasOption("verbose"); + if (command.hasOption("cmd")) { + guiMode = false; + } + if (command.hasOption("gui")) { + guiMode = true; + } + proxyHost = command.getOptionValue("host"); + if (command.hasOption("port")) { + proxyPort = Integer.parseInt(command.getOptionValue("port")); + } + username = command.getOptionValue("u"); + password = command.getOptionValue("p"); + href = command.getOptionValue("href"); + accessType = command.getOptionValue("t"); + filename = command.getOptionValue("file"); + filetype = command.getOptionValue("filetype"); + slug = command.getOptionValue("slug"); + onBehalfOf = command.getOptionValue("onBehalfOf"); + formatNamespace = command.getOptionValue("formatNamespace"); + checksumError = command.hasOption("checksumError"); + noCapture = command.hasOption("nocapture"); + if (command.hasOption("dest")) { + String dest = command.getOptionValue("dest"); + Matcher m = MULTI_PATTERN.matcher(dest); + if (!m.matches()) { + log.debug("Error with dest parameter. Ignoring value: {}", dest); + } else { + int numGroups = m.groupCount(); + for (int g = 0; g <= numGroups; g++) { + log.debug("Group ({}) is: {}", g, m.group(g)); } - if ("-noOp".equals(args[i])) { - noOp = true; - } + String group_username = m.group(1); + String group_onBehalfOf = m.group(3); + String group_password = m.group(5); + String group_url = m.group(6); + PostDestination destination = new PostDestination(group_url, + group_username, group_password, group_onBehalfOf); - if ("-verbose".equals(args[i])) { - verbose = true; - } - - if ("-cmd".equals(args[i])) { - guiMode = false; - } - - if ("-gui".equals(args[i])) { - guiMode = true; - } - - if ("-host".equals(args[i])) { - i++; - proxyHost = args[i]; - } - - if ("-port".equals(args[i])) { - i++; - proxyPort = Integer.parseInt(args[i]); - } - - if ("-u".equals(args[i])) { - i++; - username = args[i]; - } - - if ("-p".equals(args[i])) { - i++; - password = args[i]; - } - - if ("-href".equals(args[i])) { - i++; - href = args[i]; - } - - if ("-help".equals(args[i])) { - // force the calling code to display the usage information - return false; - } - - if ("-t".equals(args[i])) { - i++; - accessType = args[i]; - } - - if ("-file".equals(args[i])) { - i++; - filename = args[i]; - } - - if ("-filetype".equals(args[i])) { - i++; - filetype = args[i]; - } - - if ("-slug".equals(args[i])) { - i++; - slug = args[i]; - } - - if ("-onBehalfOf".equals(args[i])) { - i++; - onBehalfOf = args[i]; - } - - if ("-formatNamespace".equals(args[i])) { - i++; - formatNamespace = args[i]; - } - - if ("-checksumError".equals(args[i])) { - i++; - checksumError = true; - } - - if ("-dest".equals(args[i])) { - i++; - Matcher m = multiPattern.matcher(args[i]); - if (!m.matches()) { - log.debug("Error with dest parameter. Ignoring value: " + args[i]); - } else { - int numGroups = m.groupCount(); - for (int g = 0; g <= numGroups; g++) { - log.debug("Group (" + g + ") is: " + m.group(g)); - } - - String username = m.group(1); - String onBehalfOf = m.group(3); - String password = m.group(5); - String url = m.group(6); - PostDestination destination = new PostDestination(url, username, password, onBehalfOf); - - multiPost.add(destination); - } - } - - if ("-nocapture".equals(args[i])) { - i++; - noCapture = true; - } + multiPost.add(destination); } + } + try { // apply any settings if (href == null && "service".equals(accessType)) { log.error("No href specified."); return false; } - if (multiPost.size() == 0 && "multipost".equals(accessType)) { + if (multiPost.isEmpty() && "multipost".equals(accessType)) { log.error("No destinations specified"); return false; } From 134873c98ade6ab921e8cc9bad503620363d2933 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Wed, 25 Aug 2021 15:26:39 -0400 Subject: [PATCH 10/97] Replace array with Queue to avoid index-out-of-bounds, make neater. --- .../ctask/general/MetadataWebService.java | 94 ++++++++++--------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/ctask/general/MetadataWebService.java b/dspace-api/src/main/java/org/dspace/ctask/general/MetadataWebService.java index edeb2a6d02..f7ab18c01e 100644 --- a/dspace-api/src/main/java/org/dspace/ctask/general/MetadataWebService.java +++ b/dspace-api/src/main/java/org/dspace/ctask/general/MetadataWebService.java @@ -10,11 +10,13 @@ package org.dspace.ctask.general; import java.io.IOException; import java.io.InputStream; import java.sql.SQLException; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Queue; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.xml.XMLConstants; @@ -33,6 +35,7 @@ import org.apache.http.HttpStatus; import org.apache.http.client.methods.HttpGet; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.dspace.authorize.AuthorizeException; import org.dspace.content.DSpaceObject; @@ -60,18 +63,18 @@ import org.xml.sax.SAXException; * Intended use: cataloging tool in workflow and general curation. * The task uses a URL 'template' to compose the service call, e.g. * - * {@code http://www.sherpa.ac.uk/romeo/api29.php?issn=\{dc.identifier.issn\}} + *

{@code http://www.sherpa.ac.uk/romeo/api29.php?issn=\{dc.identifier.issn\}} * - * Task will substitute the value of the passed item's metadata field + *

Task will substitute the value of the passed item's metadata field * in the {parameter} position. If multiple values are present in the * item field, the first value is used. * - * The task uses another property (the datamap) to determine what data + *

The task uses another property (the datamap) to determine what data * to extract from the service response and how to use it, e.g. * - * {@code //publisher/name=>dc.publisher,//romeocolour} + *

{@code //publisher/name=>dc.publisher,//romeocolour} * - * Task will evaluate the left-hand side (or entire token) of each + *

Task will evaluate the left-hand side (or entire token) of each * comma-separated token in the property as an XPath 1.0 expression into * the response document, and if there is a mapping symbol (e.g. {@code '=>'}) and * value, it will assign the response document value(s) to the named @@ -79,48 +82,52 @@ import org.xml.sax.SAXException; * multiple values, they will all be assigned to the item field. The * mapping symbol governs the nature of metadata field assignment: * - * {@code '->'} mapping will add to any existing values in the item field - * {@code '=>'} mapping will replace any existing values in the item field - * {@code '~>'} mapping will add *only* if item field has no existing values + *

    + *
  • {@code '->'} mapping will add to any existing values in the item field
    • + *
  • {@code '=>'} mapping will replace any existing values in the item field
    • + *
  • {@code '~>'} mapping will add *only* if item field has no existing values
    • + *
* - * Unmapped data (without a mapping symbol) will simply be added to the task + *

Unmapped data (without a mapping symbol) will simply be added to the task * result string, prepended by the XPath expression (a little prettified). * Each label/value pair in the result string is separated by a space, * unless the optional 'separator' property is defined. * - * A very rudimentary facility for transformation of data is supported, e.g. + *

A very rudimentary facility for transformation of data is supported, e.g. * - * {@code http://www.crossref.org/openurl/?id=\{doi:dc.relation.isversionof\}&format=unixref} + *

{@code http://www.crossref.org/openurl/?id=\{doi:dc.relation.isversionof\}&format=unixref} * - * The 'doi:' prefix will cause the task to look for a 'transform' with that + *

The 'doi:' prefix will cause the task to look for a 'transform' with that * name, which is applied to the metadata value before parameter substitution * occurs. Transforms are defined in a task property such as the following: * - * {@code transform.doi = match 10. trunc 60} + *

{@code transform.doi = match 10. trunc 60} * - * This means exclude the value string up to the occurrence of '10.', then + *

This means exclude the value string up to the occurrence of '10.', then * truncate after 60 characters. The only transform functions currently defined: * - * {@code 'cut' } = remove number leading characters - * {@code 'trunc' } = remove trailing characters after number length - * {@code 'match' } = start match at pattern - * {@code 'text' } = append literal characters (enclose in ' ' when whitespace needed) + *

    + *
  • {@code 'cut' } = remove number leading characters
    • + *
  • {@code 'trunc' } = remove trailing characters after number length
    • + *
  • {@code 'match' } = start match at pattern
    • + *
  • {@code 'text' } = append literal characters (enclose in ' ' when whitespace needed)
    • + *
* - * If the transform results in an invalid state (e.g. cutting more characters + *

If the transform results in an invalid state (e.g. cutting more characters * than are in the value), the condition will be logged and the * un-transformed value used. * - * Transforms may also be used in datamaps, e.g. + *

Transforms may also be used in datamaps, e.g. * - * {@code //publisher/name=>shorten:dc.publisher,//romeocolour} + *

{@code //publisher/name=>shorten:dc.publisher,//romeocolour} * - * which would apply the 'shorten' transform to the service response value(s) + *

which would apply the 'shorten' transform to the service response value(s) * prior to metadata field assignment. * - * An optional property 'headers' may be defined to stipulate any HTTP headers + *

An optional property 'headers' may be defined to stipulate any HTTP headers * required in the service call. The property syntax is double-pipe separated headers: * - * {@code Accept: text/xml||Cache-Control: no-cache} + *

{@code Accept: text/xml||Cache-Control: no-cache} * * @author richardrodgers */ @@ -128,9 +135,9 @@ import org.xml.sax.SAXException; @Suspendable public class MetadataWebService extends AbstractCurationTask implements NamespaceContext { /** - * log4j category + * logging category */ - private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(MetadataWebService.class); + private static final Logger log = LogManager.getLogger(); // transform token parsing pattern protected Pattern ttPattern = Pattern.compile("\'([^\']*)\'|(\\S+)"); // URL of web service with template parameters @@ -360,42 +367,45 @@ public class MetadataWebService extends AbstractCurationTask implements Namespac if (transDef == null) { return value; } - String[] tokens = tokenize(transDef); + Queue tokens = tokenize(transDef); String retValue = value; - for (int i = 0; i < tokens.length; i += 2) { - if ("cut".equals(tokens[i]) || "trunc".equals(tokens[i])) { - int index = Integer.parseInt(tokens[i + 1]); + while (!tokens.isEmpty()) { + String function = tokens.poll(); + if ("cut".equals(function) || "trunc".equals(function)) { + String argument = tokens.poll(); + int index = Integer.parseInt(argument); if (retValue.length() > index) { - if ("cut".equals(tokens[i])) { + if ("cut".equals(function)) { retValue = retValue.substring(index); } else { retValue = retValue.substring(0, index); } - } else if ("cut".equals(tokens[i])) { - log.error("requested cut: " + index + " exceeds value length"); + } else if ("cut".equals(function)) { + log.error("requested cut: {} exceeds value length", index); return value; } - } else if ("match".equals(tokens[i])) { - int index2 = retValue.indexOf(tokens[i + 1]); + } else if ("match".equals(function)) { + String argument = tokens.poll(); + int index2 = retValue.indexOf(argument); if (index2 > 0) { retValue = retValue.substring(index2); } else { - log.error("requested match: " + tokens[i + 1] + " failed"); + log.error("requested match: {} failed", argument); return value; } - } else if ("text".equals(tokens[i])) { - retValue = retValue + tokens[i + 1]; + } else if ("text".equals(function)) { + retValue = retValue + tokens.poll(); } else { - log.error(" unknown transform operation: " + tokens[i]); + log.error(" unknown transform operation: " + function); return value; } } return retValue; } - protected String[] tokenize(String text) { - List list = new ArrayList<>(); + protected Queue tokenize(String text) { Matcher m = ttPattern.matcher(text); + Queue list = new ArrayDeque<>(m.groupCount()); while (m.find()) { if (m.group(1) != null) { list.add(m.group(1)); @@ -403,7 +413,7 @@ public class MetadataWebService extends AbstractCurationTask implements Namespac list.add(m.group(2)); } } - return list.toArray(new String[0]); + return list; } protected int getMapIndex(String mapping) { From 674c3e26072ea996f1220d57eac2b6c022a1027c Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Mon, 30 Aug 2021 16:54:48 -0400 Subject: [PATCH 11/97] Remove unnecessary boxing, unread variables. --- .../org/dspace/browse/ItemListConfig.java | 24 ++--------- .../dspace/core/LegacyPluginServiceImpl.java | 42 ++++--------------- .../org/dspace/discovery/SolrServiceImpl.java | 10 +---- 3 files changed, 14 insertions(+), 62 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/browse/ItemListConfig.java b/dspace-api/src/main/java/org/dspace/browse/ItemListConfig.java index 9cbbe8f194..6a63659c82 100644 --- a/dspace-api/src/main/java/org/dspace/browse/ItemListConfig.java +++ b/dspace-api/src/main/java/org/dspace/browse/ItemListConfig.java @@ -25,22 +25,7 @@ public class ItemListConfig { /** * a map of column number to metadata value */ - private Map metadata = new HashMap(); - - /** - * a map of column number to data type - */ - private Map types = new HashMap(); - - /** - * constant for a DATE column - */ - private static final int DATE = 1; - - /** - * constant for a TEXT column - */ - private static final int TEXT = 2; + private Map metadata = new HashMap<>(); private final transient ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); @@ -63,14 +48,11 @@ public class ItemListConfig { // parse the config int i = 1; for (String token : browseFields) { - Integer key = Integer.valueOf(i); + Integer key = i; // find out if the field is a date if (token.indexOf("(date)") > 0) { token = token.replaceAll("\\(date\\)", ""); - types.put(key, Integer.valueOf(ItemListConfig.DATE)); - } else { - types.put(key, Integer.valueOf(ItemListConfig.TEXT)); } String[] mdBits = interpretField(token.trim(), null); @@ -100,7 +82,7 @@ public class ItemListConfig { * @return array of metadata */ public String[] getMetadata(int col) { - return metadata.get(Integer.valueOf(col)); + return metadata.get(col); } /** diff --git a/dspace-api/src/main/java/org/dspace/core/LegacyPluginServiceImpl.java b/dspace-api/src/main/java/org/dspace/core/LegacyPluginServiceImpl.java index 7bbbd91d0a..e92ea137f3 100644 --- a/dspace-api/src/main/java/org/dspace/core/LegacyPluginServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/core/LegacyPluginServiceImpl.java @@ -10,7 +10,6 @@ package org.dspace.core; import java.io.BufferedReader; import java.io.FileReader; import java.io.IOException; -import java.io.Serializable; import java.lang.reflect.Array; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; @@ -173,7 +172,7 @@ public class LegacyPluginServiceImpl implements PluginService { throws PluginInstantiationException { // cache of config data for Sequence Plugins; format its // -> [ .. ] (value is Array) - Map sequenceConfig = new HashMap(); + Map sequenceConfig = new HashMap<>(); // cache the configuration for this interface after grovelling it once: // format is prefix. = @@ -220,10 +219,7 @@ public class LegacyPluginServiceImpl implements PluginService { // Map of named plugin classes, [intfc,name] -> class // Also contains intfc -> "marker" to mark when interface has been loaded. - private Map namedPluginClasses = new HashMap(); - - // Map of cached (reusable) named plugin instances, [class,name] -> instance - private Map namedInstanceCache = new HashMap(); + private final Map namedPluginClasses = new HashMap<>(); // load and cache configuration data for the given interface. private void configureNamedPlugin(String iname) @@ -413,14 +409,14 @@ public class LegacyPluginServiceImpl implements PluginService { String iname = interfaceClass.getName(); configureNamedPlugin(iname); String prefix = iname + SEP; - ArrayList result = new ArrayList(); + ArrayList result = new ArrayList<>(); for (String key : namedPluginClasses.keySet()) { if (key.startsWith(prefix)) { result.add(key.substring(prefix.length())); } } - if (result.size() == 0) { + if (result.isEmpty()) { log.error("Cannot find any names for named plugin, interface=" + iname); } @@ -508,10 +504,10 @@ public class LegacyPluginServiceImpl implements PluginService { */ // tables of config keys for each type of config line: - Map singleKey = new HashMap(); - Map sequenceKey = new HashMap(); - Map namedKey = new HashMap(); - Map selfnamedKey = new HashMap(); + Map singleKey = new HashMap<>(); + Map sequenceKey = new HashMap<>(); + Map namedKey = new HashMap<>(); + Map selfnamedKey = new HashMap<>(); // Find all property keys starting with "plugin." List keys = configurationService.getPropertyKeys("plugin."); @@ -533,7 +529,7 @@ public class LegacyPluginServiceImpl implements PluginService { // 2. Build up list of all interfaces and test that they are loadable. // don't bother testing that they are "interface" rather than "class" // since either one will work for the Plugin Manager. - ArrayList allInterfaces = new ArrayList(); + ArrayList allInterfaces = new ArrayList<>(); allInterfaces.addAll(singleKey.keySet()); allInterfaces.addAll(sequenceKey.keySet()); allInterfaces.addAll(namedKey.keySet()); @@ -547,7 +543,6 @@ public class LegacyPluginServiceImpl implements PluginService { // - each class is loadable. // - plugin.selfnamed values are each subclass of SelfNamedPlugin // - save classname in allImpls - Map allImpls = new HashMap(); // single plugins - just check that it has a valid impl. class ii = singleKey.keySet().iterator(); @@ -558,9 +553,6 @@ public class LegacyPluginServiceImpl implements PluginService { log.error("Single plugin config not found for: " + SINGLE_PREFIX + key); } else { val = val.trim(); - if (checkClassname(val, "implementation class")) { - allImpls.put(val, val); - } } } @@ -571,12 +563,6 @@ public class LegacyPluginServiceImpl implements PluginService { String[] vals = configurationService.getArrayProperty(SEQUENCE_PREFIX + key); if (vals == null || vals.length == 0) { log.error("Sequence plugin config not found for: " + SEQUENCE_PREFIX + key); - } else { - for (String val : vals) { - if (checkClassname(val, "implementation class")) { - allImpls.put(val, val); - } - } } } @@ -591,7 +577,6 @@ public class LegacyPluginServiceImpl implements PluginService { } else { for (String val : vals) { if (checkClassname(val, "selfnamed implementation class")) { - allImpls.put(val, val); checkSelfNamed(val); } } @@ -609,15 +594,6 @@ public class LegacyPluginServiceImpl implements PluginService { log.error("Named plugin config not found for: " + NAMED_PREFIX + key); } else { checkNames(key); - for (String val : vals) { - // each named plugin has two parts to the value, format: - // [classname] = [plugin-name] - String val_split[] = val.split("\\s*=\\s*"); - String classname = val_split[0]; - if (checkClassname(classname, "implementation class")) { - allImpls.put(classname, classname); - } - } } } } 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 0791824085..ec0c18bace 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java @@ -873,7 +873,7 @@ public class SolrServiceImpl implements SearchService, IndexingService { // if we found stale objects we can decide to skip execution of the remaining code to improve performance boolean skipLoadingResponse = false; // use zombieDocs to collect stale found objects - List zombieDocs = new ArrayList(); + List zombieDocs = new ArrayList<>(); QueryResponse solrQueryResponse = solrSearchCore.getSolr().query(solrQuery, solrSearchCore.REQUEST_METHOD); if (solrQueryResponse != null) { @@ -924,12 +924,6 @@ public class SolrServiceImpl implements SearchService, IndexingService { //We need to remove all the "_hl" appendix strings from our keys Map> resultMap = new HashMap<>(); for (String key : highlightedFields.keySet()) { - List highlightOriginalValue = highlightedFields.get(key); - List resultHighlightOriginalValue = new ArrayList<>(); - for (String highlightValue : highlightOriginalValue) { - String[] splitted = highlightValue.split("###"); - resultHighlightOriginalValue.add(splitted); - } resultMap.put(key.substring(0, key.lastIndexOf("_hl")), highlightedFields.get(key)); } @@ -945,7 +939,7 @@ public class SolrServiceImpl implements SearchService, IndexingService { // If any stale entries are found in the current page of results, // we remove those stale entries and rerun the same query again. // Otherwise, the query is valid and the results are returned. - if (zombieDocs.size() != 0) { + if (!zombieDocs.isEmpty()) { log.info("Cleaning " + zombieDocs.size() + " stale objects from Discovery Index"); log.info("ZombieDocs "); zombieDocs.forEach(log::info); From 90a23a000a2c9de696b0cdba36621268f5a96e0d Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Mon, 30 Aug 2021 16:56:09 -0400 Subject: [PATCH 12/97] Fix mismatched getter, setter synchronization. --- .../servicemanager/config/DSpaceConfigurationService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dspace-services/src/main/java/org/dspace/servicemanager/config/DSpaceConfigurationService.java b/dspace-services/src/main/java/org/dspace/servicemanager/config/DSpaceConfigurationService.java index 8bf037cbe3..c20ee19628 100644 --- a/dspace-services/src/main/java/org/dspace/servicemanager/config/DSpaceConfigurationService.java +++ b/dspace-services/src/main/java/org/dspace/servicemanager/config/DSpaceConfigurationService.java @@ -177,7 +177,7 @@ public final class DSpaceConfigurationService implements ConfigurationService { * @see org.dspace.services.ConfigurationService#getProperty(java.lang.String) */ @Override - public String getProperty(String name) { + public synchronized String getProperty(String name) { return getProperty(name, null); } @@ -188,7 +188,7 @@ public final class DSpaceConfigurationService implements ConfigurationService { * @see org.dspace.services.ConfigurationService#getProperty(java.lang.String, java.lang.String) */ @Override - public String getProperty(String name, String defaultValue) { + public synchronized String getProperty(String name, String defaultValue) { return getPropertyAsType(name, defaultValue); } @@ -410,7 +410,7 @@ public final class DSpaceConfigurationService implements ConfigurationService { throw new IllegalArgumentException("properties cannot be null"); } - ArrayList changed = new ArrayList(); + ArrayList changed = new ArrayList<>(); // loop through each new property entry for (Entry entry : properties.entrySet()) { From 758b02f65c92f00954b7db2326cb46aac6bc6b22 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Fri, 25 Mar 2022 17:00:04 -0400 Subject: [PATCH 13/97] [DS-3951] A new class needs to understand multiple recipients. --- .../requestitem/RequestItemEmailNotifier.java | 38 ++++++++++++++----- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemEmailNotifier.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemEmailNotifier.java index d72e42eac1..02054ee1a0 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemEmailNotifier.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemEmailNotifier.java @@ -72,28 +72,48 @@ public class RequestItemEmailNotifier { static public void sendRequest(Context context, RequestItem ri, String responseLink) throws IOException, SQLException { // Who is making this request? - RequestItemAuthor author = requestItemAuthorExtractor + List authors = requestItemAuthorExtractor .getRequestItemAuthor(context, ri.getItem()); - String authorEmail = author.getEmail(); - String authorName = author.getFullName(); // Build an email to the approver. Email email = Email.getEmail(I18nUtil.getEmailFilename(context.getCurrentLocale(), "request_item.author")); - email.addRecipient(authorEmail); + for (RequestItemAuthor author : authors) { + email.addRecipient(author.getEmail()); + } email.setReplyTo(ri.getReqEmail()); // Requester's address + email.addArgument(ri.getReqName()); // {0} Requester's name + email.addArgument(ri.getReqEmail()); // {1} Requester's address + email.addArgument(ri.isAllfiles() // {2} All bitstreams or just one? ? I18nUtil.getMessage("itemRequest.all") : ri.getBitstream().getName()); - email.addArgument(handleService.getCanonicalForm(ri.getItem().getHandle())); + + email.addArgument(handleService.getCanonicalForm(ri.getItem().getHandle())); // {3} + email.addArgument(ri.getItem().getName()); // {4} requested item's title + email.addArgument(ri.getReqMessage()); // {5} message from requester + email.addArgument(responseLink); // {6} Link back to DSpace for action - email.addArgument(authorName); // {7} corresponding author name - email.addArgument(authorEmail); // {8} corresponding author email - email.addArgument(configurationService.getProperty("dspace.name")); - email.addArgument(configurationService.getProperty("mail.helpdesk")); + + StringBuilder names = new StringBuilder(); + StringBuilder addresses = new StringBuilder(); + for (RequestItemAuthor author : authors) { + if (names.length() > 0) { + names.append("; "); + addresses.append("; "); + } + names.append(author.getFullName()); + addresses.append(author.getEmail()); + } + email.addArgument(names.toString()); // {7} corresponding author name + email.addArgument(addresses.toString()); // {8} corresponding author email + + email.addArgument(configurationService.getProperty("dspace.name")); // {9} + + email.addArgument(configurationService.getProperty("mail.helpdesk")); // {10} // Send the email. try { From bd80b0e4c79dee5365a47f38c929a98e99dd835c Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Mon, 28 Mar 2022 10:56:37 -0400 Subject: [PATCH 14/97] [DS-3951] Fix another missed usage. --- .../rest/repository/RequestItemRepository.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index d013566a2c..82f9ed3318 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -15,7 +15,9 @@ import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.sql.SQLException; +import java.util.Collections; import java.util.Date; +import java.util.List; import java.util.UUID; import javax.servlet.http.HttpServletRequest; @@ -229,14 +231,20 @@ public class RequestItemRepository } // Check for authorized user - RequestItemAuthor authorizer; + List authorizers; try { - authorizer = requestItemAuthorExtractor.getRequestItemAuthor(context, ri.getItem()); + authorizers = requestItemAuthorExtractor.getRequestItemAuthor(context, ri.getItem()); } catch (SQLException ex) { LOG.warn("Failed to find an authorizer: {}", ex.getMessage()); - authorizer = new RequestItemAuthor("", ""); + authorizers = Collections.EMPTY_LIST; } - if (!authorizer.getEmail().equals(context.getCurrentUser().getEmail())) { + + boolean authorized = false; + String requester = context.getCurrentUser().getEmail(); + for (RequestItemAuthor authorizer : authorizers) { + authorized |= authorizer.getEmail().equals(requester); + } + if (!authorized) { throw new AuthorizeException("Not authorized to approve this request"); } From 2312724f5e9bbdd413b826f699ac66214e77e019 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Mon, 28 Mar 2022 13:51:38 -0400 Subject: [PATCH 15/97] [DS-3951] Add tests for new strategy classes. --- ...AdministratorsRequestItemStrategyTest.java | 60 +++++++++++++++++++ .../CombiningRequestItemStrategyTest.java | 53 ++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 dspace-api/src/test/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategyTest.java create mode 100644 dspace-api/src/test/java/org/dspace/app/requestitem/CombiningRequestItemStrategyTest.java diff --git a/dspace-api/src/test/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategyTest.java b/dspace-api/src/test/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategyTest.java new file mode 100644 index 0000000000..ffb2e5da46 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/requestitem/CollectionAdministratorsRequestItemStrategyTest.java @@ -0,0 +1,60 @@ +/** + * 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.requestitem; + +import static org.junit.Assert.assertEquals; + +import java.util.List; + +import org.dspace.content.Collection; +import org.dspace.content.Item; +import org.dspace.core.Context; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * + * @author Mark H. Wood + */ +public class CollectionAdministratorsRequestItemStrategyTest { + private static final String NAME = "John Q. Public"; + private static final String EMAIL = "jqpublic@example.com"; + + /** + * Test of getRequestItemAuthor method, of class CollectionAdministratorsRequestItemStrategy. + * @throws java.lang.Exception passed through. + */ + @Test + public void testGetRequestItemAuthor() + throws Exception { + System.out.println("getRequestItemAuthor"); + Context context = null; + + EPerson eperson1 = Mockito.mock(EPerson.class); + Mockito.when(eperson1.getEmail()).thenReturn(EMAIL); + Mockito.when(eperson1.getFullName()).thenReturn(NAME); + + Group group1 = Mockito.mock(Group.class); + Mockito.when(group1.getMembers()).thenReturn(List.of(eperson1)); + + Collection collection1 = Mockito.mock(Collection.class); + Mockito.when(collection1.getAdministrators()).thenReturn(group1); + + Item item = Mockito.mock(Item.class); + Mockito.when(item.getCollections()).thenReturn(List.of(collection1)); + + CollectionAdministratorsRequestItemStrategy instance = new CollectionAdministratorsRequestItemStrategy(); + List result = instance.getRequestItemAuthor(context, + item); + assertEquals("Should be one author", 1, result.size()); + assertEquals("Name should match " + NAME, NAME, result.get(0).getFullName()); + assertEquals("Email should match " + EMAIL, EMAIL, result.get(0).getEmail()); + } +} diff --git a/dspace-api/src/test/java/org/dspace/app/requestitem/CombiningRequestItemStrategyTest.java b/dspace-api/src/test/java/org/dspace/app/requestitem/CombiningRequestItemStrategyTest.java new file mode 100644 index 0000000000..90047ca1bf --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/requestitem/CombiningRequestItemStrategyTest.java @@ -0,0 +1,53 @@ +/** + * 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.requestitem; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder; + +import java.util.List; + +import org.dspace.content.Item; +import org.dspace.core.Context; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * + * @author Mark H. Wood + */ +public class CombiningRequestItemStrategyTest { + /** + * Test of getRequestItemAuthor method, of class CombiningRequestItemStrategy. + * @throws java.lang.Exception passed through. + */ + @Test + public void testGetRequestItemAuthor() + throws Exception { + System.out.println("getRequestItemAuthor"); + Context context = null; + + Item item = Mockito.mock(Item.class); + RequestItemAuthor author1 = new RequestItemAuthor("Pat Paulsen", "ppaulsen@example.com"); + RequestItemAuthor author2 = new RequestItemAuthor("Alfred E. Neuman", "aeneuman@example.com"); + RequestItemAuthor author3 = new RequestItemAuthor("Alias Undercover", "aundercover@example.com"); + + RequestItemAuthorExtractor strategy1 = Mockito.mock(RequestItemHelpdeskStrategy.class); + Mockito.when(strategy1.getRequestItemAuthor(context, item)).thenReturn(List.of(author1)); + + RequestItemAuthorExtractor strategy2 = Mockito.mock(RequestItemHelpdeskStrategy.class); + Mockito.when(strategy2.getRequestItemAuthor(context, item)).thenReturn(List.of(author2, author3)); + + List strategies = List.of(strategy1, strategy2); + + CombiningRequestItemStrategy instance = new CombiningRequestItemStrategy(strategies); + List result = instance.getRequestItemAuthor(context, + item); + assertThat(result, containsInAnyOrder(author1, author2, author3)); + } +} From 2f437b701815623bee4ac954a71fb3d600562574 Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Tue, 29 Mar 2022 16:05:48 -0400 Subject: [PATCH 16/97] [DS-4300] Refactor for testability. --- .../org/dspace/administer/StructBuilder.java | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java b/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java index fcf6fd0c23..6e69f93572 100644 --- a/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java +++ b/dspace-api/src/main/java/org/dspace/administer/StructBuilder.java @@ -117,8 +117,6 @@ public class StructBuilder { protected static final HandleService handleService = HandleServiceFactory.getInstance().getHandleService(); - private static boolean keepHandles; - /** * Default constructor */ @@ -229,8 +227,8 @@ public class StructBuilder { inputStream = new FileInputStream(input); } - keepHandles = options.hasOption("k"); - importStructure(context, inputStream, outputStream); + boolean keepHandles = options.hasOption("k"); + importStructure(context, inputStream, outputStream, keepHandles); // save changes from import context.complete(); } @@ -243,6 +241,7 @@ public class StructBuilder { * @param context * @param input XML which describes the new communities and collections. * @param output input, annotated with the new objects' identifiers. + * @param keepHandles true if Handles should be set from input. * @throws IOException * @throws ParserConfigurationException * @throws SAXException @@ -250,7 +249,7 @@ public class StructBuilder { * @throws SQLException */ static void importStructure(Context context, InputStream input, - OutputStream output) + OutputStream output, boolean keepHandles) throws IOException, ParserConfigurationException, SQLException, TransformerException { @@ -314,7 +313,7 @@ public class StructBuilder { NodeList first = XPathAPI.selectNodeList(document, "/import_structure/community"); // run the import starting with the top level communities - elements = handleCommunities(context, first, null); + elements = handleCommunities(context, first, null, keepHandles); } catch (TransformerException ex) { System.err.format("Input content not understood: %s%n", ex.getMessage()); System.exit(1); @@ -633,10 +632,12 @@ public class StructBuilder { * @param context the context of the request * @param communities a nodelist of communities to create along with their sub-structures * @param parent the parent community of the nodelist of communities to create + * @param keepHandles use Handles from input. * @return an element array containing additional information regarding the * created communities (e.g. the handles they have been assigned) */ - private static Element[] handleCommunities(Context context, NodeList communities, Community parent) + private static Element[] handleCommunities(Context context, NodeList communities, + Community parent, boolean keepHandles) throws TransformerException, SQLException, AuthorizeException { Element[] elements = new Element[communities.getLength()]; @@ -728,11 +729,13 @@ public class StructBuilder { // handle sub communities NodeList subCommunities = XPathAPI.selectNodeList(tn, "community"); - Element[] subCommunityElements = handleCommunities(context, subCommunities, community); + Element[] subCommunityElements = handleCommunities(context, + subCommunities, community, keepHandles); // handle collections NodeList collections = XPathAPI.selectNodeList(tn, "collection"); - Element[] collectionElements = handleCollections(context, collections, community); + Element[] collectionElements = handleCollections(context, + collections, community, keepHandles); int j; for (j = 0; j < subCommunityElements.length; j++) { @@ -757,7 +760,8 @@ public class StructBuilder { * @return an Element array containing additional information about the * created collections (e.g. the handle) */ - private static Element[] handleCollections(Context context, NodeList collections, Community parent) + private static Element[] handleCollections(Context context, + NodeList collections, Community parent, boolean keepHandles) throws TransformerException, SQLException, AuthorizeException { Element[] elements = new Element[collections.getLength()]; From 0e98044e95b0592e4b0501c006c7541fb756840a Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Tue, 29 Mar 2022 16:06:22 -0400 Subject: [PATCH 17/97] [DS-4300] Test with specified handles enabled. --- .../dspace/administer/StructBuilderIT.java | 89 ++++++++++++++++--- 1 file changed, 76 insertions(+), 13 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java b/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java index 7abe3618ed..5bd6144fbe 100644 --- a/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java +++ b/dspace-api/src/test/java/org/dspace/administer/StructBuilderIT.java @@ -8,6 +8,7 @@ package org.dspace.administer; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -18,9 +19,10 @@ import java.sql.SQLException; import java.util.Iterator; import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.Source; -import javax.xml.transform.TransformerException; import javax.xml.transform.stream.StreamSource; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.dspace.AbstractIntegrationTest; import org.dspace.authorize.AuthorizeException; import org.dspace.content.Collection; @@ -29,13 +31,12 @@ import org.dspace.content.MetadataSchemaEnum; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.CollectionService; import org.dspace.content.service.CommunityService; +import org.dspace.handle.Handle; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.w3c.dom.Attr; import org.w3c.dom.Node; import org.xml.sax.SAXException; @@ -53,7 +54,7 @@ import org.xmlunit.diff.Difference; */ public class StructBuilderIT extends AbstractIntegrationTest { - private static final Logger log = LoggerFactory.getLogger(StructBuilderIT.class); + private static final Logger log = LogManager.getLogger(); private static final CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); @@ -93,23 +94,24 @@ public class StructBuilderIT public void tearDown() { } + private static final String COMMUNITY_1_HANDLE = "https://hdl.handle.net/1/1"; /** Test structure document. */ private static final String IMPORT_DOCUMENT = "\n" + "\n" + - " \n" + + " \n" + " Top Community 0\n" + " A top level community\n" + " Testing 1 2 3\n" + " 1969\n" + " A sidebar\n" + - " \n" + + " \n" + " Sub Community 0.0\n" + " A sub community\n" + " Live from New York....\n" + " 1957\n" + " Another sidebar\n" + - " \n" + + " \n" + " Collection 0.0.0\n" + " A collection\n" + " Our next guest needs no introduction\n" + @@ -119,7 +121,7 @@ public class StructBuilderIT " Testing\n" + " \n" + " \n" + - " \n" + + " \n" + " Collection 0.1\n" + " Another collection\n" + " Fourscore and seven years ago\n" + @@ -160,11 +162,7 @@ public class StructBuilderIT byte[] inputBytes = IMPORT_DOCUMENT.getBytes(StandardCharsets.UTF_8); context.turnOffAuthorisationSystem(); try (InputStream input = new ByteArrayInputStream(inputBytes);) { - StructBuilder.importStructure(context, input, outputDocument); - } catch (IOException | SQLException - | ParserConfigurationException | TransformerException ex) { - System.err.println(ex.getMessage()); - System.exit(1); + StructBuilder.importStructure(context, input, outputDocument, false); } finally { context.restoreAuthSystemState(); } @@ -198,6 +196,71 @@ public class StructBuilderIT // TODO spot-check some objects. } + /** + * Test of importStructure method, with given Handles. + * + * @throws java.lang.Exception passed through. + */ + @Test + public void testImportStructureWithHandles() + throws Exception { + System.out.println("importStructure"); + + // Run the method under test and collect its output. + ByteArrayOutputStream outputDocument + = new ByteArrayOutputStream(IMPORT_DOCUMENT.length() * 2 * 2); + byte[] inputBytes = IMPORT_DOCUMENT.getBytes(StandardCharsets.UTF_8); + context.turnOffAuthorisationSystem(); + try (InputStream input = new ByteArrayInputStream(inputBytes);) { + StructBuilder.importStructure(context, input, outputDocument, true); + } finally { + context.restoreAuthSystemState(); + } + + // Check a chosen object for the right Handle. + boolean found = false; + for (Community community : communityService.findAllTop(context)) { + for (Handle handle : community.getHandles()) { + if (handle.getHandle().equals(COMMUNITY_1_HANDLE)) { + found = true; + break; + } + if (found) { + break; + } + } + } + assertTrue("A community should have its specified handle", found); + + // Compare import's output with its input. + // N.B. here we rely on StructBuilder to emit communities and + // collections in the same order as the input document. If that changes, + // we will need a smarter NodeMatcher, probably based on children. + Source output = new StreamSource( + new ByteArrayInputStream(outputDocument.toByteArray())); + Source reference = new StreamSource( + new ByteArrayInputStream( + IMPORT_DOCUMENT.getBytes(StandardCharsets.UTF_8))); + Diff myDiff = DiffBuilder.compare(reference).withTest(output) + .normalizeWhitespace() +// .withNodeFilter(new MyNodeFilter()) + .withAttributeFilter((Attr attr) -> + !attr.getName().equals("identifier")) + .checkForIdentical() + .build(); + + // Was there a difference? + // Always output differences -- one is expected. + ComparisonFormatter formatter = new DefaultComparisonFormatter(); + for (Difference difference : myDiff.getDifferences()) { + System.err.println(difference.toString(formatter)); + } + // Test for *significant* differences. + assertFalse("Output does not match input.", isDifferent(myDiff)); + + // TODO spot-check some objects. + } + /** * Test of exportStructure method, of class StructBuilder. * @throws ParserConfigurationException passed through. From 13b147a691e8f9011640c45cf3623a3bebd458a2 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 11 Apr 2022 14:53:35 +0200 Subject: [PATCH 18/97] [CST-5340] refactored OpenAIREFundingData provider --- .../impl/OpenAIREFundingDataProvider.java | 192 ++++-------------- .../config/spring/api/external-openaire.xml | 68 ++++++- .../config/spring/api/external-openaire.xml | 76 ++++++- 3 files changed, 177 insertions(+), 159 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/external/provider/impl/OpenAIREFundingDataProvider.java b/dspace-api/src/main/java/org/dspace/external/provider/impl/OpenAIREFundingDataProvider.java index 3dcd7d16a6..8ca5b7c0ea 100644 --- a/dspace-api/src/main/java/org/dspace/external/provider/impl/OpenAIREFundingDataProvider.java +++ b/dspace-api/src/main/java/org/dspace/external/provider/impl/OpenAIREFundingDataProvider.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.Base64; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -33,6 +34,7 @@ import org.dspace.content.dto.MetadataValueDTO; import org.dspace.external.OpenAIRERestConnector; import org.dspace.external.model.ExternalDataObject; import org.dspace.external.provider.AbstractExternalDataProvider; +import org.dspace.importer.external.metadatamapping.MetadataFieldConfig; import org.springframework.beans.factory.annotation.Autowired; /** @@ -40,13 +42,9 @@ import org.springframework.beans.factory.annotation.Autowired; * will deal with the OpenAIRE External Data lookup * * @author paulo-graca - * */ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { - /** - * log4j logger - */ private static Logger log = org.apache.logging.log4j.LogManager.getLogger(OpenAIREFundingDataProvider.class); /** @@ -54,6 +52,16 @@ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { */ protected static final String PREFIX = "info:eu-repo/grantAgreement"; + private static final String TITLE = "dcTitle"; + private static final String SUBJECT = "dcSubject"; + private static final String AWARD_URI = "awardURI"; + private static final String FUNDER_NAME = "funderName"; + private static final String SPATIAL = "coverageSpatial"; + private static final String AWARD_NUMBER = "awardNumber"; + private static final String FUNDER_ID = "funderIdentifier"; + private static final String FUNDING_STREAM = "fundingStream"; + private static final String TITLE_ALTERNATIVE = "titleAlternative"; + /** * rows default limit */ @@ -69,11 +77,9 @@ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { */ protected OpenAIRERestConnector connector; - /** - * required method - */ - public void init() throws IOException { - } + protected Map metadataFields; + + public void init() throws IOException {} @Override public String getSourceIdentifier() { @@ -266,14 +272,22 @@ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { } } + public Map getMetadataFields() { + return metadataFields; + } + + public void setMetadataFields(Map metadataFields) { + this.metadataFields = metadataFields; + } + /** * OpenAIRE Funding External Data Builder Class * * @author pgraca - * */ - public static class ExternalDataObjectBuilder { - ExternalDataObject externalDataObject; + public class ExternalDataObjectBuilder { + + private ExternalDataObject externalDataObject; public ExternalDataObjectBuilder(Project project) { String funderIdPrefix = "urn:openaire:"; @@ -283,46 +297,42 @@ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { for (FundingTreeType fundingTree : projectHelper.getFundingTreeTypes()) { FunderType funder = fundingTree.getFunder(); // Funder name - this.addFunderName(funder.getName()); + this.addMetadata(metadataFields.get(FUNDER_NAME), funder.getName()); // Funder Id - convert it to an urn - this.addFunderID(funderIdPrefix + funder.getId()); + this.addMetadata(metadataFields.get(FUNDER_ID), funderIdPrefix + funder.getId()); // Jurisdiction - this.addFunderJuristiction(funder.getJurisdiction()); + this.addMetadata(metadataFields.get(SPATIAL), funder.getJurisdiction()); FundingHelper fundingHelper = new FundingHelper( - fundingTree.getFundingLevel2OrFundingLevel1OrFundingLevel0()); + fundingTree.getFundingLevel2OrFundingLevel1OrFundingLevel0()); // Funding description for (FundingType funding : fundingHelper.getFirstAvailableFunding()) { - this.addFundingStream(funding.getDescription()); + this.addMetadata(metadataFields.get(FUNDING_STREAM), funding.getDescription()); } } // Title for (String title : projectHelper.getTitles()) { - this.addAwardTitle(title); + this.addMetadata(metadataFields.get(TITLE), title); this.setDisplayValue(title); this.setValue(title); } - // Code for (String code : projectHelper.getCodes()) { - this.addAwardNumber(code); + this.addMetadata(metadataFields.get(AWARD_NUMBER), code); } - // Website url for (String url : projectHelper.getWebsiteUrls()) { - this.addAwardURI(url); + this.addMetadata(metadataFields.get(AWARD_URI), url); } - // Acronyms for (String acronym : projectHelper.getAcronyms()) { - this.addFundingItemAcronym(acronym); + this.addMetadata(metadataFields.get(TITLE_ALTERNATIVE), acronym); } - // Keywords for (String keyword : projectHelper.getKeywords()) { - this.addSubject(keyword); + this.addMetadata(metadataFields.get(SUBJECT), keyword); } } @@ -366,7 +376,6 @@ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { * @return ExternalDataObjectBuilder */ public ExternalDataObjectBuilder setId(String id) { - // we use base64 encoding in order to use slashes / and other // characters that must be escaped for the <:entry-id> String base64Id = Base64.getEncoder().encodeToString(id.getBytes()); @@ -374,128 +383,10 @@ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { return this; } - /** - * Add metadata dc.identifier - * - * @param metadata identifier - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addIdentifier(String identifier) { - this.externalDataObject.addMetadata(new MetadataValueDTO("dc", "identifier", null, null, identifier)); - return this; - } - - /** - * Add metadata project.funder.name - * - * @param metadata funderName - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addFunderName(String funderName) { - this.externalDataObject.addMetadata(new MetadataValueDTO("project", "funder", "name", null, funderName)); - return this; - } - - /** - * Add metadata project.funder.identifier - * - * @param metadata funderId - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addFunderID(String funderID) { - this.externalDataObject - .addMetadata(new MetadataValueDTO("project", "funder", "identifier", null, funderID)); - return this; - } - - /** - * Add metadata dc.title - * - * @param metadata awardTitle - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addAwardTitle(String awardTitle) { - this.externalDataObject.addMetadata(new MetadataValueDTO("dc", "title", null, null, awardTitle)); - return this; - } - - /** - * Add metadata oaire.fundingStream - * - * @param metadata fundingStream - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addFundingStream(String fundingStream) { - this.externalDataObject - .addMetadata(new MetadataValueDTO("oaire", "fundingStream", null, null, fundingStream)); - return this; - } - - /** - * Add metadata oaire.awardNumber - * - * @param metadata awardNumber - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addAwardNumber(String awardNumber) { - this.externalDataObject.addMetadata(new MetadataValueDTO("oaire", "awardNumber", null, null, awardNumber)); - return this; - } - - /** - * Add metadata oaire.awardURI - * - * @param metadata websiteUrl - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addAwardURI(String websiteUrl) { - this.externalDataObject.addMetadata(new MetadataValueDTO("oaire", "awardURI", null, null, websiteUrl)); - return this; - } - - /** - * Add metadata dc.title.alternative - * - * @param metadata fundingItemAcronym - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addFundingItemAcronym(String fundingItemAcronym) { - this.externalDataObject - .addMetadata(new MetadataValueDTO("dc", "title", "alternative", null, fundingItemAcronym)); - return this; - } - - /** - * Add metadata dc.coverage.spatial - * - * @param metadata funderJuristiction - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addFunderJuristiction(String funderJuristiction) { - this.externalDataObject - .addMetadata(new MetadataValueDTO("dc", "coverage", "spatial", null, funderJuristiction)); - return this; - } - - /** - * Add metadata dc.description - * - * @param metadata description - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addDescription(String description) { - this.externalDataObject.addMetadata(new MetadataValueDTO("dc", "description", null, null, description)); - return this; - } - - /** - * Add metadata dc.subject - * - * @param metadata subject - * @return ExternalDataObjectBuilder - */ - public ExternalDataObjectBuilder addSubject(String subject) { - this.externalDataObject.addMetadata(new MetadataValueDTO("dc", "subject", null, null, subject)); + public ExternalDataObjectBuilder addMetadata(MetadataFieldConfig metadataField, String value) { + this.externalDataObject.addMetadata(new MetadataValueDTO(metadataField.getSchema(), + metadataField.getElement(), + metadataField.getQualifier(), null, value)); return this; } @@ -508,4 +399,5 @@ public class OpenAIREFundingDataProvider extends AbstractExternalDataProvider { return this.externalDataObject; } } -} + +} \ No newline at end of file diff --git a/dspace-api/src/test/data/dspaceFolder/config/spring/api/external-openaire.xml b/dspace-api/src/test/data/dspaceFolder/config/spring/api/external-openaire.xml index e10d04a16f..f1e6c30d13 100644 --- a/dspace-api/src/test/data/dspaceFolder/config/spring/api/external-openaire.xml +++ b/dspace-api/src/test/data/dspaceFolder/config/spring/api/external-openaire.xml @@ -1,6 +1,10 @@ - @@ -15,11 +19,71 @@ init-method="init"> + Project + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/dspace/config/spring/api/external-openaire.xml b/dspace/config/spring/api/external-openaire.xml index f483ce7210..3ffa39c948 100644 --- a/dspace/config/spring/api/external-openaire.xml +++ b/dspace/config/spring/api/external-openaire.xml @@ -1,6 +1,10 @@ - @@ -10,17 +14,75 @@ - + + + Project - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From f20d2ca3007d8a6019ec2d85bdf8152f10734d58 Mon Sep 17 00:00:00 2001 From: Toni Prieto Date: Tue, 7 Jun 2022 21:27:45 +0200 Subject: [PATCH 19/97] Ignore diacritics in browse by title --- .../src/main/java/org/dspace/browse/BrowseEngine.java | 7 ++++++- .../src/main/java/org/dspace/sort/OrderFormatTitle.java | 2 ++ .../main/java/org/dspace/sort/OrderFormatTitleMarc21.java | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/browse/BrowseEngine.java b/dspace-api/src/main/java/org/dspace/browse/BrowseEngine.java index 099c45d3ad..2510ae301f 100644 --- a/dspace-api/src/main/java/org/dspace/browse/BrowseEngine.java +++ b/dspace-api/src/main/java/org/dspace/browse/BrowseEngine.java @@ -203,7 +203,12 @@ public class BrowseEngine { // get the table name that we are going to be getting our data from dao.setTable(browseIndex.getTableName()); - dao.setStartsWith(StringUtils.lowerCase(scope.getStartsWith())); + if (scope.getBrowseIndex().getDataType().equals(OrderFormat.TITLE)) { + // For browsing by title, apply the same normalization applied to indexed titles + dao.setStartsWith(normalizeJumpToValue(scope.getStartsWith())); + } else { + dao.setStartsWith(StringUtils.lowerCase(scope.getStartsWith())); + } // tell the browse query whether we are ascending or descending on the value dao.setAscending(scope.isAscending()); diff --git a/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitle.java b/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitle.java index eb3586dc61..b745f0719c 100644 --- a/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitle.java +++ b/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitle.java @@ -10,6 +10,7 @@ package org.dspace.sort; import org.dspace.text.filter.DecomposeDiactritics; import org.dspace.text.filter.LowerCaseAndTrim; import org.dspace.text.filter.StandardInitialArticleWord; +import org.dspace.text.filter.StripDiacritics; import org.dspace.text.filter.TextFilter; /** @@ -21,6 +22,7 @@ public class OrderFormatTitle extends AbstractTextFilterOFD { { filters = new TextFilter[] {new StandardInitialArticleWord(), new DecomposeDiactritics(), + new StripDiacritics(), new LowerCaseAndTrim()}; } } diff --git a/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitleMarc21.java b/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitleMarc21.java index 670e5c87e5..fa9ba29725 100644 --- a/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitleMarc21.java +++ b/dspace-api/src/main/java/org/dspace/sort/OrderFormatTitleMarc21.java @@ -10,6 +10,7 @@ package org.dspace.sort; import org.dspace.text.filter.DecomposeDiactritics; import org.dspace.text.filter.LowerCaseAndTrim; import org.dspace.text.filter.MARC21InitialArticleWord; +import org.dspace.text.filter.StripDiacritics; import org.dspace.text.filter.StripLeadingNonAlphaNum; import org.dspace.text.filter.TextFilter; @@ -22,6 +23,7 @@ public class OrderFormatTitleMarc21 extends AbstractTextFilterOFD { { filters = new TextFilter[] {new MARC21InitialArticleWord(), new DecomposeDiactritics(), + new StripDiacritics(), new StripLeadingNonAlphaNum(), new LowerCaseAndTrim()}; } From 115460b94df69ec06f1e0b229708c9432601aad6 Mon Sep 17 00:00:00 2001 From: emanuele Date: Tue, 28 Jun 2022 09:14:18 +0200 Subject: [PATCH 20/97] [CST-6201] added NPE checks in getItemsWithPossibleChangesBefore method --- .../src/main/java/org/dspace/xoai/app/XOAI.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java b/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java index 700105899a..bde480ce12 100644 --- a/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java +++ b/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java @@ -8,6 +8,7 @@ package org.dspace.xoai.app; import static com.lyncode.xoai.dataprovider.core.Granularity.Second; +import static java.util.Objects.nonNull; import static org.dspace.xoai.util.ItemUtils.retrieveMetadata; import java.io.ByteArrayOutputStream; @@ -21,9 +22,11 @@ import java.util.Date; import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Objects; import java.util.UUID; import javax.xml.stream.XMLStreamException; +import com.carrotsearch.hppc.ObjectObjectScatterMap; import com.lyncode.xoai.dataprovider.exceptions.ConfigurationException; import com.lyncode.xoai.dataprovider.exceptions.WritingXmlException; import com.lyncode.xoai.dataprovider.xml.XmlOutputContext; @@ -223,8 +226,14 @@ public class XOAI { for (int i = 0; i < documents.getNumFound(); i++) { Item item = itemService.find(context, UUID.fromString((String) documents.get(i).getFieldValue("item.id"))); - if (item.getLastModified().before(last)) { - items.add(item); + if (nonNull(item)) { + if (nonNull(item.getLastModified())) { + if (item.getLastModified().before(last)) { + items.add(item); + } + } else { + log.warn("Skipping item with id " + item.getID()); + } } } return items.iterator(); From d1019740dbad5e115b7c1d5986850065710b7ac9 Mon Sep 17 00:00:00 2001 From: emanuele Date: Tue, 28 Jun 2022 16:19:18 +0200 Subject: [PATCH 21/97] [CST-6201] implemented solr cursors to avoid IndexOutOfBoundsException --- .../main/java/org/dspace/xoai/app/XOAI.java | 56 ++++++++++++++----- 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java b/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java index bde480ce12..9da66815c0 100644 --- a/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java +++ b/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java @@ -9,6 +9,9 @@ package org.dspace.xoai.app; import static com.lyncode.xoai.dataprovider.core.Granularity.Second; import static java.util.Objects.nonNull; +import static org.apache.commons.lang.StringUtils.EMPTY; +import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_PARAM; +import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_START; import static org.dspace.xoai.util.ItemUtils.retrieveMetadata; import java.io.ByteArrayOutputStream; @@ -22,11 +25,9 @@ import java.util.Date; import java.util.Iterator; import java.util.LinkedList; import java.util.List; -import java.util.Objects; import java.util.UUID; import javax.xml.stream.XMLStreamException; -import com.carrotsearch.hppc.ObjectObjectScatterMap; import com.lyncode.xoai.dataprovider.exceptions.ConfigurationException; import com.lyncode.xoai.dataprovider.exceptions.WritingXmlException; import com.lyncode.xoai.dataprovider.xml.XmlOutputContext; @@ -35,14 +36,18 @@ import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; import org.apache.commons.cli.DefaultParser; import org.apache.commons.cli.Options; +import org.apache.commons.lang.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.SolrQuery.ORDER; import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.params.CursorMarkParams; import org.dspace.authorize.ResourcePolicy; import org.dspace.authorize.factory.AuthorizeServiceFactory; import org.dspace.authorize.service.AuthorizeService; @@ -80,6 +85,9 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext public class XOAI { private static Logger log = LogManager.getLogger(XOAI.class); + // needed because the solr query only returns 10 rows by default + private static final int ROW_NUM = 30000; + private final Context context; private boolean optimize; private final boolean verbose; @@ -220,24 +228,44 @@ public class XOAI { */ private Iterator getItemsWithPossibleChangesBefore(Date last) throws DSpaceSolrIndexerException, IOException { try { - SolrQuery params = new SolrQuery("item.willChangeStatus:true").addField("item.id"); - SolrDocumentList documents = DSpaceSolrSearch.query(solrServerResolver.getServer(), params); + SolrQuery params = new SolrQuery("item.willChangeStatus:true").addField("item.id").setRows(100) + .addSort("item.handle", SolrQuery.ORDER.asc); + SolrClient solrClient = solrServerResolver.getServer(); + List items = new LinkedList<>(); - for (int i = 0; i < documents.getNumFound(); i++) { - Item item = itemService.find(context, - UUID.fromString((String) documents.get(i).getFieldValue("item.id"))); - if (nonNull(item)) { - if (nonNull(item.getLastModified())) { - if (item.getLastModified().before(last)) { - items.add(item); + boolean done = false; + /* + * Using solr cursors to paginate and prevent the query from returning 10 + * SolrDocument objects only. + */ + String cursorMark = CURSOR_MARK_START; + String nextCursorMark = EMPTY; + + while (!done) { + params.set(CURSOR_MARK_PARAM, cursorMark); + QueryResponse response = solrClient.query(params); + nextCursorMark = response.getNextCursorMark(); + + for (SolrDocument document : response.getResults()) { + Item item = itemService.find(context, UUID.fromString((String) document.getFieldValue("item.id"))); + if (nonNull(item)) { + if (nonNull(item.getLastModified())) { + if (item.getLastModified().before(last)) { + items.add(item); + } + } else { + log.warn("Skipping item with id " + item.getID()); } - } else { - log.warn("Skipping item with id " + item.getID()); } } + + if (cursorMark.equals(nextCursorMark)) { + done = true; + } + cursorMark = nextCursorMark; } return items.iterator(); - } catch (SolrServerException | SQLException | DSpaceSolrException ex) { + } catch (SolrServerException | SQLException ex) { throw new DSpaceSolrIndexerException(ex.getMessage(), ex); } } From 1fa7049b7d7438a7b82bd5c958812f4f627681b6 Mon Sep 17 00:00:00 2001 From: emanuele Date: Tue, 28 Jun 2022 16:42:26 +0200 Subject: [PATCH 22/97] [CST-6201] fixed checkstyle violations --- .../main/java/org/dspace/xoai/app/XOAI.java | 154 +++++++----------- 1 file changed, 63 insertions(+), 91 deletions(-) diff --git a/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java b/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java index 9da66815c0..e27a3ee947 100644 --- a/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java +++ b/dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java @@ -36,7 +36,6 @@ import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; import org.apache.commons.cli.DefaultParser; import org.apache.commons.cli.Options; -import org.apache.commons.lang.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.solr.client.solrj.SolrClient; @@ -47,7 +46,6 @@ import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.common.SolrDocument; import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.common.params.CursorMarkParams; import org.dspace.authorize.ResourcePolicy; import org.dspace.authorize.factory.AuthorizeServiceFactory; import org.dspace.authorize.service.AuthorizeService; @@ -86,8 +84,6 @@ public class XOAI { private static Logger log = LogManager.getLogger(XOAI.class); // needed because the solr query only returns 10 rows by default - private static final int ROW_NUM = 30000; - private final Context context; private boolean optimize; private final boolean verbose; @@ -105,8 +101,8 @@ public class XOAI { private final AuthorizeService authorizeService; private final ItemService itemService; - private final static ConfigurationService configurationService = DSpaceServicesFactory - .getInstance().getConfigurationService(); + private final static ConfigurationService configurationService = DSpaceServicesFactory.getInstance() + .getConfigurationService(); private List extensionPlugins; @@ -163,9 +159,8 @@ public class XOAI { System.out.println("Using full import."); result = this.indexAll(); } else { - SolrQuery solrParams = new SolrQuery("*:*") - .addField("item.lastmodified") - .addSort("item.lastmodified", ORDER.desc).setRows(1); + SolrQuery solrParams = new SolrQuery("*:*").addField("item.lastmodified") + .addSort("item.lastmodified", ORDER.desc).setRows(1); SolrDocumentList results = DSpaceSolrSearch.query(solrServerResolver.getServer(), solrParams); if (results.getNumFound() == 0) { @@ -178,7 +173,6 @@ public class XOAI { } solrServerResolver.getServer().commit(); - if (optimize) { println("Optimizing Index"); solrServerResolver.getServer().optimize(); @@ -194,12 +188,10 @@ public class XOAI { } private int index(Date last) throws DSpaceSolrIndexerException, IOException { - System.out - .println("Incremental import. Searching for documents modified after: " - + last.toString()); + System.out.println("Incremental import. Searching for documents modified after: " + last.toString()); /* - * Index all changed or new items or items whose visibility is viable to - * change due to an embargo. + * Index all changed or new items or items whose visibility is viable to change + * due to an embargo. */ try { Iterator discoverableChangedItems = itemService @@ -215,15 +207,13 @@ public class XOAI { } /** - * Get all items already in the index which are viable to change visibility - * due to an embargo. Only consider those which haven't been modified - * anyways since the last update, so they aren't updated twice in one import - * run. + * Get all items already in the index which are viable to change visibility due + * to an embargo. Only consider those which haven't been modified anyways since + * the last update, so they aren't updated twice in one import run. * - * @param last - * maximum date for an item to be considered for an update - * @return Iterator over list of items which might have changed their - * visibility since the last update. + * @param last maximum date for an item to be considered for an update + * @return Iterator over list of items which might have changed their visibility + * since the last update. * @throws DSpaceSolrIndexerException */ private Iterator getItemsWithPossibleChangesBefore(Date last) throws DSpaceSolrIndexerException, IOException { @@ -245,7 +235,7 @@ public class XOAI { params.set(CURSOR_MARK_PARAM, cursorMark); QueryResponse response = solrClient.query(params); nextCursorMark = response.getNextCursorMark(); - + for (SolrDocument document : response.getResults()) { Item item = itemService.find(context, UUID.fromString((String) document.getFieldValue("item.id"))); if (nonNull(item)) { @@ -258,7 +248,7 @@ public class XOAI { } } } - + if (cursorMark.equals(nextCursorMark)) { done = true; } @@ -287,11 +277,10 @@ public class XOAI { } /** - * Check if an item is already indexed. Using this, it is possible to check - * if withdrawn or nondiscoverable items have to be indexed at all. + * Check if an item is already indexed. Using this, it is possible to check if + * withdrawn or nondiscoverable items have to be indexed at all. * - * @param item - * Item that should be checked for its presence in the index. + * @param item Item that should be checked for its presence in the index. * @return has it been indexed? */ private boolean checkIfIndexed(Item item) throws IOException { @@ -303,11 +292,11 @@ public class XOAI { return false; } } - /** + + /** * Check if an item is flagged visible in the index. * - * @param item - * Item that should be checked for its presence in the index. + * @param item Item that should be checked for its presence in the index. * @return has it been indexed? */ private boolean checkIfVisibleInOAI(Item item) throws IOException { @@ -324,8 +313,7 @@ public class XOAI { } } - private int index(Iterator iterator) - throws DSpaceSolrIndexerException { + private int index(Iterator iterator) throws DSpaceSolrIndexerException { try { int i = 0; int batchSize = configurationService.getIntProperty("oai.import.batch.size", 1000); @@ -339,7 +327,7 @@ public class XOAI { } else { list.add(this.index(item)); } - //Uncache the item to keep memory consumption low + // Uncache the item to keep memory consumption low context.uncacheEntity(item); } catch (SQLException | IOException | XMLStreamException | WritingXmlException ex) { @@ -371,12 +359,11 @@ public class XOAI { } /** - * Method to get the most recent date on which the item changed concerning - * the OAI deleted status (policy start and end dates for all anonymous READ + * Method to get the most recent date on which the item changed concerning the + * OAI deleted status (policy start and end dates for all anonymous READ * policies and the standard last modification date) * - * @param item - * Item + * @param item Item * @return date * @throws SQLException */ @@ -419,17 +406,16 @@ public class XOAI { boolean isIndexed = this.checkIfIndexed(item); /* - * If the item is not under embargo, it should be visible. If it is, - * make it invisible if this is the first time it is indexed. For - * subsequent index runs, keep the current status, so that if the item - * is embargoed again, it is flagged as deleted instead and does not - * just disappear, or if it is still under embargo, it won't become - * visible and be known to harvesters as deleted before it gets - * disseminated for the first time. The item has to be indexed directly - * after publication even if it is still embargoed, because its - * lastModified date will not change when the embargo end date (or start - * date) is reached. To circumvent this, an item which will change its - * status in the future will be marked as such. + * If the item is not under embargo, it should be visible. If it is, make it + * invisible if this is the first time it is indexed. For subsequent index runs, + * keep the current status, so that if the item is embargoed again, it is + * flagged as deleted instead and does not just disappear, or if it is still + * under embargo, it won't become visible and be known to harvesters as deleted + * before it gets disseminated for the first time. The item has to be indexed + * directly after publication even if it is still embargoed, because its + * lastModified date will not change when the embargo end date (or start date) + * is reached. To circumvent this, an item which will change its status in the + * future will be marked as such. */ boolean isPublic = isEmbargoed ? (isIndexed ? isCurrentlyVisible : false) : true; @@ -441,33 +427,31 @@ public class XOAI { doc.addField("item.willChangeStatus", willChangeStatus(item)); /* - * Mark an item as deleted not only if it is withdrawn, but also if it - * is made private, because items should not simply disappear from OAI - * with a transient deletion policy. Do not set the flag for still - * invisible embargoed items, because this will override the item.public - * flag. + * Mark an item as deleted not only if it is withdrawn, but also if it is made + * private, because items should not simply disappear from OAI with a transient + * deletion policy. Do not set the flag for still invisible embargoed items, + * because this will override the item.public flag. */ doc.addField("item.deleted", (item.isWithdrawn() || !item.isDiscoverable() || (isEmbargoed ? isPublic : false))); /* - * An item that is embargoed will potentially not be harvested by - * incremental harvesters if the from and until params do not encompass - * both the standard lastModified date and the anonymous-READ resource - * policy start date. The same is true for the end date, where - * harvesters might not get a tombstone record. Therefore, consider all - * relevant policy dates and the standard lastModified date and take the - * most recent of those which have already passed. + * An item that is embargoed will potentially not be harvested by incremental + * harvesters if the from and until params do not encompass both the standard + * lastModified date and the anonymous-READ resource policy start date. The same + * is true for the end date, where harvesters might not get a tombstone record. + * Therefore, consider all relevant policy dates and the standard lastModified + * date and take the most recent of those which have already passed. */ - doc.addField("item.lastmodified", SolrUtils.getDateFormatter() - .format(this.getMostRecentModificationDate(item))); + doc.addField("item.lastmodified", + SolrUtils.getDateFormatter().format(this.getMostRecentModificationDate(item))); if (item.getSubmitter() != null) { doc.addField("item.submitter", item.getSubmitter().getEmail()); } - for (Collection col: item.getCollections()) { + for (Collection col : item.getCollections()) { doc.addField("item.collections", "col_" + col.getHandle().replace("/", "_")); } for (Community com : collectionsService.flatParentCommunities(context, item)) { @@ -494,8 +478,7 @@ public class XOAI { // Message output before processing - for debugging purposes if (verbose) { - println(String.format("Item %s with handle %s is about to be indexed", - item.getID().toString(), handle)); + println(String.format("Item %s with handle %s is about to be indexed", item.getID().toString(), handle)); } ByteArrayOutputStream out = new ByteArrayOutputStream(); @@ -513,8 +496,7 @@ public class XOAI { doc.addField("item.compile", out.toString()); if (verbose) { - println(String.format("Item %s with handle %s indexed", - item.getID().toString(), handle)); + println(String.format("Item %s with handle %s indexed", item.getID().toString(), handle)); } return doc; @@ -547,12 +529,10 @@ public class XOAI { return pub; } - private static boolean getKnownExplanation(Throwable t) { if (t instanceof ConnectException) { - System.err.println("Solr server (" - + configurationService.getProperty("oai.solr.url", "") - + ") is down, turn it on."); + System.err.println( + "Solr server (" + configurationService.getProperty("oai.solr.url", "") + ") is down, turn it on."); return true; } @@ -581,7 +561,7 @@ public class XOAI { } private static void cleanCache(XOAIItemCacheService xoaiItemCacheService, XOAICacheService xoaiCacheService) - throws IOException { + throws IOException { System.out.println("Purging cached OAI responses."); xoaiItemCacheService.deleteAll(); xoaiCacheService.deleteAll(); @@ -594,10 +574,8 @@ public class XOAI { public static void main(String[] argv) throws IOException, ConfigurationException { - - AnnotationConfigApplicationContext applicationContext = new AnnotationConfigApplicationContext(new Class[] { - BasicConfiguration.class - }); + AnnotationConfigApplicationContext applicationContext = new AnnotationConfigApplicationContext( + new Class[] { BasicConfiguration.class }); XOAICacheService cacheService = applicationContext.getBean(XOAICacheService.class); XOAIItemCacheService itemCacheService = applicationContext.getBean(XOAIItemCacheService.class); @@ -608,21 +586,19 @@ public class XOAI { CommandLineParser parser = new DefaultParser(); Options options = new Options(); options.addOption("c", "clear", false, "Clear index before indexing"); - options.addOption("o", "optimize", false, - "Optimize index at the end"); + options.addOption("o", "optimize", false, "Optimize index at the end"); options.addOption("v", "verbose", false, "Verbose output"); options.addOption("h", "help", false, "Shows some help"); options.addOption("n", "number", true, "FOR DEVELOPMENT MUST DELETE"); CommandLine line = parser.parse(options, argv); - String[] validSolrCommands = {COMMAND_IMPORT, COMMAND_CLEAN_CACHE}; - String[] validDatabaseCommands = {COMMAND_CLEAN_CACHE, COMMAND_COMPILE_ITEMS, COMMAND_ERASE_COMPILED_ITEMS}; - + String[] validSolrCommands = { COMMAND_IMPORT, COMMAND_CLEAN_CACHE }; + String[] validDatabaseCommands = { COMMAND_CLEAN_CACHE, COMMAND_COMPILE_ITEMS, + COMMAND_ERASE_COMPILED_ITEMS }; boolean solr = true; // Assuming solr by default solr = !("database").equals(configurationService.getProperty("oai.storage", "solr")); - boolean run = false; if (line.getArgs().length > 0) { if (solr) { @@ -644,10 +620,7 @@ public class XOAI { if (COMMAND_IMPORT.equals(command)) { ctx = new Context(Context.Mode.READ_ONLY); - XOAI indexer = new XOAI(ctx, - line.hasOption('o'), - line.hasOption('c'), - line.hasOption('v')); + XOAI indexer = new XOAI(ctx, line.hasOption('o'), line.hasOption('c'), line.hasOption('v')); applicationContext.getAutowireCapableBeanFactory().autowireBean(indexer); @@ -672,8 +645,7 @@ public class XOAI { } System.out.println("OAI 2.0 manager action ended. It took " - + ((System.currentTimeMillis() - start) / 1000) - + " seconds."); + + ((System.currentTimeMillis() - start) / 1000) + " seconds."); } else { usage(); } @@ -725,7 +697,7 @@ public class XOAI { private static void usage() { boolean solr = true; // Assuming solr by default - solr = !("database").equals(configurationService.getProperty("oai.storage","solr")); + solr = !("database").equals(configurationService.getProperty("oai.storage", "solr")); if (solr) { System.out.println("OAI Manager Script"); From f9d6091c2cf27dde94e82c1b41a6b74f7c91b4f4 Mon Sep 17 00:00:00 2001 From: Ben Bosman Date: Wed, 6 Jul 2022 13:52:49 +0200 Subject: [PATCH 23/97] Refactor IPTable --- .../service/impl/ClientInfoServiceImpl.java | 2 +- .../org/dspace/statistics/util/IPTable.java | 238 +++++++++--------- 2 files changed, 119 insertions(+), 121 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java b/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java index f63a7a4f91..01fc6d4e72 100644 --- a/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java @@ -139,7 +139,7 @@ public class ClientInfoServiceImpl implements ClientInfoService { // If our IPTable is not empty, log the trusted proxies and return it if (!ipTable.isEmpty()) { - log.info("Trusted proxies (configure via 'proxies.trusted.ipranges'): {}", ipTable.toSet().toString()); + log.info("Trusted proxies (configure via 'proxies.trusted.ipranges'): {}", ipTable.toString()); return ipTable; } else { return null; diff --git a/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java b/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java index 139b75e8cf..b3ae961d35 100644 --- a/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java +++ b/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java @@ -7,9 +7,10 @@ */ package org.dspace.statistics.util; -import java.util.HashMap; +import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.HashSet; -import java.util.Map; +import java.util.Iterator; import java.util.Set; import org.apache.logging.log4j.LogManager; @@ -25,8 +26,32 @@ public class IPTable { private static final Logger log = LogManager.getLogger(IPTable.class); /* A lookup tree for IP addresses and SubnetRanges */ - private final Map>>> map - = new HashMap<>(); + private final Set ipRanges = new HashSet<>(); + + /** + * Internal class representing an IP range + */ + static class IPRange { + + /* Lowest address in the range */ + private final long ipLo; + + /* Highest address in the range */ + private final long ipHi; + + IPRange(long ipLo, long ipHi) { + this.ipLo = ipLo; + this.ipHi = ipHi; + } + + public long getIpLo() { + return ipLo; + } + + public long getIpHi() { + return ipHi; + } + } /** * Can be full v4 IP, subnet or range string. @@ -45,155 +70,114 @@ public class IPTable { */ public void add(String ip) throws IPFormatException { - String[] start; + String start; - String[] end; + String end; String[] range = ip.split("-"); - if (range.length >= 2) { + if (range.length == 2) { - start = range[0].trim().split("/")[0].split("\\."); - end = range[1].trim().split("/")[0].split("\\."); + start = range[0].trim(); + end = range[1].trim(); - if (start.length != 4 || end.length != 4) { - throw new IPFormatException(ip + " - Ranges need to be full IPv4 Addresses"); + try { + long ipLo = ipToLong(InetAddress.getByName(start)); + long ipHi = ipToLong(InetAddress.getByName(end)); + ipRanges.add(new IPRange(ipLo, ipHi)); + return; + } catch (UnknownHostException e) { + throw new IPFormatException(ip + " - Range format should be similar to 1.2.3.0-1.2.3.255"); } - if (!(start[0].equals(end[0]) && start[1].equals(end[1]) && start[2].equals(end[2]))) { - throw new IPFormatException(ip + " - Ranges can only be across the last subnet x.y.z.0 - x.y.z.254"); - } - - } else { + } else if (ip.contains("/")) { //need to ignore CIDR notation for the moment. //ip = ip.split("\\/")[0]; - String[] subnets = ip.split("\\."); - - if (subnets.length < 3) { - throw new IPFormatException(ip + " - require at least three subnet places (255.255.255.0"); - } - - start = subnets; - end = subnets; - } - - if (start.length >= 3) { - Map>> first = map.get(start[0]); - - if (first == null) { - first = new HashMap<>(); - map.put(start[0], first); - } - - Map> second = first.get(start[1]); - - if (second == null) { - second = new HashMap<>(); - first.put(start[1], second); - } - - Set third = second.get(start[2]); - - if (third == null) { - third = new HashSet<>(); - second.put(start[2], third); - } - - //now populate fourth place (* or value 0-254); - - if (start.length == 3) { - third.add("*"); - } - - if (third.contains("*")) { - return; - } - - if (start.length >= 4) { - int s = Integer.valueOf(start[3]); - int e = Integer.valueOf(end[3]); - for (int i = s; i <= e; i++) { - third.add(String.valueOf(i)); + String[] parts = ip.split("/"); + try { + byte[] octets = InetAddress.getByName(parts[0]).getAddress(); + long result = 0; + for (byte octet : octets) { + result <<= 8; + result |= octet & 0xff; } + long mask = (long) Math.pow(2, 32 - Integer.parseInt(parts[1])); + long ipLo = (result / mask) * mask; + long ipHi = (( (result / mask) + 1) * mask) - 1; + ipRanges.add(new IPRange(ipLo, ipHi)); + return; + } catch (Exception e) { + throw new IPFormatException(ip + " - Range format should be similar to 172.16.0.0/12"); + } + + } else { + try { + long ipLo = ipToLong(InetAddress.getByName(ip)); + ipRanges.add(new IPRange(ipLo, ipLo)); + return; + } catch (UnknownHostException e) { + throw new IPFormatException(ip + " - IP address format should be similar to 1.2.3.14"); } } } + public static long ipToLong(InetAddress ip) { + byte[] octets = ip.getAddress(); + long result = 0; + for (byte octet : octets) { + result <<= 8; + result |= octet & 0xff; + } + return result; + } + + public static String longToIp(long ip) { + long part = ip; + String[] parts = new String[4]; + for (int i = 0; i < 4; i++) { + long octet = part & 0xff; + parts[3-i] = String.valueOf(octet); + part = part / 256; + } + + return parts[0]+"."+parts[1]+"."+parts[2]+"."+parts[3]; + } + /** * Check whether a given address is contained in this netblock. * * @param ip the address to be tested * @return true if {@code ip} is within this table's limits. Returns false - * if {@link ip} looks like an IPv6 address. + * if {@code ip} looks like an IPv6 address. * @throws IPFormatException Exception Class to deal with IPFormat errors. */ public boolean contains(String ip) throws IPFormatException { - String[] subnets = ip.split("\\."); - - // Does it look like IPv6? - if (subnets.length > 4 || ip.contains("::")) { - log.warn("Address {} assumed not to match. IPv6 is not implemented.", ip); - return false; + try { + long ipToTest = ipToLong(InetAddress.getByName(ip)); + return ipRanges.stream() + .anyMatch(ipRange -> (ipToTest >= ipRange.getIpLo() && ipToTest <= ipRange.getIpHi())); + } catch (UnknownHostException e) { + throw new IPFormatException("ip not valid"); } - - // Does it look like a subnet? - if (subnets.length < 4) { - throw new IPFormatException("needs to be a single IP address"); - } - - Map>> first = map.get(subnets[0]); - - if (first == null) { - return false; - } - - Map> second = first.get(subnets[1]); - - if (second == null) { - return false; - } - - Set third = second.get(subnets[2]); - - if (third == null) { - return false; - } - - return third.contains(subnets[3]) || third.contains("*"); - } /** - * Convert to a Set. + * Convert to a Set. This set contains all IPs in the range * * @return this table's content as a Set */ public Set toSet() { HashSet set = new HashSet<>(); - for (Map.Entry>>> first : map.entrySet()) { - String firstString = first.getKey(); - Map>> secondMap = first.getValue(); - - for (Map.Entry>> second : secondMap.entrySet()) { - String secondString = second.getKey(); - Map> thirdMap = second.getValue(); - - for (Map.Entry> third : thirdMap.entrySet()) { - String thirdString = third.getKey(); - Set fourthSet = third.getValue(); - - if (fourthSet.contains("*")) { - set.add(firstString + "." + secondString + "." + thirdString); - } else { - for (String fourth : fourthSet) { - set.add(firstString + "." + secondString + "." + thirdString + "." + fourth); - } - } - - } + Iterator ipRangeIterator = ipRanges.iterator(); + while (ipRangeIterator.hasNext()) { + IPRange ipRange = ipRangeIterator.next(); + long ipLo = ipRange.getIpLo(); + long ipHi = ipRange.getIpHi(); + for (long ip = ipLo; ip <= ipHi; ip++) { + set.add(longToIp(ip).toString()); } } @@ -205,7 +189,7 @@ public class IPTable { * @return true if empty, false otherwise */ public boolean isEmpty() { - return map.isEmpty(); + return ipRanges.isEmpty(); } /** @@ -217,5 +201,19 @@ public class IPTable { } } - + @Override + public String toString() { + StringBuilder stringBuilder = new StringBuilder(); + Iterator ipRangeIterator = ipRanges.iterator(); + while (ipRangeIterator.hasNext()) { + IPRange ipRange = ipRangeIterator.next(); + stringBuilder.append(longToIp(ipRange.getIpLo())) + .append("-") + .append(longToIp(ipRange.getIpHi())); + if (ipRangeIterator.hasNext()) { + stringBuilder.append(", "); + } + } + return stringBuilder.toString(); + } } From 36748cd27d7878207863668f310f712051d1263c Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 11 Jul 2022 12:24:38 +0200 Subject: [PATCH 24/97] 92917: Javadocs & minor fixes --- .../service/impl/ClientInfoServiceImpl.java | 2 +- .../org/dspace/statistics/util/IPTable.java | 31 +++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java b/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java index 01fc6d4e72..f4c8818e02 100644 --- a/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/service/impl/ClientInfoServiceImpl.java @@ -139,7 +139,7 @@ public class ClientInfoServiceImpl implements ClientInfoService { // If our IPTable is not empty, log the trusted proxies and return it if (!ipTable.isEmpty()) { - log.info("Trusted proxies (configure via 'proxies.trusted.ipranges'): {}", ipTable.toString()); + log.info("Trusted proxies (configure via 'proxies.trusted.ipranges'): {}", ipTable); return ipTable; } else { return null; diff --git a/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java b/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java index b3ae961d35..4fad3cf383 100644 --- a/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java +++ b/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java @@ -44,10 +44,18 @@ public class IPTable { this.ipHi = ipHi; } + /** + * Get the lowest address in the range + * @return the lowest address as a long integer + */ public long getIpLo() { return ipLo; } + /** + * Get the highest address in the range + * @return the highest address as a long integer + */ public long getIpHi() { return ipHi; } @@ -91,9 +99,6 @@ public class IPTable { } } else if (ip.contains("/")) { - //need to ignore CIDR notation for the moment. - //ip = ip.split("\\/")[0]; - String[] parts = ip.split("/"); try { byte[] octets = InetAddress.getByName(parts[0]).getAddress(); @@ -122,6 +127,11 @@ public class IPTable { } } + /** + * Convert an IP address to a long integer + * @param ip the IP address + * @return + */ public static long ipToLong(InetAddress ip) { byte[] octets = ip.getAddress(); long result = 0; @@ -132,16 +142,21 @@ public class IPTable { return result; } + /** + * Convert a long integer into an IP address string + * @param ip the IP address as a long integer + * @return + */ public static String longToIp(long ip) { long part = ip; String[] parts = new String[4]; for (int i = 0; i < 4; i++) { long octet = part & 0xff; - parts[3-i] = String.valueOf(octet); + parts[3 - i] = String.valueOf(octet); part = part / 256; } - return parts[0]+"."+parts[1]+"."+parts[2]+"."+parts[3]; + return parts[0] + "." + parts[1] + "." + parts[2] + "." + parts[3]; } /** @@ -177,7 +192,7 @@ public class IPTable { long ipLo = ipRange.getIpLo(); long ipHi = ipRange.getIpHi(); for (long ip = ipLo; ip <= ipHi; ip++) { - set.add(longToIp(ip).toString()); + set.add(longToIp(ip)); } } @@ -201,6 +216,10 @@ public class IPTable { } } + /** + * Represent this IP table as a string + * @return a string containing all IP ranges in this IP table + */ @Override public String toString() { StringBuilder stringBuilder = new StringBuilder(); From 01d5bf4f4a71c9eb419aaa50a2af5e8d0cfae38f Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Mon, 11 Jul 2022 14:48:24 +0200 Subject: [PATCH 25/97] 92917: Support implicit ranges & update unit tests --- .../org/dspace/statistics/util/IPTable.java | 57 ++++++++++------- .../dspace/statistics/util/IPTableTest.java | 61 ++++++++++++++++--- 2 files changed, 86 insertions(+), 32 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java b/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java index 4fad3cf383..24420c7a95 100644 --- a/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java +++ b/dspace-api/src/main/java/org/dspace/statistics/util/IPTable.java @@ -13,6 +13,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Set; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -98,31 +99,41 @@ public class IPTable { throw new IPFormatException(ip + " - Range format should be similar to 1.2.3.0-1.2.3.255"); } - } else if (ip.contains("/")) { - String[] parts = ip.split("/"); - try { - byte[] octets = InetAddress.getByName(parts[0]).getAddress(); - long result = 0; - for (byte octet : octets) { - result <<= 8; - result |= octet & 0xff; - } - long mask = (long) Math.pow(2, 32 - Integer.parseInt(parts[1])); - long ipLo = (result / mask) * mask; - long ipHi = (( (result / mask) + 1) * mask) - 1; - ipRanges.add(new IPRange(ipLo, ipHi)); - return; - } catch (Exception e) { - throw new IPFormatException(ip + " - Range format should be similar to 172.16.0.0/12"); + } else { + // Convert implicit ranges to netmask format + // 192 -> 192.0.0.0/8 + // 192.168 -> 192.168.0.0/16 + // 192.168.1 -> 192.168.1.0/24 + int periods = StringUtils.countMatches(ip, '.'); + if (periods < 3) { + ip = StringUtils.join(ip, StringUtils.repeat(".0", 4 - periods - 1), "/", (periods + 1) * 8); } - } else { - try { - long ipLo = ipToLong(InetAddress.getByName(ip)); - ipRanges.add(new IPRange(ipLo, ipLo)); - return; - } catch (UnknownHostException e) { - throw new IPFormatException(ip + " - IP address format should be similar to 1.2.3.14"); + if (ip.contains("/")) { + String[] parts = ip.split("/"); + try { + byte[] octets = InetAddress.getByName(parts[0]).getAddress(); + long result = 0; + for (byte octet : octets) { + result <<= 8; + result |= octet & 0xff; + } + long mask = (long) Math.pow(2, 32 - Integer.parseInt(parts[1])); + long ipLo = (result / mask) * mask; + long ipHi = (( (result / mask) + 1) * mask) - 1; + ipRanges.add(new IPRange(ipLo, ipHi)); + return; + } catch (Exception e) { + throw new IPFormatException(ip + " - Range format should be similar to 172.16.0.0/12"); + } + } else { + try { + long ipLo = ipToLong(InetAddress.getByName(ip)); + ipRanges.add(new IPRange(ipLo, ipLo)); + return; + } catch (UnknownHostException e) { + throw new IPFormatException(ip + " - IP address format should be similar to 1.2.3.14"); + } } } } diff --git a/dspace-api/src/test/java/org/dspace/statistics/util/IPTableTest.java b/dspace-api/src/test/java/org/dspace/statistics/util/IPTableTest.java index 320cc55a0d..1dbbdb6cd0 100644 --- a/dspace-api/src/test/java/org/dspace/statistics/util/IPTableTest.java +++ b/dspace-api/src/test/java/org/dspace/statistics/util/IPTableTest.java @@ -56,14 +56,15 @@ public class IPTableTest { IPTable instance = new IPTable(); // Add IP address instance.add(LOCALHOST); - // Add IP range + // Add IP range (contains 256 addresses) instance.add("192.168.1"); - // Make sure both exist + // Make sure it returns the addresses for all ranges Set ipSet = instance.toSet(); - assertEquals(2, ipSet.size()); + assertEquals(257, ipSet.size()); assertTrue(ipSet.contains(LOCALHOST)); - assertTrue(ipSet.contains("192.168.1")); + assertTrue(ipSet.contains("192.168.1.0")); + assertTrue(ipSet.contains("192.168.1.255")); } @Test @@ -76,13 +77,13 @@ public class IPTableTest { assertEquals(1, instance.toSet().size()); instance = new IPTable(); - // Add IP range & then add an IP from within that range + // Add IP range w/ 256 addresses & then add an IP from within that range instance.add("192.168.1"); instance.add("192.168.1.1"); // Verify only the range exists Set ipSet = instance.toSet(); - assertEquals(1, ipSet.size()); - assertTrue(ipSet.contains("192.168.1")); + assertEquals(256, ipSet.size()); + assertTrue(ipSet.contains("192.168.1.1")); instance = new IPTable(); // Now, switch order. Add IP address, then add a range encompassing that IP @@ -90,8 +91,8 @@ public class IPTableTest { instance.add("192.168.1"); // Verify only the range exists ipSet = instance.toSet(); - assertEquals(1, ipSet.size()); - assertTrue(ipSet.contains("192.168.1")); + assertEquals(256, ipSet.size()); + assertTrue(ipSet.contains("192.168.1.1")); } /** @@ -120,6 +121,48 @@ public class IPTableTest { assertTrue("IP within an add()ed range should match", contains); } + @Test + public void testDashRangeContains() throws Exception { + IPTable instance = new IPTable(); + instance.add("192.168.0.0 - 192.168.0.245"); + + assertTrue("Range should contain lower limit", instance.contains("192.168.0.0")); + assertTrue("Range should contain upper limit", instance.contains("192.168.0.245")); + assertTrue("Range should contain value in between limits", instance.contains("192.168.0.123")); + assertTrue("Range should contain value in between limits", instance.contains("192.168.0.234")); + + assertFalse("Range should not contain value below lower limit", instance.contains("192.167.255.255")); + assertFalse("Range should not contain value above upper limit", instance.contains("192.168.0.246")); + } + + @Test + public void testSubnetRangeContains() throws Exception { + IPTable instance = new IPTable(); + instance.add("192.168.0.0/30"); // translates to 192.168.0.0 - 192.168.0.3 + + assertTrue("Range should contain lower limit", instance.contains("192.168.0.0")); + assertTrue("Range should contain upper limit", instance.contains("192.168.0.3")); + assertTrue("Range should contain values in between limits", instance.contains("192.168.0.1")); + assertTrue("Range should contain values in between limits", instance.contains("192.168.0.2")); + + assertFalse("Range should not contain value below lower limit", instance.contains("192.167.255.255")); + assertFalse("Range should not contain value above upper limit", instance.contains("192.168.0.4")); + } + + @Test + public void testImplicitRangeContains() throws Exception { + IPTable instance = new IPTable(); + instance.add("192.168.1"); + + assertTrue("Range should contain lower limit", instance.contains("192.168.1.0")); + assertTrue("Range should contain upper limit", instance.contains("192.168.1.255")); + assertTrue("Range should contain values in between limits", instance.contains("192.168.1.123")); + assertTrue("Range should contain values in between limits", instance.contains("192.168.1.234")); + + assertFalse("Range should not contain value below lower limit", instance.contains("192.168.0.0")); + assertFalse("Range should not contain value above upper limit", instance.contains("192.168.2.0")); + } + /** * Test of isEmpty method, of class IPTable. * @throws java.lang.Exception passed through. From 93912118fd2f8b9e2ba75be3dc3c25549ed51fd1 Mon Sep 17 00:00:00 2001 From: ammanabrolua <99397044+ammanabrolua@users.noreply.github.com> Date: Tue, 12 Jul 2022 20:56:30 +0200 Subject: [PATCH 26/97] added --help option --- .../administer/CreateAdministrator.java | 97 ++++++++++++++++++- 1 file changed, 95 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java index 80d69f3b66..9aad45564e 100644 --- a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java +++ b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java @@ -14,6 +14,7 @@ import java.util.Locale; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; import org.apache.commons.cli.DefaultParser; +import org.apache.commons.cli.HelpFormatter; import org.apache.commons.cli.Options; import org.apache.commons.lang3.StringUtils; import org.dspace.core.Context; @@ -26,6 +27,7 @@ import org.dspace.eperson.service.GroupService; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; + /** * A command-line tool for creating an initial administrator for setting up a * DSpace site. Prompts for an e-mail address, last name, first name and @@ -64,16 +66,34 @@ public final class CreateAdministrator { throws Exception { CommandLineParser parser = new DefaultParser(); Options options = new Options(); + Console console = System.console(); CreateAdministrator ca = new CreateAdministrator(); + char[] password1 = null; + char[] password2 = null; + boolean dataOK = false; + String language = I18nUtil.getDefaultLocale().getLanguage(); + options.addOption("e", "email", true, "administrator email address"); options.addOption("f", "first", true, "administrator first name"); + options.addOption("h", "help", false, "explain create-administrator options"); options.addOption("l", "last", true, "administrator last name"); options.addOption("c", "language", true, "administrator language"); options.addOption("p", "password", true, "administrator password"); - CommandLine line = parser.parse(options, argv); + CommandLine line = null; + + try { + + line = parser.parse(options, argv); + + } catch (Exception e) { + + System.out.println(e.getMessage() + "\nTry \"dspace create-administrator -h\" to print help information."); + System.exit(1); + + } if (line.hasOption("e") && line.hasOption("f") && line.hasOption("l") && line.hasOption("c") && line.hasOption("p")) { @@ -81,7 +101,80 @@ public final class CreateAdministrator { line.getOptionValue("f"), line.getOptionValue("l"), line.getOptionValue("c"), line.getOptionValue("p")); } else { - ca.negotiateAdministratorDetails(); + + if (line.hasOption("h")) { + String header = "\nA command-line tool for creating an initial administrator for setting up a" + + " DSpace site. Unless all the required parameters are passed it will" + + " prompt for an e-mail address, last name, first name and password from" + + " standard input.. An administrator group is then created and the data passed" + + " in used to create an e-person in that group.\n\n"; + String footer = "\n"; + HelpFormatter formatter = new HelpFormatter(); + formatter.printHelp("dspace create-administrator", header, options, footer, true); + return; + } else { + + ConfigurationService cfg = DSpaceServicesFactory.getInstance().getConfigurationService(); + + if (!line.hasOption('p') && line.hasOption("e") && line.hasOption("f") && line.hasOption("l") + && (line.hasOption("c") || (!line.hasOption("c") + && cfg.getProperty("webui.supported.locales") == null))) { + + + if (cfg.getProperty("webui.supported.locales") != null) { + System.out.println("Select one of the following languages: " + + cfg.getProperty("webui.supported.locales")); + System.out.print("Language: "); + System.out.flush(); + + language = console.readLine(); + + if (language != null) { + language = language.trim(); + language = I18nUtil.getSupportedLocale(new Locale(language)).getLanguage(); + } + } + + while (!dataOK) { + System.out.println("Password will not display on screen."); + System.out.print("Password: "); + System.out.flush(); + + password1 = console.readPassword(); + + System.out.print("Again to confirm: "); + System.out.flush(); + + password2 = console.readPassword(); + + //TODO real password validation + if (password1.length > 1 && Arrays.equals(password1, password2)) { + // password OK + System.out.print("Is the above data correct? (y or n): "); + System.out.flush(); + + String s = console.readLine(); + + if (s != null) { + s = s.trim(); + if (s.toLowerCase().startsWith("y")) { + dataOK = true; + } + } + } else { + System.out.println("Passwords don't match"); + } + } + ca.createAdministrator(line.getOptionValue("e"), + line.getOptionValue("f"), line.getOptionValue("l"), + language, String.valueOf(password1)); + Arrays.fill(password1, ' '); + Arrays.fill(password2, ' '); + + } else { + ca.negotiateAdministratorDetails(); + } + } } } From 188cecc0b450bc64697316925c85aef63b893ac4 Mon Sep 17 00:00:00 2001 From: Yura Bondarenko Date: Fri, 5 Nov 2021 12:52:33 +0100 Subject: [PATCH 27/97] 84011: Fix AssignOriginalSubmitterAction claimed user --- .../actions/userassignment/AssignOriginalSubmitterAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/userassignment/AssignOriginalSubmitterAction.java b/dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/userassignment/AssignOriginalSubmitterAction.java index 5d934ba189..0cd82fe770 100644 --- a/dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/userassignment/AssignOriginalSubmitterAction.java +++ b/dspace-api/src/main/java/org/dspace/xmlworkflow/state/actions/userassignment/AssignOriginalSubmitterAction.java @@ -136,7 +136,7 @@ public class AssignOriginalSubmitterAction extends UserSelectionAction { protected void createTaskForEPerson(Context c, XmlWorkflowItem wfi, Step step, WorkflowActionConfig actionConfig, EPerson user) throws SQLException, AuthorizeException, IOException { if (claimedTaskService.find(c, wfi, step.getId(), actionConfig.getId()) != null) { - workflowRequirementsService.addClaimedUser(c, wfi, step, c.getCurrentUser()); + workflowRequirementsService.addClaimedUser(c, wfi, step, user); XmlWorkflowServiceFactory.getInstance().getXmlWorkflowService() .createOwnedTask(c, wfi, step, actionConfig, user); } From 84e6be5bb37b815e20d54bb1f9bf9820c5dc6245 Mon Sep 17 00:00:00 2001 From: ammanabrolua <99397044+ammanabrolua@users.noreply.github.com> Date: Wed, 13 Jul 2022 14:39:23 +0200 Subject: [PATCH 28/97] added --help option and fixed code to reflect review --- .../administer/CreateAdministrator.java | 226 +++++++----------- 1 file changed, 87 insertions(+), 139 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java index 9aad45564e..01f76c93e0 100644 --- a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java +++ b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java @@ -66,14 +66,14 @@ public final class CreateAdministrator { throws Exception { CommandLineParser parser = new DefaultParser(); Options options = new Options(); - Console console = System.console(); + // Console console = System.console(); CreateAdministrator ca = new CreateAdministrator(); - char[] password1 = null; - char[] password2 = null; - boolean dataOK = false; - String language = I18nUtil.getDefaultLocale().getLanguage(); + // char[] password1 = null; + // char[] password2 = null; + // boolean dataOK = false; + // String language = I18nUtil.getDefaultLocale().getLanguage(); options.addOption("e", "email", true, "administrator email address"); options.addOption("f", "first", true, "administrator first name"); @@ -100,81 +100,18 @@ public final class CreateAdministrator { ca.createAdministrator(line.getOptionValue("e"), line.getOptionValue("f"), line.getOptionValue("l"), line.getOptionValue("c"), line.getOptionValue("p")); - } else { - - if (line.hasOption("h")) { - String header = "\nA command-line tool for creating an initial administrator for setting up a" + + } else if (line.hasOption("h")) { + String header = "\nA command-line tool for creating an initial administrator for setting up a" + " DSpace site. Unless all the required parameters are passed it will" + " prompt for an e-mail address, last name, first name and password from" + " standard input.. An administrator group is then created and the data passed" + " in used to create an e-person in that group.\n\n"; - String footer = "\n"; - HelpFormatter formatter = new HelpFormatter(); - formatter.printHelp("dspace create-administrator", header, options, footer, true); - return; - } else { - - ConfigurationService cfg = DSpaceServicesFactory.getInstance().getConfigurationService(); - - if (!line.hasOption('p') && line.hasOption("e") && line.hasOption("f") && line.hasOption("l") - && (line.hasOption("c") || (!line.hasOption("c") - && cfg.getProperty("webui.supported.locales") == null))) { - - - if (cfg.getProperty("webui.supported.locales") != null) { - System.out.println("Select one of the following languages: " - + cfg.getProperty("webui.supported.locales")); - System.out.print("Language: "); - System.out.flush(); - - language = console.readLine(); - - if (language != null) { - language = language.trim(); - language = I18nUtil.getSupportedLocale(new Locale(language)).getLanguage(); - } - } - - while (!dataOK) { - System.out.println("Password will not display on screen."); - System.out.print("Password: "); - System.out.flush(); - - password1 = console.readPassword(); - - System.out.print("Again to confirm: "); - System.out.flush(); - - password2 = console.readPassword(); - - //TODO real password validation - if (password1.length > 1 && Arrays.equals(password1, password2)) { - // password OK - System.out.print("Is the above data correct? (y or n): "); - System.out.flush(); - - String s = console.readLine(); - - if (s != null) { - s = s.trim(); - if (s.toLowerCase().startsWith("y")) { - dataOK = true; - } - } - } else { - System.out.println("Passwords don't match"); - } - } - ca.createAdministrator(line.getOptionValue("e"), - line.getOptionValue("f"), line.getOptionValue("l"), - language, String.valueOf(password1)); - Arrays.fill(password1, ' '); - Arrays.fill(password2, ' '); - - } else { - ca.negotiateAdministratorDetails(); - } - } + String footer = "\n"; + HelpFormatter formatter = new HelpFormatter(); + formatter.printHelp("dspace create-administrator", header, options, footer, true); + return; + } else { + ca.negotiateAdministratorDetails(line); } } @@ -196,7 +133,7 @@ public final class CreateAdministrator { * * @throws Exception if error */ - protected void negotiateAdministratorDetails() + protected void negotiateAdministratorDetails(CommandLine line) throws Exception { Console console = System.console(); @@ -210,79 +147,90 @@ public final class CreateAdministrator { char[] password1 = null; char[] password2 = null; String language = I18nUtil.getDefaultLocale().getLanguage(); + ConfigurationService cfg = DSpaceServicesFactory.getInstance().getConfigurationService(); + boolean flag = false; + + + if (!line.hasOption('p') && line.hasOption("e") && line.hasOption("f") && line.hasOption("l") + && (line.hasOption("c") || (!line.hasOption("c")) + && cfg.getProperty("webui.supported.locales") == null)) { + flag = true; + } while (!dataOK) { - System.out.print("E-mail address: "); - System.out.flush(); - - email = console.readLine(); - if (!StringUtils.isBlank(email)) { - email = email.trim(); - } else { - System.out.println("Please provide an email address."); - continue; - } - - System.out.print("First name: "); - System.out.flush(); - - firstName = console.readLine(); - - if (firstName != null) { - firstName = firstName.trim(); - } - - System.out.print("Last name: "); - System.out.flush(); - - lastName = console.readLine(); - - if (lastName != null) { - lastName = lastName.trim(); - } - - ConfigurationService cfg = DSpaceServicesFactory.getInstance().getConfigurationService(); - if (cfg.hasProperty("webui.supported.locales")) { - System.out.println("Select one of the following languages: " - + cfg.getProperty("webui.supported.locales")); - System.out.print("Language: "); + if (flag == false) { + System.out.print("E-mail address: "); System.out.flush(); - language = console.readLine(); - - if (language != null) { - language = language.trim(); - language = I18nUtil.getSupportedLocale(new Locale(language)).getLanguage(); + email = console.readLine(); + if (!StringUtils.isBlank(email)) { + email = email.trim(); + } else { + System.out.println("Please provide an email address."); + continue; } - } - System.out.println("Password will not display on screen."); - System.out.print("Password: "); - System.out.flush(); - - password1 = console.readPassword(); - - System.out.print("Again to confirm: "); - System.out.flush(); - - password2 = console.readPassword(); - - //TODO real password validation - if (password1.length > 1 && Arrays.equals(password1, password2)) { - // password OK - System.out.print("Is the above data correct? (y or n): "); + System.out.print("First name: "); System.out.flush(); - String s = console.readLine(); + firstName = console.readLine(); - if (s != null) { - s = s.trim(); - if (s.toLowerCase().startsWith("y")) { - dataOK = true; + if (firstName != null) { + firstName = firstName.trim(); + } + + System.out.print("Last name: "); + System.out.flush(); + + lastName = console.readLine(); + + if (lastName != null) { + lastName = lastName.trim(); + } + } else { + + if (cfg.hasProperty("webui.supported.locales")) { + System.out.println("Select one of the following languages: " + + cfg.getProperty("webui.supported.locales")); + System.out.print("Language: "); + System.out.flush(); + + language = console.readLine(); + + if (language != null) { + language = language.trim(); + language = I18nUtil.getSupportedLocale(new Locale(language)).getLanguage(); } } - } else { - System.out.println("Passwords don't match"); + + System.out.println("Password will not display on screen."); + System.out.print("Password: "); + System.out.flush(); + + password1 = console.readPassword(); + + System.out.print("Again to confirm: "); + System.out.flush(); + + password2 = console.readPassword(); + + //TODO real password validation + if (password1.length > 1 && Arrays.equals(password1, password2)) { + // password OK + System.out.print("Is the above data correct? (y or n): "); + System.out.flush(); + + String s = console.readLine(); + + if (s != null) { + s = s.trim(); + if (s.toLowerCase().startsWith("y")) { + dataOK = true; + } + } + } else { + System.out.println("Passwords don't match"); + } } } From b0a04ce1d05edf72133807472c9daf5c34684ca8 Mon Sep 17 00:00:00 2001 From: ammanabrolua <99397044+ammanabrolua@users.noreply.github.com> Date: Wed, 13 Jul 2022 15:00:24 +0200 Subject: [PATCH 29/97] removed comments and changed flag expression --- .../java/org/dspace/administer/CreateAdministrator.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java index 01f76c93e0..f674fc5664 100644 --- a/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java +++ b/dspace-api/src/main/java/org/dspace/administer/CreateAdministrator.java @@ -66,15 +66,9 @@ public final class CreateAdministrator { throws Exception { CommandLineParser parser = new DefaultParser(); Options options = new Options(); - // Console console = System.console(); CreateAdministrator ca = new CreateAdministrator(); - // char[] password1 = null; - // char[] password2 = null; - // boolean dataOK = false; - // String language = I18nUtil.getDefaultLocale().getLanguage(); - options.addOption("e", "email", true, "administrator email address"); options.addOption("f", "first", true, "administrator first name"); options.addOption("h", "help", false, "explain create-administrator options"); @@ -158,7 +152,7 @@ public final class CreateAdministrator { } while (!dataOK) { - if (flag == false) { + if (!flag) { System.out.print("E-mail address: "); System.out.flush(); From b0316b4f74b35dc0b9d2e346898c6f5421b00fdf Mon Sep 17 00:00:00 2001 From: "Mark H. Wood" Date: Wed, 13 Jul 2022 12:01:51 -0400 Subject: [PATCH 30/97] Clean up all recommendations, plus scads of missing Javascript semicolons. --- .../org/dspace/app/mediafilter/Brand.java | 10 +- .../org/dspace/app/mediafilter/BrandText.java | 2 +- .../java/org/dspace/app/util/DCInputSet.java | 2 - .../java/org/dspace/browse/BrowseIndex.java | 8 -- .../java/org/dspace/eperson/dao/GroupDAO.java | 2 +- .../identifier/DOIIdentifierProvider.java | 2 +- .../util/SolrUpgradePre6xStatistics.java | 4 +- .../webapp/static/reports/restCollReport.js | 45 +++--- .../webapp/static/reports/restQueryReport.js | 37 +++-- .../main/webapp/static/reports/restReport.js | 136 +++++++++--------- .../repository/ViewEventRestRepository.java | 4 +- .../impl/BitstreamResourcePolicyUtils.java | 9 +- .../org/dspace/sword/CollectionDepositor.java | 2 +- .../java/org/dspace/sword/ItemDepositor.java | 2 +- 14 files changed, 126 insertions(+), 139 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/Brand.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/Brand.java index 2d963dd3da..9e28edad45 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/Brand.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/Brand.java @@ -21,10 +21,10 @@ import java.awt.image.BufferedImage; */ public class Brand { - private int brandWidth; - private int brandHeight; - private Font font; - private int xOffset; + private final int brandWidth; + private final int brandHeight; + private final Font font; + private final int xOffset; /** * Constructor to set up footer image attributes. @@ -92,7 +92,7 @@ public class Brand { * do the text placements and preparatory work for the brand image generation * * @param brandImage a BufferedImage object where the image is created - * @param identifier and Identifier object describing what text is to be placed in what + * @param brandText an Identifier object describing what text is to be placed in what * position within the brand */ private void drawImage(BufferedImage brandImage, diff --git a/dspace-api/src/main/java/org/dspace/app/mediafilter/BrandText.java b/dspace-api/src/main/java/org/dspace/app/mediafilter/BrandText.java index ae77f6048b..9110740643 100644 --- a/dspace-api/src/main/java/org/dspace/app/mediafilter/BrandText.java +++ b/dspace-api/src/main/java/org/dspace/app/mediafilter/BrandText.java @@ -39,7 +39,7 @@ class BrandText { * its location within a rectangular area. * * @param location one of the class location constants e.g. Identifier.BL - * @param the text associated with the location + * @param text text associated with the location */ public BrandText(String location, String text) { this.location = location; diff --git a/dspace-api/src/main/java/org/dspace/app/util/DCInputSet.java b/dspace-api/src/main/java/org/dspace/app/util/DCInputSet.java index 2359bf1bff..e903c11b13 100644 --- a/dspace-api/src/main/java/org/dspace/app/util/DCInputSet.java +++ b/dspace-api/src/main/java/org/dspace/app/util/DCInputSet.java @@ -17,7 +17,6 @@ import org.dspace.core.Utils; * Class representing all DC inputs required for a submission, organized into pages * * @author Brian S. Hughes, based on work by Jenny Toves, OCLC - * @version $Revision$ */ public class DCInputSet { @@ -34,7 +33,6 @@ public class DCInputSet { * constructor * * @param formName form name - * @param mandatoryFlags * @param rows the rows * @param listMap map */ diff --git a/dspace-api/src/main/java/org/dspace/browse/BrowseIndex.java b/dspace-api/src/main/java/org/dspace/browse/BrowseIndex.java index 859063272a..8d065c21ce 100644 --- a/dspace-api/src/main/java/org/dspace/browse/BrowseIndex.java +++ b/dspace-api/src/main/java/org/dspace/browse/BrowseIndex.java @@ -313,14 +313,6 @@ public final class BrowseIndex { return name; } - /** - * @param name The name to set. - */ -// public void setName(String name) -// { -// this.name = name; -// } - /** * Get the SortOption associated with this index. * diff --git a/dspace-api/src/main/java/org/dspace/eperson/dao/GroupDAO.java b/dspace-api/src/main/java/org/dspace/eperson/dao/GroupDAO.java index ab37aa4047..2cc77129f0 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/dao/GroupDAO.java +++ b/dspace-api/src/main/java/org/dspace/eperson/dao/GroupDAO.java @@ -45,7 +45,7 @@ public interface GroupDAO extends DSpaceObjectDAO, DSpaceObjectLegacySupp * Find all groups ordered by the specified metadata fields ascending * * @param context The DSpace context - * @param sortMetadataFields The metadata fields to sort on + * @param metadataSortFields The metadata fields to sort on * @param pageSize how many results return * @param offset the position of the first result to return * @return A list of all groups, ordered by metadata fields diff --git a/dspace-api/src/main/java/org/dspace/identifier/DOIIdentifierProvider.java b/dspace-api/src/main/java/org/dspace/identifier/DOIIdentifierProvider.java index 2d7914a75a..66e7b94a4b 100644 --- a/dspace-api/src/main/java/org/dspace/identifier/DOIIdentifierProvider.java +++ b/dspace-api/src/main/java/org/dspace/identifier/DOIIdentifierProvider.java @@ -543,7 +543,7 @@ public class DOIIdentifierProvider extends FilteredIdentifierProvider { if (DELETED.equals(doiRow.getStatus()) || TO_BE_DELETED.equals(doiRow.getStatus())) { throw new DOIIdentifierException("You tried to update the metadata" - + "of a DOI that is marked as DELETED.", + + " of a DOI that is marked as DELETED.", DOIIdentifierException.DOI_IS_DELETED); } diff --git a/dspace-api/src/main/java/org/dspace/util/SolrUpgradePre6xStatistics.java b/dspace-api/src/main/java/org/dspace/util/SolrUpgradePre6xStatistics.java index 12a9970539..7dcebcc09f 100644 --- a/dspace-api/src/main/java/org/dspace/util/SolrUpgradePre6xStatistics.java +++ b/dspace-api/src/main/java/org/dspace/util/SolrUpgradePre6xStatistics.java @@ -240,8 +240,8 @@ public class SolrUpgradePre6xStatistics { /** * Print a status message appended with the processing time for the operation * - * @param header - * Message to display + * @param numProcessed + * count of records processed so far. * @param fromStart * if true, report on processing time since the start of the program */ diff --git a/dspace-rest/src/main/webapp/static/reports/restCollReport.js b/dspace-rest/src/main/webapp/static/reports/restCollReport.js index 1d1c04ae07..0cd6462ccb 100644 --- a/dspace-rest/src/main/webapp/static/reports/restCollReport.js +++ b/dspace-rest/src/main/webapp/static/reports/restCollReport.js @@ -11,7 +11,7 @@ var CollReport = function() { //this.hasSorttable = function(){return true;} this.getLangSuffix = function(){ return "[en]"; - } + }; //Indicate if Password Authentication is supported //this.makeAuthLink = function(){return true;}; @@ -38,7 +38,7 @@ var CollReport = function() { icollection : "", ifilter : "", }; - } + }; this.getCurrentParameters = function(){ return { "show_fields[]" : this.myMetadataFields.getShowFields(), @@ -49,7 +49,7 @@ var CollReport = function() { icollection : $("#icollection").val(), ifilter : $("#ifilter").val(), }; - } + }; var self = this; this.init = function() { @@ -61,7 +61,7 @@ var CollReport = function() { collapsible: true, active: 2 }); - } + }; this.myAuth.callback = function(data) { self.createCollectionTable(); @@ -71,7 +71,7 @@ var CollReport = function() { $("#refresh-fields,#refresh-fields-bits").bind("click", function(){ self.drawItemTable($("#icollection").val(), $("#ifilter").val(), 0); }); - } + }; this.createCollectionTable = function() { var self = this; @@ -93,7 +93,7 @@ var CollReport = function() { self.myHtmlUtil.makeTotalCol(thn); self.addCollections(); - } + }; this.addCollections = function() { var self = this; @@ -197,7 +197,7 @@ var CollReport = function() { $(".showCollections").attr("disabled", false); } }); - } + }; this.loadData = function() { self.spinner.spin($("h1")[0]); @@ -208,7 +208,7 @@ var CollReport = function() { $("#table tr.data").addClass("processing"); self.myFilters.filterString = self.myFilters.getFilterList(); self.doRow(0, self.THREADS, self.loadId); - } + }; this.doRow = function(row, threads, curLoadId) { if (self.loadId != curLoadId) return; @@ -285,14 +285,14 @@ var CollReport = function() { $("#table").addClass("sortable"); sorttable.makeSortable($("#table")[0]); } - } - + }; + this.totalFilters = function() { - var colcount = $("#table tr th").length; - for(var i=4; i= self.TOOBIG) { td.addClass("toobig"); - title+= "\nIt will take significant time to apply this filter to the entire collection." + title+= "\nIt will take significant time to apply this filter to the entire collection."; } td.attr("title", title); return false; @@ -359,7 +359,7 @@ var CollReport = function() { self.totalFilters(); } return true; - } + }; this.setCellCount = function(tr, cid, offset, isPartial, itemFilter) { var filterName = itemFilter["filter-name"]; @@ -391,7 +391,7 @@ var CollReport = function() { $("#ifilter").val(filterName); }); } - } + }; this.drawItemTable = function(cid, filter, offset) { @@ -433,7 +433,7 @@ var CollReport = function() { offset: offset, "show_fields[]" : fields, "show_fields_bits[]" : bitfields, - } + }; $.ajax({ url: "/rest/filtered-collections/"+cid, @@ -452,7 +452,6 @@ var CollReport = function() { self.myHtmlUtil.addTd(tr, item.name).addClass("ititle"); if (fields != null) { $.each(fields, function(index, field){ - var text = ""; var td = self.myHtmlUtil.addTd(tr, ""); $.each(item.metadata, function(mindex,mv){ if (mv.key == field) { @@ -493,7 +492,7 @@ var CollReport = function() { $("#itemResults").accordion("option", "active", self.IACCIDX_ITEM); } }); - } + }; //Ignore the first column containing a row number and the item handle this.exportCol = function(colnum, col) { @@ -503,8 +502,8 @@ var CollReport = function() { data += (colnum == 1) ? "" : ","; data += self.exportCell(col); return data; - } -} + }; +}; CollReport.prototype = Object.create(Report.prototype); $(document).ready(function(){ diff --git a/dspace-rest/src/main/webapp/static/reports/restQueryReport.js b/dspace-rest/src/main/webapp/static/reports/restQueryReport.js index 9a8297fb69..18e9a61d08 100644 --- a/dspace-rest/src/main/webapp/static/reports/restQueryReport.js +++ b/dspace-rest/src/main/webapp/static/reports/restQueryReport.js @@ -12,7 +12,7 @@ var QueryReport = function() { //this.hasSorttable = function(){return true;} this.getLangSuffix = function(){ return "[en]"; - } + }; //Indicate if Password Authentication is supported //this.makeAuthLink = function(){return true;}; @@ -31,7 +31,7 @@ var QueryReport = function() { "limit" : this.ITEM_LIMIT, "offset" : 0, }; - } + }; this.getCurrentParameters = function(){ var expand = "parentCollection,metadata"; if (this.myBitstreamFields.hasBitstreamFields()) { @@ -54,21 +54,20 @@ var QueryReport = function() { paramArr[paramArr.length] = $(this).val(); }); return params; - } + }; var self = this; this.init = function() { this.baseInit(); - var communitySelector = new CommunitySelector(this, $("#collSelector"), this.myReportParameters.params["collSel[]"]); - } + }; this.initMetadataFields = function() { this.myMetadataFields = new QueryableMetadataFields(self); this.myMetadataFields.load(); - } + }; this.myAuth.callback = function(data) { - $(".query-button").click(function(){self.runQuery();}) - } + $(".query-button").click(function(){self.runQuery();}); + }; this.runQuery = function() { this.spinner.spin($("body")[0]); @@ -93,7 +92,7 @@ var QueryReport = function() { $("button").not("#next,#prev").attr("disabled", false); } }); - } + }; this.drawItemFilterTable = function(data) { $("#itemtable").replaceWith($('
')); @@ -150,7 +149,7 @@ var QueryReport = function() { }); var fieldtext = self.myBitstreamFields.getKeyText(key, item, data.bitfields); for(var j=0; j"+fieldtext[j]+"")) + td.append($("

"+fieldtext[j]+"
")); } } }); @@ -173,7 +172,7 @@ var QueryReport = function() { sorttable.makeSortable(itbl[0]); } $("#metadatadiv").accordion("option", "active", $("#metadatadiv > h3").length - 1); - } + }; //Ignore the first column containing a row number and the item handle, get handle for the collection this.exportCol = function(colnum, col) { @@ -189,8 +188,8 @@ var QueryReport = function() { } else { data += self.exportCell(col); } return data; - } -} + }; +}; QueryReport.prototype = Object.create(Report.prototype); $(document).ready(function(){ @@ -223,7 +222,7 @@ var QueryableMetadataFields = function(report) { self.initQueries(); report.spinner.stop(); $(".query-button").attr("disabled", false); - } + }; this.initQueries = function() { $("#predefselect") @@ -271,7 +270,7 @@ var QueryableMetadataFields = function(report) { self.drawFilterQuery("*","matches","^.*[^[:ascii:]].*$"); } }); - } + }; this.drawFilterQuery = function(pField, pOp, pVal) { var div = $("
").appendTo("#queries"); @@ -327,7 +326,7 @@ var QueryableMetadataFields = function(report) { self.queryButtons(); }); self.queryButtons(); - } + }; this.valField = function(valop) { var val = valop.val(); @@ -337,7 +336,7 @@ var QueryableMetadataFields = function(report) { if (disableval) { valinput.val(""); } - } + }; this.queryButtons = function() { $("button.field_plus").attr("disabled",true); @@ -346,6 +345,6 @@ var QueryableMetadataFields = function(report) { if ($("button.field_minus").length == 1) { $("button.field_minus").attr("disabled",true); } - } -} + }; +}; QueryableMetadataFields.prototype = Object.create(MetadataFields.prototype); \ No newline at end of file diff --git a/dspace-rest/src/main/webapp/static/reports/restReport.js b/dspace-rest/src/main/webapp/static/reports/restReport.js index 747e78f75a..8c9dfd9ad4 100644 --- a/dspace-rest/src/main/webapp/static/reports/restReport.js +++ b/dspace-rest/src/main/webapp/static/reports/restReport.js @@ -11,7 +11,7 @@ var Report = function() { this.COUNT_LIMIT = 500; this.ITEM_LIMIT = 100; - this.ROOTPATH = "/handle/" + this.ROOTPATH = "/handle/"; //Indicate if Password Authentication is supported this.makeAuthLink = function(){return false;}; @@ -23,23 +23,23 @@ var Report = function() { //Override this to return obj.id for DSpace 5 versions this.getId = function(obj) { return obj.uuid; - } + }; //Override this method is sortable.js has been included this.hasSorttable = function() { return false; - } + }; this.getDefaultParameters = function(){ return {}; - } + }; this.getCurrentParameters = function(){ return {}; - } + }; this.saveUrl = function() { this.myReportParameters.saveAsUrl(this.getCurrentParameters()); - } + }; this.getLoginPayload = function() { //Placeholder to allow a customized report to prompt for email/password @@ -53,15 +53,15 @@ var Report = function() { } else { return {email: email, password: pass}; } - } + }; this.getLangSuffix = function(){ return ""; - } + }; this.myAuth = new Auth(this); this.myAuth.authStat(); this.myAuth.callback = function(data) { self.spinner.stop(); - } + }; this.myHtmlUtil = new HtmlUtil(); this.spinner = new Spinner({ lines: 13, // The number of lines to draw @@ -103,7 +103,7 @@ var Report = function() { itemsTitle += " (" + (offset+1) + " - " + last + suff + ")"; $("#prev,#next").attr("disabled",true); $("#itemdiv h3").text(itemsTitle); - $("#exlimit").removeClass("red"); + $("#exlimit").removeClass("red"); if (offset > 0) $("#prev").attr("disabled", false); $("#prev").off("click").on("click", funcdec); //in case of filters, always allow next @@ -112,14 +112,14 @@ var Report = function() { $("#next").attr("disabled", false); } else if (offset + limit < total) { $("#next").attr("disabled", false); - $("#exlimit").addClass("red"); + $("#exlimit").addClass("red"); } else if (limit == total) { //total may only be accurate to one page $("#next").attr("disabled", false); - $("#exlimit").addClass("red"); + $("#exlimit").addClass("red"); } $("#next").off("click").on("click", funcinc); - } + }; this.myReportParameters = undefined; this.myFilters = undefined; @@ -128,12 +128,12 @@ var Report = function() { this.initMetadataFields = function() { this.myMetadataFields = new MetadataFields(self); this.myMetadataFields.load(); - } + }; this.initBitstreamFields = function() { this.myBitstreamFields = new BitstreamFields(self); this.myBitstreamFields.load(); - } + }; this.baseInit = function() { this.myReportParameters = new ReportParameters( @@ -144,7 +144,7 @@ var Report = function() { this.myFilters = new Filters(this.myReportParameters.params["filters"]); this.initMetadataFields(); this.initBitstreamFields(); - this.getActiveTab = function(){return 1;} + this.getActiveTab = function(){return 1;}; $("#metadatadiv").accordion({ heightStyle: "content", collapsible: true, @@ -158,7 +158,7 @@ var Report = function() { }); this.myFilters.createFilterTable(this.myReportParameters.params.filters); this.myAuth.init(); - } + }; this.makeCsv = function(rows) { var itemdata = ""; @@ -169,13 +169,13 @@ var Report = function() { }); }); return itemdata; - } + }; this.export = function(rows) { var itemdata = "data:text/csv;charset=utf-8," + this.makeCsv(rows); var encodedUri = encodeURI(itemdata); window.open(encodedUri); - } + }; //this is meant to be overridden for each report this.exportCol = function(colnum, col) { @@ -183,7 +183,7 @@ var Report = function() { data += (colnum == 0) ? "" : ","; data += self.exportCell(col); return data; - } + }; this.exportCell = function(col) { data = "\""; @@ -199,13 +199,12 @@ var Report = function() { }); data += "\""; return data; - } + }; this.init = function() { this.baseInit(); - } - -} + }; +}; var Auth = function(report) { this.report = report; @@ -214,7 +213,7 @@ var Auth = function(report) { }; this.saveToken = function(data) { this.TOKEN = data; - } + }; this.init = function() { var loginPayload = report.getLoginPayload(); if (loginPayload == undefined) { @@ -240,7 +239,7 @@ var Auth = function(report) { self.callback(); } }); - } + }; this.verifyShibLogin = function() { var self = this; @@ -248,7 +247,7 @@ var Auth = function(report) { url: "/rest/shibboleth-login", success: self.authStat }); - } + }; this.authStat = function() { var self = this; @@ -278,7 +277,7 @@ var Auth = function(report) { } } }); - } + }; this.logout = function() { var self = this; @@ -291,15 +290,15 @@ var Auth = function(report) { self.authStat(); } }); - } + }; this.getHeaders = function() { var HEADERS = {}; if (this.TOKEN != null) { HEADERS['rest-dspace-token'] = this.TOKEN; } return HEADERS; - } -} + }; +}; var ReportParameters = function(defaultParams, prmstr) { this.params = defaultParams; @@ -326,21 +325,21 @@ var ReportParameters = function(defaultParams, prmstr) { this.getOffset = function() { var offset = $("#offset").val(); return $.isNumeric(offset) ? Number(offset) : this.offset; - } + }; this.getNextOffset = function() { return this.getOffset() + this.getLimit(); - } + }; this.getPrevOffset = function() { var v = this.getOffset() - this.getLimit(); return v < 0 ? 0 : v; - } + }; this.getLimit = function() { var limit = $("#limit").val(); return $.isNumeric(limit) ? Number(limit) : this.limit; - } + }; this.updateOffset = function(increment) { var val = $("#offset").val(); @@ -352,13 +351,13 @@ var ReportParameters = function(defaultParams, prmstr) { $("#offset").val(this.getPrevOffset()); } } - } + }; this.saveAsUrl = function(params) { var pstr = $.param(params).replace(/%5B%5D/g,"[]"); window.location.search = pstr; - } -} + }; +}; var Filters = function() { @@ -397,7 +396,7 @@ var Filters = function() { }); } ); - } + }; this.addFilter = function(val, categories, category, title, description, cname) { var catdiv = null; @@ -425,7 +424,7 @@ var Filters = function() { div.append(label); catdiv.append(div); return input; - } + }; this.getFilterList = function() { var list=""; @@ -441,8 +440,8 @@ var Filters = function() { list = "none"; } return list; - } -} + }; +}; var MetadataFields = function(report) { this.metadataSchemas = undefined; @@ -461,21 +460,20 @@ var MetadataFields = function(report) { complete: function(xhr, status) { } }); - } + }; this.initFields = function(data, report) { var params = report.myReportParameters.params; self.metadataSchemas = data; self.drawShowFields(params["show_fields[]"]); - } + }; this.getShowFields = function(){ var val = $("#show-fields select").val(); return val == null ? Array() : val; - } + }; this.drawShowFields = function(pfields) { - var self = this; var sel = $(""); @@ -584,7 +583,7 @@ var BitstreamFields = function(report) { } sel.append(opt); } - } + }; this.getKeyText = function(key, item, bitfields) { @@ -620,15 +619,15 @@ var BitstreamFields = function(report) { } }); return ret; - } -} + }; +}; var HtmlUtil = function() { this.addTr = function(tbl) { var tr = $(""); tbl.append(tr); return tr; - } + }; this.addTd = function(tr, val) { var td = $(""); @@ -637,7 +636,7 @@ var HtmlUtil = function() { } tr.append(td); return td; - } + }; this.addTh = function(tr, val) { var th = $(""); @@ -646,12 +645,11 @@ var HtmlUtil = function() { } tr.append(th); return th; - } - + }; this.addTdAnchor = function(tr, val, href) { return this.addTd(tr, this.getAnchor(val, href)); - } + }; this.getAnchor = function(val, href) { var a = $(""); @@ -660,29 +658,29 @@ var HtmlUtil = function() { a.attr("target", "_blank"); a.attr("rel", "noopener noreferrer"); return a; - } + }; this.createOpt = function(name, val) { var opt = $("