From a10426b858cf092210802b3710a3943a20ee3961 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 4 Nov 2024 16:56:26 +0100 Subject: [PATCH 1/4] [DSC-2014] add tests for RequestItemMetadataStrategy --- .../RequestItemMetadataStrategyTest.java | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java diff --git a/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java new file mode 100644 index 000000000000..6a295ac1b019 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java @@ -0,0 +1,153 @@ +/** + * 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.sql.SQLException; +import java.util.List; + +import org.dspace.AbstractUnitTest; +import org.dspace.builder.AbstractBuilder; +import org.dspace.builder.CollectionBuilder; +import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.EPersonBuilder; +import org.dspace.builder.ItemBuilder; +import org.dspace.content.Collection; +import org.dspace.content.Community; +import org.dspace.content.Item; +import org.dspace.content.factory.ContentServiceFactory; +import org.dspace.content.service.ItemService; +import org.dspace.core.Context; +import org.dspace.discovery.SearchUtils; +import org.dspace.eperson.EPerson; +import org.dspace.services.ConfigurationService; +import org.dspace.services.factory.DSpaceServicesFactory; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; + +/** + * Test suit for RequestItemMetadataStrategy + * + * @author Mykhaylo Boychuk (mykhaylo.boychuk at 4science.com) + */ +public class RequestItemMetadataStrategyTest extends AbstractUnitTest { + private static final String AUTHOR_ADDRESS = "john.doe@example.com"; + + private ItemService itemService; + private ConfigurationService configurationService; + + private static EPerson johnDoe; + + private Item item; + private Item orphanItem; + private Item notVadilEmailItem; + + @BeforeClass + public static void setUpClass() throws SQLException { + AbstractBuilder.init(); + Context context = new Context(); + context.turnOffAuthorisationSystem(); + johnDoe = EPersonBuilder.createEPerson(context) + .withEmail(AUTHOR_ADDRESS) + .withNameInMetadata("John", "Doe") + .build(); + context.restoreAuthSystemState(); + context.complete(); + } + + @AfterClass + public static void tearDownClass() { + AbstractBuilder.destroy(); + } + + @Before + public void setUp() { + this.itemService = ContentServiceFactory.getInstance().getItemService(); + this.configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + + context = new Context(); + context.setCurrentUser(johnDoe); + context.turnOffAuthorisationSystem(); + Community community = CommunityBuilder.createCommunity(context).build(); + Collection collection = CollectionBuilder.createCollection(context, community).build(); + item = ItemBuilder.createItem(context, collection) + .withTitle("Misha Boychuk") + .withPersonEmail("misha.boychuk@test.com") + .build(); + + orphanItem = ItemBuilder.createItem(context, collection) + .withTitle("Orphan Item") + .withPersonEmail("orphanItem@test.com") + .build(); + orphanItem.setSubmitter(null); + + notVadilEmailItem = ItemBuilder.createItem(context, collection) + .withTitle("Not Valid email Item") + .withPersonEmail("test@test") + .build(); + context.restoreAuthSystemState(); + } + + /** + * Test of getRequestItemAuthor method, of class RequestItemMetadataStrategy. + * @throws Exception passed through. + */ + @Test + public void testGetRequestItemAuthor() throws Exception { + RequestItemMetadataStrategy metadataStrategy = new RequestItemMetadataStrategy(); + metadataStrategy.setItemService(this.itemService); + metadataStrategy.setConfigurationService(this.configurationService); + metadataStrategy.setEmailMetadata("person.email"); + metadataStrategy.setFullNameMetadata("dc.title"); + List author = metadataStrategy.getRequestItemAuthor(context, item); + assertEquals("misha.boychuk@test.com", author.get(0).getEmail()); + assertEquals("Misha Boychuk", author.get(0).getFullName()); + + this.configurationService.setProperty("mail.helpdesk", "helpdesk@test.com"); + this.configurationService.setProperty("mail.helpdesk.name", "Helpdesk Name"); + + // item without 'person.notExist' metadata & submitter is null + RequestItemMetadataStrategy metadataStrategy2 = new RequestItemMetadataStrategy(); + metadataStrategy2.setItemService(this.itemService); + metadataStrategy2.setConfigurationService(this.configurationService); + metadataStrategy2.setEmailMetadata("person.notExist"); + metadataStrategy2.setFullNameMetadata("dc.title"); + List author2 = metadataStrategy2.getRequestItemAuthor(context, orphanItem); + assertEquals("helpdesk@test.com", author2.get(0).getEmail()); + assertEquals("Helpdesk Name", author2.get(0).getFullName()); + + // item without 'person.notExist' metadata & submitter is null & helpdesk is null + this.configurationService.setProperty("mail.helpdesk", null); + this.configurationService.setProperty("mail.admin", "adminTest@test.com"); + this.configurationService.setProperty("mail.admin.name", "Admin Name"); + + RequestItemMetadataStrategy metadataStrategy3 = new RequestItemMetadataStrategy(); + metadataStrategy3.setItemService(this.itemService); + metadataStrategy3.setConfigurationService(this.configurationService); + metadataStrategy3.setEmailMetadata("person.notExist"); + metadataStrategy3.setFullNameMetadata("dc.title"); + List author3 = metadataStrategy3.getRequestItemAuthor(context, orphanItem); + assertEquals("adminTest@test.com", author3.get(0).getEmail()); + assertEquals("Admin Name", author3.get(0).getFullName()); + + // not valid email test + RequestItemMetadataStrategy metadataStrategy4 = new RequestItemMetadataStrategy(); + metadataStrategy4.setItemService(this.itemService); + metadataStrategy4.setConfigurationService(this.configurationService); + metadataStrategy4.setEmailMetadata("person.email"); + metadataStrategy4.setFullNameMetadata("dc.title"); + List author4 = metadataStrategy4.getRequestItemAuthor(context, notVadilEmailItem); + assertEquals(AUTHOR_ADDRESS, author4.get(0).getEmail()); + assertEquals(johnDoe.getFullName(), author4.get(0).getFullName()); + } + +} From 3503d17109988306c0c7772e34fde98b5b6cec5d Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 4 Nov 2024 16:57:58 +0100 Subject: [PATCH 2/4] [DSC-2014] improve RequestItemMetadataStrategy --- .../RequestItemMetadataStrategy.java | 67 ++++++++++++------- 1 file changed, 41 insertions(+), 26 deletions(-) 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 4372ab9b09b0..775159dbbbdf 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 @@ -13,6 +13,7 @@ import java.util.List; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.validator.routines.EmailValidator; import org.dspace.content.Item; import org.dspace.content.MetadataValue; import org.dspace.content.service.ItemService; @@ -57,6 +58,7 @@ public List getRequestItemAuthor(Context context, Item item) } boolean useNames = vals.size() == nameVals.size(); if (!vals.isEmpty()) { + EmailValidator emailValidator = EmailValidator.getInstance(false, false); authors = new ArrayList<>(vals.size()); for (int authorIndex = 0; authorIndex < vals.size(); authorIndex++) { String email = vals.get(authorIndex).getValue(); @@ -66,42 +68,47 @@ public List getRequestItemAuthor(Context context, Item item) } if (StringUtils.isBlank(fullname)) { - fullname = I18nUtil.getMessage( - "org.dspace.app.requestitem.RequestItemMetadataStrategy.unnamed", - context); + fullname = email; + } + + if (emailValidator.isValid(email)) { + RequestItemAuthor author = new RequestItemAuthor(fullname, email); + authors.add(author); } - RequestItemAuthor author = new RequestItemAuthor( - fullname, email); - authors.add(author); } - return authors; + return authors.size() > 0 ? authors : fallback(context, item); } else { - return Collections.EMPTY_LIST; + return fallback(context, item); } } else { - // Uses the basic strategy to look for the original submitter - authors = super.getRequestItemAuthor(context, item); + return fallback(context, item); + } + } - // Remove from the list authors that do not have email addresses. - for (RequestItemAuthor author : authors) { - if (null == author.getEmail()) { - authors.remove(author); - } + private List fallback(Context context, Item item) throws SQLException { + List authors; + // Uses the basic strategy to look for the original submitter + authors = super.getRequestItemAuthor(context, item); + + // Remove from the list authors that do not have email addresses. + for (RequestItemAuthor author : authors) { + if (null == author.getEmail()) { + authors.remove(author); } + } - if (authors.isEmpty()) { // No author email addresses! Fall back - //First get help desk name and email - String email = configurationService.getProperty("mail.helpdesk"); - String name = configurationService.getProperty("mail.helpdesk.name"); - // If help desk mail is null get the mail and name of admin - if (email == null) { - email = configurationService.getProperty("mail.admin"); - name = configurationService.getProperty("mail.admin.name"); - } - authors.add(new RequestItemAuthor(name, email)); + if (authors.isEmpty()) { // No author email addresses! Fall back + //First get help desk name and email + String email = configurationService.getProperty("mail.helpdesk"); + String name = configurationService.getProperty("mail.helpdesk.name"); + // If help desk mail is null get the mail and name of admin + if (email == null) { + email = configurationService.getProperty("mail.admin"); + name = configurationService.getProperty("mail.admin.name"); } - return authors; + authors.add(new RequestItemAuthor(name, email)); } + return authors; } public void setEmailMetadata(@NonNull String emailMetadata) { @@ -112,4 +119,12 @@ public void setFullNameMetadata(@NonNull String fullNameMetadata) { this.fullNameMetadata = fullNameMetadata; } + public void setConfigurationService(ConfigurationService configurationService) { + this.configurationService = configurationService; + } + + public void setItemService(ItemService itemService) { + this.itemService = itemService; + } + } From 318fcbe8f421d965d8fd8d08ef53d13c63466415 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 4 Nov 2024 17:17:43 +0100 Subject: [PATCH 3/4] [DSC-2014] remove unused imports --- .../org/dspace/app/requestitem/RequestItemMetadataStrategy.java | 1 - .../dspace/app/requestitem/RequestItemMetadataStrategyTest.java | 2 -- 2 files changed, 3 deletions(-) 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 775159dbbbdf..3f2ab5b786d3 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 @@ -18,7 +18,6 @@ import org.dspace.content.MetadataValue; import org.dspace.content.service.ItemService; import org.dspace.core.Context; -import org.dspace.core.I18nUtil; import org.dspace.services.ConfigurationService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.lang.NonNull; diff --git a/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java index 6a295ac1b019..84c0d3b6ab66 100644 --- a/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java +++ b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java @@ -24,7 +24,6 @@ import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.ItemService; import org.dspace.core.Context; -import org.dspace.discovery.SearchUtils; import org.dspace.eperson.EPerson; import org.dspace.services.ConfigurationService; import org.dspace.services.factory.DSpaceServicesFactory; @@ -32,7 +31,6 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -import org.springframework.beans.factory.annotation.Autowired; /** * Test suit for RequestItemMetadataStrategy From 5bc0e3eec8c5c4b597d5dd5d66879f54f3cfafcc Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Tue, 5 Nov 2024 10:34:53 +0100 Subject: [PATCH 4/4] [DSC-2014] split test method --- .../RequestItemMetadataStrategyTest.java | 103 +++++++++++------- 1 file changed, 65 insertions(+), 38 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java index 84c0d3b6ab66..14bd96ba124b 100644 --- a/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java +++ b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemMetadataStrategyTest.java @@ -33,7 +33,7 @@ import org.junit.Test; /** - * Test suit for RequestItemMetadataStrategy + * Test suite for RequestItemMetadataStrategy * * @author Mykhaylo Boychuk (mykhaylo.boychuk at 4science.com) */ @@ -47,7 +47,7 @@ public class RequestItemMetadataStrategyTest extends AbstractUnitTest { private Item item; private Item orphanItem; - private Item notVadilEmailItem; + private Item notValidEmailItem; @BeforeClass public static void setUpClass() throws SQLException { @@ -72,6 +72,9 @@ public void setUp() { this.itemService = ContentServiceFactory.getInstance().getItemService(); this.configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + this.configurationService.setProperty("mail.helpdesk", "helpdesk@test.com"); + this.configurationService.setProperty("mail.helpdesk.name", "Helpdesk Name"); + context = new Context(); context.setCurrentUser(johnDoe); context.turnOffAuthorisationSystem(); @@ -88,7 +91,7 @@ public void setUp() { .build(); orphanItem.setSubmitter(null); - notVadilEmailItem = ItemBuilder.createItem(context, collection) + notValidEmailItem = ItemBuilder.createItem(context, collection) .withTitle("Not Valid email Item") .withPersonEmail("test@test") .build(); @@ -100,52 +103,76 @@ public void setUp() { * @throws Exception passed through. */ @Test - public void testGetRequestItemAuthor() throws Exception { + public void testMetadataStrategy() throws Exception { RequestItemMetadataStrategy metadataStrategy = new RequestItemMetadataStrategy(); metadataStrategy.setItemService(this.itemService); metadataStrategy.setConfigurationService(this.configurationService); metadataStrategy.setEmailMetadata("person.email"); metadataStrategy.setFullNameMetadata("dc.title"); - List author = metadataStrategy.getRequestItemAuthor(context, item); - assertEquals("misha.boychuk@test.com", author.get(0).getEmail()); - assertEquals("Misha Boychuk", author.get(0).getFullName()); - this.configurationService.setProperty("mail.helpdesk", "helpdesk@test.com"); - this.configurationService.setProperty("mail.helpdesk.name", "Helpdesk Name"); + List authors = metadataStrategy.getRequestItemAuthor(context, item); + assertEquals("misha.boychuk@test.com", authors.get(0).getEmail()); + assertEquals("Misha Boychuk", authors.get(0).getFullName()); + + List authorsOforphanItem = metadataStrategy.getRequestItemAuthor(context, orphanItem); + assertEquals("orphanItem@test.com", authorsOforphanItem.get(0).getEmail()); + assertEquals("Orphan Item", authorsOforphanItem.get(0).getFullName()); + + List authorsOfnotValidEmailItem = + metadataStrategy.getRequestItemAuthor(context, notValidEmailItem); + assertEquals(AUTHOR_ADDRESS, authorsOfnotValidEmailItem.get(0).getEmail()); + assertEquals("John Doe", authorsOfnotValidEmailItem.get(0).getFullName()); + } + + @Test + public void testMissingMetadataStrategy() throws Exception { + RequestItemMetadataStrategy metadataStrategy = new RequestItemMetadataStrategy(); + metadataStrategy.setItemService(this.itemService); + metadataStrategy.setConfigurationService(this.configurationService); + metadataStrategy.setEmailMetadata("person.notExist"); + metadataStrategy.setFullNameMetadata("dc.title"); + + List authors = metadataStrategy.getRequestItemAuthor(context, item); + assertEquals(AUTHOR_ADDRESS, authors.get(0).getEmail()); + assertEquals("John Doe", authors.get(0).getFullName()); - // item without 'person.notExist' metadata & submitter is null - RequestItemMetadataStrategy metadataStrategy2 = new RequestItemMetadataStrategy(); - metadataStrategy2.setItemService(this.itemService); - metadataStrategy2.setConfigurationService(this.configurationService); - metadataStrategy2.setEmailMetadata("person.notExist"); - metadataStrategy2.setFullNameMetadata("dc.title"); - List author2 = metadataStrategy2.getRequestItemAuthor(context, orphanItem); - assertEquals("helpdesk@test.com", author2.get(0).getEmail()); - assertEquals("Helpdesk Name", author2.get(0).getFullName()); - - // item without 'person.notExist' metadata & submitter is null & helpdesk is null + List authorsOforphanItem = metadataStrategy.getRequestItemAuthor(context, orphanItem); + assertEquals("helpdesk@test.com", authorsOforphanItem.get(0).getEmail()); + assertEquals("Helpdesk Name", authorsOforphanItem.get(0).getFullName()); + + List authorsOfnotValidEmailItem = + metadataStrategy.getRequestItemAuthor(context, notValidEmailItem); + assertEquals(AUTHOR_ADDRESS, authorsOfnotValidEmailItem.get(0).getEmail()); + assertEquals("John Doe", authorsOfnotValidEmailItem.get(0).getFullName()); + } + + @Test + public void testNotConfiguredMetadataStrategy() throws Exception { + RequestItemMetadataStrategy metadataStrategy = new RequestItemMetadataStrategy(); + metadataStrategy.setItemService(this.itemService); + metadataStrategy.setConfigurationService(this.configurationService); + + List authors = metadataStrategy.getRequestItemAuthor(context, item); + assertEquals(AUTHOR_ADDRESS, authors.get(0).getEmail()); + assertEquals("John Doe", authors.get(0).getFullName()); + + List authorsOforphanItem = metadataStrategy.getRequestItemAuthor(context, orphanItem); + assertEquals("helpdesk@test.com", authorsOforphanItem.get(0).getEmail()); + assertEquals("Helpdesk Name", authorsOforphanItem.get(0).getFullName()); + + // set helpdesk to null this.configurationService.setProperty("mail.helpdesk", null); this.configurationService.setProperty("mail.admin", "adminTest@test.com"); this.configurationService.setProperty("mail.admin.name", "Admin Name"); - RequestItemMetadataStrategy metadataStrategy3 = new RequestItemMetadataStrategy(); - metadataStrategy3.setItemService(this.itemService); - metadataStrategy3.setConfigurationService(this.configurationService); - metadataStrategy3.setEmailMetadata("person.notExist"); - metadataStrategy3.setFullNameMetadata("dc.title"); - List author3 = metadataStrategy3.getRequestItemAuthor(context, orphanItem); - assertEquals("adminTest@test.com", author3.get(0).getEmail()); - assertEquals("Admin Name", author3.get(0).getFullName()); - - // not valid email test - RequestItemMetadataStrategy metadataStrategy4 = new RequestItemMetadataStrategy(); - metadataStrategy4.setItemService(this.itemService); - metadataStrategy4.setConfigurationService(this.configurationService); - metadataStrategy4.setEmailMetadata("person.email"); - metadataStrategy4.setFullNameMetadata("dc.title"); - List author4 = metadataStrategy4.getRequestItemAuthor(context, notVadilEmailItem); - assertEquals(AUTHOR_ADDRESS, author4.get(0).getEmail()); - assertEquals(johnDoe.getFullName(), author4.get(0).getFullName()); + List authorsOforphanItem2 = metadataStrategy.getRequestItemAuthor(context, orphanItem); + assertEquals("adminTest@test.com", authorsOforphanItem2.get(0).getEmail()); + assertEquals("Admin Name", authorsOforphanItem2.get(0).getFullName()); + + List authorsOfnotValidEmailItem = + metadataStrategy.getRequestItemAuthor(context, notValidEmailItem); + assertEquals(AUTHOR_ADDRESS, authorsOfnotValidEmailItem.get(0).getEmail()); + assertEquals("John Doe", authorsOfnotValidEmailItem.get(0).getFullName()); } }