From 80e8388480b27d02c9d6379acef959d73e4ae362 Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Wed, 13 Dec 2023 11:06:40 +0100 Subject: [PATCH 1/5] skip processing in EmbeddablePlumXMetricProvider as soon as possible --- .../embeddable/impl/EmbeddablePlumXMetricProvider.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java b/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java index d9930c8e413c..5d760a0059ae 100644 --- a/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java +++ b/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java @@ -139,6 +139,10 @@ public void setPublicationListViewEnabled(boolean publicationListViewEnabled) { @Override public boolean hasMetric(Context context, Item item, List retrivedStoredMetrics) { + if (!super.hasMetric(context, item, retrivedStoredMetrics)) { + return false; + } + String entityType = getEntityType(item); if (entityType != null) { if (entityType.equals("Person")) { @@ -162,8 +166,6 @@ public boolean hasMetric(Context context, Item item, List retrivedS } } } - } else { - return false; } return false; } From db655d0e441b6dedb6afea708a69cac03472462f Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Wed, 13 Dec 2023 12:13:54 +0100 Subject: [PATCH 2/5] try to fix broken unit tests --- .../impl/EmbeddablePlumXMetricProviderTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java b/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java index 119aea38021d..8ab729f11c3e 100644 --- a/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java +++ b/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java @@ -54,6 +54,8 @@ public void setUp() throws Exception { provider.dataPopup = "left"; provider.listDataWidth = "350px"; provider.listDataPopup = "left"; + provider.setPersonListViewEnabled(true); + provider.setPublicationListViewEnabled(true); } @Test @@ -105,8 +107,8 @@ public void innerHtmlForPersonItem() { .thenReturn("0000-0002-9029-1854"); String template = provider.innerHtml(context, item); - assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":false," + - "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":false," + + assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":true," + + "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":true," + "\"type\":\"Person\",\"list-type\":\"Person\",\"placeholder\":\"\",\"list-placeholder\":\"\"," + "\"src\":\"//cdn.plu.mx/widget-person.js\",\"href\":\"https://plu.mx/plum/u/?orcid=0000-0002-9029-1854\"," + "\"list-src\":\"//cdn.plu.mx/widget-person.js\"," + @@ -131,8 +133,8 @@ public void innerHtmlForPublicationItem() { .thenReturn("10.1016/j.gene.2009.04.019"); String template = provider.innerHtml(context, item); - assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":false," + - "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":false," + + assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":true," + + "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":true," + "\"type\":\"Publication\",\"list-type\":\"Publication\",\"placeholder\":\"\"," + "\"list-placeholder\":\"\",\"src\":\"//cdn.plu.mx/widget-popup.js\"," + "\"href\":\"https://plu.mx/plum/a/?doi=10.1016/j.gene.2009.04.019\"," + From e2281865ffb11d756fe27060f2cc732aaa4e1025 Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Wed, 13 Dec 2023 13:16:33 +0100 Subject: [PATCH 3/5] check return value of isEnabled directly --- .../metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java b/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java index 5d760a0059ae..1e63b1bad974 100644 --- a/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java +++ b/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java @@ -139,7 +139,7 @@ public void setPublicationListViewEnabled(boolean publicationListViewEnabled) { @Override public boolean hasMetric(Context context, Item item, List retrivedStoredMetrics) { - if (!super.hasMetric(context, item, retrivedStoredMetrics)) { + if (!this.isEnabled()) { return false; } From 3b936b57ae6e7b694532cc08952cd5ac2d122dd3 Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Wed, 13 Dec 2023 15:46:13 +0100 Subject: [PATCH 4/5] improve enabled checks in hasMetric --- .../impl/EmbeddablePlumXMetricProvider.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java b/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java index 1e63b1bad974..6b0b6b0f20a5 100644 --- a/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java +++ b/dspace-api/src/main/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProvider.java @@ -139,31 +139,29 @@ public void setPublicationListViewEnabled(boolean publicationListViewEnabled) { @Override public boolean hasMetric(Context context, Item item, List retrivedStoredMetrics) { - if (!this.isEnabled()) { + if (! isEnabled()) { return false; } - + String entityType = getEntityType(item); if (entityType != null) { if (entityType.equals("Person")) { // if it is of type person use orcid String orcid = getItemService() .getMetadataFirstValue(item, "person", "identifier", "orcid", Item.ANY); - if (orcid != null) { - return true; - } else { + if (orcid == null) { return false; } + return personListViewEnabled || personDetailViewEnabled; } else { // if it is of type publication use doi if (entityType.equals("Publication")) { String doiIdentifier = getItemService() .getMetadataFirstValue(item, "dc", "identifier", "doi", Item.ANY); - if (doiIdentifier != null) { - return true; - } else { + if (doiIdentifier == null) { return false; } + return publicationListViewEnabled || publicationDetailViewEnabled; } } } From 636046ba0f28be9b369f02b2d52634c78d6bb34c Mon Sep 17 00:00:00 2001 From: Sascha Szott Date: Wed, 13 Dec 2023 15:46:44 +0100 Subject: [PATCH 5/5] improved test coverage --- .../EmbeddablePlumXMetricProviderTest.java | 98 ++++++++++++++----- 1 file changed, 73 insertions(+), 25 deletions(-) diff --git a/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java b/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java index 8ab729f11c3e..5a376059692b 100644 --- a/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java +++ b/dspace-api/src/test/java/org/dspace/metrics/embeddable/impl/EmbeddablePlumXMetricProviderTest.java @@ -10,7 +10,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; import org.dspace.content.Item; @@ -19,6 +18,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -31,7 +31,7 @@ public class EmbeddablePlumXMetricProviderTest { @Mock private ItemService itemService; - @Mock + @InjectMocks private EmbeddablePlumXMetricProvider provider; @Mock Context context; @@ -40,22 +40,15 @@ public class EmbeddablePlumXMetricProviderTest { @Before public void setUp() throws Exception { - when(provider.innerHtml(any(), any())).thenCallRealMethod(); - when(provider.getEntityType(any())).thenCallRealMethod(); - when(provider.hasMetric(any(), any(), any())).thenCallRealMethod(); - when(provider.getItemService()).thenReturn(itemService); - - provider.personPlumXScript = "//cdn.plu.mx/widget-person.js"; - provider.publicationPlumXScript = "//cdn.plu.mx/widget-popup.js"; - provider.publicationHref = "https://plu.mx/plum/a/"; - provider.personHref = "https://plu.mx/plum/u/"; - provider.dataNumArtifacts = 5; - provider.dataWidth = "350px"; - provider.dataPopup = "left"; - provider.listDataWidth = "350px"; - provider.listDataPopup = "left"; - provider.setPersonListViewEnabled(true); - provider.setPublicationListViewEnabled(true); + provider.setPersonPlumXScript("//cdn.plu.mx/widget-person.js"); + provider.setPublicationPlumXScript("//cdn.plu.mx/widget-popup.js"); + provider.setPublicationHref("https://plu.mx/plum/a/"); + provider.setPersonHref("https://plu.mx/plum/u/"); + provider.setDataNumArtifacts(5); + provider.setDataWidth("350px"); + provider.setDataPopup("left"); + provider.setListDataWidth("350px"); + provider.setListDataPopup("left"); } @Test @@ -66,6 +59,7 @@ public void hasMetricEmptyEntityType() { @Test public void hasMetricPublicationWithoutDoi() { + provider.setPublicationListViewEnabled(true); when(itemService.getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)).thenReturn("Publication"); when(itemService.getMetadataFirstValue(item, "dc", "identifier", "doi", Item.ANY)).thenReturn(null); boolean hasMetric = provider.hasMetric(context, item, null); @@ -73,7 +67,8 @@ public void hasMetricPublicationWithoutDoi() { } @Test - public void hasMetricPublicationWithDoi() { + public void hasMetricPublicationWithDoiEnabled() { + provider.setPublicationListViewEnabled(true); when(itemService.getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)).thenReturn("Publication"); when(itemService.getMetadataFirstValue(item, "dc", @@ -83,7 +78,19 @@ public void hasMetricPublicationWithDoi() { } @Test - public void hasMetricPublicationWithOrcid() { + public void hasMetricPublicationWithDoiDisabled() { + provider.setPersonListViewEnabled(true); + when(itemService.getMetadataFirstValue(item, "dspace", + "entity", "type", Item.ANY)).thenReturn("Publication"); + when(itemService.getMetadataFirstValue(item, "dc", + "identifier", "doi", Item.ANY)).thenReturn("10.1016/j.gene.2009.04.019"); + boolean hasMetric = provider.hasMetric(context, item, null); + assertFalse(hasMetric); + } + + @Test + public void hasMetricPersonWithOrcidEnabled() { + provider.setPersonListViewEnabled(true); when(itemService.getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)).thenReturn("Person"); when(itemService.getMetadataFirstValue(item, "person", @@ -93,7 +100,19 @@ public void hasMetricPublicationWithOrcid() { } @Test - public void hasMetricPublicationWithoutOrcid() { + public void hasMetricPersonWithOrcidDisabled() { + provider.setPublicationListViewEnabled(true); + when(itemService.getMetadataFirstValue(item, "dspace", + "entity", "type", Item.ANY)).thenReturn("Person"); + when(itemService.getMetadataFirstValue(item, "person", + "identifier", "orcid", Item.ANY)).thenReturn("0000-0002-9029-1854"); + boolean hasMetric = provider.hasMetric(context, item, null); + assertFalse(hasMetric); + } + + @Test + public void hasMetricPersonWithoutOrcid() { + provider.setPersonListViewEnabled(true); when(itemService.getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)).thenReturn("Person"); when(itemService.getMetadataFirstValue(item, "person", "identifier", "orcid", Item.ANY)).thenReturn(null); boolean hasMetric = provider.hasMetric(context, item, null); @@ -102,13 +121,15 @@ public void hasMetricPublicationWithoutOrcid() { @Test public void innerHtmlForPersonItem() { + provider.setPersonListViewEnabled(true); + provider.setPersonDetailViewEnabled(true); when(itemService.getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)).thenReturn("Person"); when(itemService.getMetadataFirstValue(item, "person", "identifier", "orcid", Item.ANY)) .thenReturn("0000-0002-9029-1854"); String template = provider.innerHtml(context, item); - assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":true," + - "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":true," + + assertEquals("{\"data-person-badge-enabled\":true,\"list-data-person-badge-enabled\":true," + + "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":false," + "\"type\":\"Person\",\"list-type\":\"Person\",\"placeholder\":\"\",\"list-placeholder\":\"\"," + "\"src\":\"//cdn.plu.mx/widget-person.js\",\"href\":\"https://plu.mx/plum/u/?orcid=0000-0002-9029-1854\"," + "\"list-src\":\"//cdn.plu.mx/widget-person.js\"," + @@ -128,13 +149,15 @@ public void innerHtmlForPersonItem() { @Test public void innerHtmlForPublicationItem() { + provider.setPublicationListViewEnabled(true); + provider.setPublicationDetailViewEnabled(true); when(itemService.getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)).thenReturn("Publication"); when(itemService.getMetadataFirstValue(item, "dc", "identifier", "doi", Item.ANY)) .thenReturn("10.1016/j.gene.2009.04.019"); String template = provider.innerHtml(context, item); - assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":true," + - "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":true," + + assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":false," + + "\"data-publication-badge-enabled\":true,\"list-data-publication-badge-enabled\":true," + "\"type\":\"Publication\",\"list-type\":\"Publication\",\"placeholder\":\"\"," + "\"list-placeholder\":\"\",\"src\":\"//cdn.plu.mx/widget-popup.js\"," + "\"href\":\"https://plu.mx/plum/a/?doi=10.1016/j.gene.2009.04.019\"," + @@ -152,4 +175,29 @@ public void innerHtmlForPublicationItem() { "\"list-data-hide-socialmedia\":false,\"list-data-hide-citations\":false," + "\"list-data-pass-hidden-categories\":false,\"list-data-detail-same-page\":false}", template); } + + @Test + public void innerHtmlForOtherItem() { + when(itemService.getMetadataFirstValue(item, "dspace", "entity", "type", Item.ANY)).thenReturn("Other"); + String template = provider.innerHtml(context, item); + + assertEquals("{\"data-person-badge-enabled\":false,\"list-data-person-badge-enabled\":false," + + "\"data-publication-badge-enabled\":false,\"list-data-publication-badge-enabled\":false," + + "\"type\":\"Other\",\"list-type\":\"Other\",\"placeholder\":\"\",\"list-placeholder\":\"\"," + + "\"src\":\"//cdn.plu.mx/widget-popup.js\",\"href\":\"https://plu.mx/plum/a/?doi=null\"," + + "\"list-src\":\"//cdn.plu.mx/widget-popup.js\"," + + "\"list-href\":\"https://plu.mx/plum/a/?doi=null\",\"data-no-name\":false," + + "\"data-num-artifacts\":5,\"data-width\":\"350px\",\"data-no-description\":false," + + "\"data-no-stats\":false,\"data-no-thumbnail\":false,\"data-no-artifacts\":false," + + "\"data-popup\":\"left\",\"data-hide-when-empty\":false,\"data-hide-usage\":false," + + "\"data-hide-captures\":false,\"data-hide-mentions\":false,\"data-hide-socialmedia\":false," + + "\"data-hide-citations\":false,\"data-pass-hidden-categories\":false,\"data-detail-same-page\":false," + + "\"list-data-no-name\":false,\"list-data-num-artifacts\":0,\"list-data-width\":\"350px\"," + + "\"list-data-no-description\":false,\"list-data-no-stats\":false,\"list-data-no-thumbnail\":false," + + "\"list-data-no-artifacts\":false,\"list-data-popup\":\"left\",\"list-data-hide-when-empty\":false," + + "\"list-data-hide-usage\":false,\"list-data-hide-captures\":false,\"list-data-hide-mentions\":false," + + "\"list-data-hide-socialmedia\":false,\"list-data-hide-citations\":false," + + "\"list-data-pass-hidden-categories\":false,\"list-data-detail-same-page\":false}", template); + } + }