Merge pull request #311 from europeana/EA-3643-zoho-country-improve

1) org country improved to contain only id, type, and prefLabel; 2)
europeana · Mar 6, 2024 · bf7372c · bf7372c
2 parents fac7db1 + bdc614b
commit bf7372c
Show file tree

Hide file tree

Showing 11 changed files with 156 additions and 70 deletions.
diff --git a/...definitions/src/main/java/eu/europeana/entitymanagement/definitions/model/Vocabulary.java b/...definitions/src/main/java/eu/europeana/entitymanagement/definitions/model/Vocabulary.java
@@ -1,7 +1,9 @@
 package eu.europeana.entitymanagement.definitions.model;
 
+import static eu.europeana.entitymanagement.vocabulary.WebEntityFields.ID;
 import static eu.europeana.entitymanagement.vocabulary.WebEntityFields.IN_SCHEME;
 import static eu.europeana.entitymanagement.vocabulary.WebEntityFields.PREF_LABEL;
+import static eu.europeana.entitymanagement.vocabulary.WebEntityFields.TYPE;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -10,6 +12,7 @@
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonPropertyOrder;
 import com.fasterxml.jackson.annotation.JsonSetter;
 import dev.morphia.annotations.EntityListeners;
 import dev.morphia.annotations.Id;
@@ -25,6 +28,12 @@
 /**
  * class used for storing static vocabularies (e.g. europeana roles)
  */
+@JsonPropertyOrder({
+  ID,
+  TYPE,
+  PREF_LABEL,
+  IN_SCHEME
+})
 public class Vocabulary {
 
   @Transient
@@ -80,12 +89,12 @@ public String toString() {
     return String.format("Vocabulary.id: %s", getId());
   }
 
-  @JsonGetter
+  @JsonGetter(ID)
   public String getId() {
     return id;
   }
 
-  @JsonSetter
+  @JsonSetter(ID)
   public void setId(String id) {
     this.id = id;
   }
@@ -110,6 +119,7 @@ public void setPrefLabel(Map<String, String> prefLabel) {
     this.prefLabel = prefLabel;
   }
 
+  @JsonGetter(TYPE)
   public String getType() {
     return type;
   }

diff --git a/...-definitions/src/main/java/eu/europeana/entitymanagement/web/xml/model/XmlEdmCountry.java b/...-definitions/src/main/java/eu/europeana/entitymanagement/web/xml/model/XmlEdmCountry.java
@@ -0,0 +1,56 @@
+package eu.europeana.entitymanagement.web.xml.model;
+
+import static eu.europeana.entitymanagement.web.xml.model.XmlConstants.NAMESPACE_EDM;
+import static eu.europeana.entitymanagement.web.xml.model.XmlConstants.NAMESPACE_RDF;
+import static eu.europeana.entitymanagement.web.xml.model.XmlConstants.PREF_LABEL;
+import static eu.europeana.entitymanagement.web.xml.model.XmlConstants.RESOURCE;
+import static eu.europeana.entitymanagement.web.xml.model.XmlConstants.XML_PLACE;
+import java.util.ArrayList;
+import java.util.List;
+import javax.xml.bind.annotation.XmlAccessType;
+import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlAttribute;
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.XmlType;
+import eu.europeana.entitymanagement.definitions.model.Place;
+
+@XmlRootElement(namespace = NAMESPACE_EDM, name = XML_PLACE)
+@XmlAccessorType(XmlAccessType.FIELD)
+@XmlType(
+    propOrder = {
+      RESOURCE,
+      PREF_LABEL
+    })
+public class XmlEdmCountry {
+
+  @XmlAttribute(namespace = NAMESPACE_RDF, name = XmlConstants.RESOURCE)
+  private String resource;
+
+  @XmlElement(namespace = XmlConstants.NAMESPACE_SKOS, name = PREF_LABEL)
+  private List<LabelledResource> prefLabel = new ArrayList<>();
+
+  public XmlEdmCountry(Place place) {
+    this.resource=place.getAbout();
+    this.prefLabel= RdfXmlUtils.convertMapToXmlMultilingualString(place.getPrefLabel());
+  }
+
+  public XmlEdmCountry() {
+  }
+
+  public List<LabelledResource> getPrefLabel() {
+    return this.prefLabel;
+  }
+
+  public void setPrefLabel(List<LabelledResource> prefLabel) {
+    this.prefLabel=prefLabel;
+  }
+
+  public String getResource() {
+    return resource;
+  }
+
+  public void setResource(String resource) {
+    this.resource = resource;
+  }
+}
diff --git a/...itions/src/main/java/eu/europeana/entitymanagement/web/xml/model/XmlOrganizationImpl.java b/...itions/src/main/java/eu/europeana/entitymanagement/web/xml/model/XmlOrganizationImpl.java
@@ -79,7 +79,7 @@ public class XmlOrganizationImpl extends XmlBaseEntityImpl<Organization> {
   private List<XmlConceptImpl> europeanaRole = new ArrayList<>();
 
   @XmlElement(namespace = NAMESPACE_EDM, name = XML_COUNTRY)
-  private XmlPlaceImpl country;
+  private XmlEdmCountry country;
 
   @XmlElement(namespace = NAMESPACE_FOAF, name = XML_HOMEPAGE)
   private LabelledResource homepage;
@@ -122,7 +122,7 @@ public XmlOrganizationImpl(Organization organization) {
     }
 
     if(organization.getCountry() != null) {
-      this.country = new XmlPlaceImpl(organization.getCountry());
+      this.country = new XmlEdmCountry(organization.getCountry());
     }
 
     if (organization.getHomepage() != null) {
@@ -160,7 +160,7 @@ public Organization toEntityModel() throws EntityModelCreationException {
     //set country id (external dereferencers deliver only the ids, not transitive data)
     if(getCountry() != null) {
       //we need to extract the countryID as well (xml about holds the entityId)
-      entity.setCountryId(getCountry().getAbout());
+      entity.setCountryId(getCountry().getResource());
     }
 
     if (getHomepage() != null) {
@@ -198,7 +198,7 @@ public List<XmlConceptImpl> getEuropeanaRole() {
     return europeanaRole;
   }
 
-  public XmlPlaceImpl getCountry() {
+  public XmlEdmCountry getCountry() {
     return country;
   }
 

diff --git a/.../src/main/java/eu/europeana/entitymanagement/mongo/repository/EntityRecordRepository.java b/.../src/main/java/eu/europeana/entitymanagement/mongo/repository/EntityRecordRepository.java
@@ -6,7 +6,6 @@
 import static dev.morphia.query.experimental.filters.Filters.ne;
 import static dev.morphia.query.experimental.filters.Filters.or;
 import static eu.europeana.entitymanagement.definitions.EntityRecordFields.DISABLED;
-import static eu.europeana.entitymanagement.definitions.EntityRecordFields.ENTITY;
 import static eu.europeana.entitymanagement.definitions.EntityRecordFields.ENTITY_EXACT_MATCH;
 import static eu.europeana.entitymanagement.definitions.EntityRecordFields.ENTITY_ID;
 import static eu.europeana.entitymanagement.definitions.EntityRecordFields.ENTITY_MODIFIED;
@@ -87,34 +86,12 @@ public List<EntityRecord> findByEntityIds(
   }
 
   /**
-   * Find and return EntityRecord that matches the given parameters
-   *
-   * @param entityId ID of the dataset
-   * @return EntityRecord the database record
-   */
-  public EntityRecord findByEntityId(String entityId) {
-    return findEntityRecord(entityId);
-  }
-
-  /**
-   * Find and return EntityRecord that matches the given parameters
-   *
-   * @param entityId ID of the dataset
-   * @param consolidatedOnly if set to true, the retrieved record will only include the consolidated entity
-   * @return EntityRecord the database record
-   */
-  public EntityRecord findByEntityId(String entityId, boolean consolidatedOnly) {
-      return consolidatedOnly ? findEntityRecord(entityId, ENTITY) : findEntityRecord(entityId);
-  }
-
-  /**
-   * Find and return EntityRecord that matches the given parameters
-   *
-   * @param entityId ID of the dataset
-   * @param consolidatedOnly if set to true, the retrieved record will only include the consolidated entity
-   * @return EntityRecord the database record
+   * Find entity record with id, and pick only the given fields from the record
+   * @param entityId
+   * @param fields
+   * @return
    */
-  protected EntityRecord findEntityRecord(String entityId, String... fields) {
+  public EntityRecord findEntityRecord(String entityId, String... fields) {
     Query<EntityRecord> query = getDataStore()
         .find(EntityRecord.class);
 
@@ -133,7 +110,7 @@ protected EntityRecord findEntityRecord(String entityId, String... fields) {
       return query.first();
     }
   }
-
+    
   public List<EntityIdDisabledStatus> getEntityIds(
       List<String> entityIds, boolean excludeDisabled) {
     List<EntityRecord> entityRecords = findByEntityIds(entityIds, excludeDisabled, false);
@@ -205,6 +182,21 @@ public List<EntityRecord> findEntitiesByCoreference(
     return query.iterator().toList();
   }
 
+  public List<EntityRecord> findByEntityIdsOrCoreference(List<String> uris) {
+    // Get all EntityRecords that have the given uris as their entityId or in the sameAs/exactMatch field 
+    List<Filter> filters = new ArrayList<>();
+    filters.add(or(in(ENTITY_ID, uris), in(ENTITY_SAME_AS, uris), in(ENTITY_EXACT_MATCH, uris)));
+    // Only fetch active records. Disabled records have a date value
+    filters.add(eq(DISABLED, null));    
+
+    return getDataStore()
+        .find(EntityRecord.class)
+        .filter(filters.toArray(Filter[]::new))
+        .iterator()
+        .toList();
+  }
+
+
   public List<EntityRecord> saveBulk(List<EntityRecord> entityRecords) {
     return getDataStore().save(entityRecords);
   }

diff --git a/...c/integration-test/java/eu/europeana/entitymanagement/mongo/EntityRecordRepositoryIT.java b/...c/integration-test/java/eu/europeana/entitymanagement/mongo/EntityRecordRepositoryIT.java
@@ -3,13 +3,6 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-
-import eu.europeana.entitymanagement.AbstractIntegrationTest;
-import eu.europeana.entitymanagement.definitions.model.Aggregation;
-import eu.europeana.entitymanagement.definitions.model.EntityProxy;
-import eu.europeana.entitymanagement.definitions.model.EntityRecord;
-import eu.europeana.entitymanagement.definitions.model.TimeSpan;
-import eu.europeana.entitymanagement.definitions.model.WebResource;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
@@ -19,6 +12,12 @@
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
 import org.springframework.boot.test.context.SpringBootTest;
+import eu.europeana.entitymanagement.AbstractIntegrationTest;
+import eu.europeana.entitymanagement.definitions.model.Aggregation;
+import eu.europeana.entitymanagement.definitions.model.EntityProxy;
+import eu.europeana.entitymanagement.definitions.model.EntityRecord;
+import eu.europeana.entitymanagement.definitions.model.TimeSpan;
+import eu.europeana.entitymanagement.definitions.model.WebResource;
 
 @SpringBootTest
 @AutoConfigureMockMvc
@@ -32,7 +31,7 @@ public void setup() {
   @Test
   public void shouldCorrectlyInsertAndRetrieve() {
     EntityRecord savedRecord = createEntityRecord();
-    EntityRecord retrievedRecord = entityRecordRepository.findByEntityId(savedRecord.getEntityId());
+    EntityRecord retrievedRecord = entityRecordRepository.findEntityRecord(savedRecord.getEntityId());
 
     assertEquals(retrievedRecord.getEntityId(), savedRecord.getEntityId());
 
@@ -45,7 +44,7 @@ void shouldDisableEntities() {
     EntityRecord savedRecord = createEntityRecord();
     entityRecordRepository.disableBulk(List.of(savedRecord.getEntityId()));
 
-    EntityRecord retrievedRecord = entityRecordRepository.findByEntityId(savedRecord.getEntityId());
+    EntityRecord retrievedRecord = entityRecordRepository.findEntityRecord(savedRecord.getEntityId());
     assertTrue(retrievedRecord.isDisabled());
   }
 

diff --git a/...rc/integration-test/java/eu/europeana/entitymanagement/web/EntityMultipleRetrievalIT.java b/...rc/integration-test/java/eu/europeana/entitymanagement/web/EntityMultipleRetrievalIT.java
@@ -5,14 +5,15 @@
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
-
-import eu.europeana.entitymanagement.testutils.IntegrationTestUtils;
 import java.util.List;
+import java.util.Optional;
 import org.junit.jupiter.api.Test;
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.http.MediaType;
 import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
+import com.zoho.crm.api.record.Record;
+import eu.europeana.entitymanagement.testutils.IntegrationTestUtils;
 
 @SpringBootTest
 @AutoConfigureMockMvc
@@ -69,6 +70,29 @@ void multipleEntitiesRetrievedSuccessfully() throws Exception {
         .andExpect(jsonPath("$.items.*.id", is(List.of(entityId3, entityId1, entityId2))));
   }
 
+  @Test
+  void multipleEntitiesRetrieveOldOrganizationIds() throws Exception {
+    //register zoho GFM org with old id in sameAs
+    String europeanaMetadata = loadFile(IntegrationTestUtils.ORGANIZATION_REGISTER_GFM_ZOHO_JSON);
+    Optional<Record> zohoRecord =
+        IntegrationTestUtils.getZohoOrganizationRecord(
+            IntegrationTestUtils.ORGANIZATION_GFM_URI_ZOHO);
+    assert zohoRecord.isPresent() : "Mocked Zoho response not loaded";
+    String entityId = createOrganization(europeanaMetadata, zohoRecord.get()).getEntityId();
+
+    //please check that this id is also in the sameAs of the org json file (IntegrationTestUtils.ORGANIZATION_REGISTER_GFM_ZOHO_JSON)
+    String oldId="http://data.europeana.eu/organization/486281000000940433";
+
+    mockMvc
+    .perform(
+        post(IntegrationTestUtils.BASE_SERVICE_URL + "/retrieve")
+            .contentType(MediaType.APPLICATION_JSON)
+            .content(objectMapper.writeValueAsString(List.of(oldId))))
+    .andExpect(status().isOk())
+    .andExpect(jsonPath("$.items.*.id", is(List.of(entityId))));
+
+  }
+
   @Test
   void multipleEntitiesRetrievedNotFound() throws Exception {
     mockMvc

diff --git a/...-tests/src/integration-test/java/eu/europeana/entitymanagement/web/EntityRetrievalIT.java b/...-tests/src/integration-test/java/eu/europeana/entitymanagement/web/EntityRetrievalIT.java
@@ -524,6 +524,7 @@ public void retrieveOrganizationJsonExternalWithCountryAndRoleDereference() thro
         .andExpect(jsonPath("$.type", is(EntityTypes.Organization.getEntityType())))
         .andExpect(jsonPath("$.sameAs").isNotEmpty())
         .andExpect(jsonPath("$.country.prefLabel.en", is("Sweden")))
+        .andExpect(jsonPath("$.country.latitude").doesNotExist()) //only the country prefLabel, id, and type should be present
         .andExpect(jsonPath("$.europeanaRole[0].prefLabel.en", is("Providing Institution")));
   }
 
@@ -603,7 +604,8 @@ public void retrieveOrganizationXmlExternalShouldBeSuccessful() throws Exception
         .andExpect(xpath(entityBaseXpath + "/@rdf:about", xmlNamespaces).string(entityId))
         .andExpect(
             xpath(entityBaseXpath + "/skos:prefLabel", xmlNamespaces).nodeCount(greaterThan(0)))
-        .andExpect(xpath(entityBaseXpath + "/edm:countryPlace", xmlNamespaces).doesNotExist());
+        .andExpect(xpath(entityBaseXpath + "/edm:country/@rdf:resource", xmlNamespaces).exists())
+        .andExpect(xpath(entityBaseXpath + "/edm:country/skos:prefLabel", xmlNamespaces).doesNotExist());
   }
 
   @Test

diff --git a/...nagement-tests/src/integration-test/resources/content/organization_register_zoho_gfm.json b/...nagement-tests/src/integration-test/resources/content/organization_register_zoho_gfm.json
@@ -1,4 +1,7 @@
 {
   "type": "Organization",
-  "id": "https://crm.zoho.eu/crm/org20085137532/tab/Accounts/486281000000940433"
+  "id": "https://crm.zoho.eu/crm/org20085137532/tab/Accounts/486281000000940433",
+  "sameAs": [
+	  "http://data.europeana.eu/organization/486281000000940433"
+  ]
 }
diff --git a/entity-management-web/src/main/java/eu/europeana/entitymanagement/web/EMController.java b/entity-management-web/src/main/java/eu/europeana/entitymanagement/web/EMController.java
@@ -638,31 +638,17 @@ private ResponseEntity<String> createResponseForRetrieve(EntityTypes type, Strin
   private ResponseEntity<String> createResponseMultipleEntities(List<String> entityIds,
       HttpServletRequest request) throws EuropeanaApiException {
     List<EntityRecord> entityRecords =
-        entityRecordService.retrieveMultipleByEntityIds(entityIds, true, true);
+        entityRecordService.retrieveMultipleByEntityIdsOrCoreference(entityIds);
     if (entityRecords.isEmpty()) {
       throw new EntityNotFoundException(entityIds.toString());
     }
 
-    // LinkedHashMap iterates keys() and values() in order of insertion. Using a map
-    // improves sort performance significantly
-    Map<String, EntityRecord> sortedEntityRecordMap = new LinkedHashMap<>(entityIds.size());
-    for (String id : entityIds) {
-      sortedEntityRecordMap.put(id, null);
-    }
-
-    for (EntityRecord entityRecord : entityRecords) {
-      sortedEntityRecordMap.replace(entityRecord.getEntityId(), entityRecord);
-    }
-
     // create response headers
     String contentType = HttpHeaders.CONTENT_TYPE_JSONLD_UTF8;
     org.springframework.http.HttpHeaders headers = createAllowHeader(request);
     headers.add(HttpHeaders.CONTENT_TYPE, contentType);
 
-    // remove null values in response
-    List<EntityRecord> responseBody = sortedEntityRecordMap.values().stream()
-        .filter(Objects::nonNull).collect(Collectors.toList());
-    String body = serialize(responseBody);
+    String body = serialize(entityRecords);
     return ResponseEntity.status(HttpStatus.OK).headers(headers).body(body);
   }
 

diff --git a/...-web/src/main/java/eu/europeana/entitymanagement/web/service/BaseEntityRecordService.java b/...-web/src/main/java/eu/europeana/entitymanagement/web/service/BaseEntityRecordService.java
@@ -586,7 +586,7 @@ protected void processCountryReference(Organization org) {
       // replace wikidata country ids
       org.setCountryId(europeanaCountryId);
       // search reference
-      EntityRecord orgCountry = entityRecordRepository.findByEntityId(europeanaCountryId);
+      EntityRecord orgCountry = entityRecordRepository.findEntityRecord(europeanaCountryId);
       if (orgCountry != null) {
         org.setCountryRef(orgCountry);
       } else if (logger.isWarnEnabled()) {