update organization fields (unmap from zoho or permanently remove) #289
Conversation
…peana/entity-management.git into EA-3641-zoho-fields-updates
over secondary entity)
country to europeana place; 3) lazy retrieval of the country reference
| @@ -151,6 +151,9 @@ public class EntityManagementConfiguration implements InitializingBean { | |||
|
|
|||
| @Value("${spring.profiles.active:}") | |||
| private String activeProfileString; | |||
|
|
|||
| @Value("${zoho.country.mapping.file:#{null}}") | |||
There was a problem hiding this comment.
this is not needed to be configurable, it can be placed in resources as it doesn't contain sentitive or dinamic information
| @@ -360,6 +363,10 @@ public boolean isGenerateOrganizationEuropeanaId() { | |||
| return generateOrganizationEuropeanaId; | |||
| } | |||
|
|
|||
| public String getZohoCountryMappingFile() { | |||
There was a problem hiding this comment.
not needed, it is sufficient to have it in the services that reads the file
|
|
||
| Address that = (Address) o; | ||
|
|
||
| if (!Objects.equals(about, that.getAbout())) return false; |
There was a problem hiding this comment.
objects.equals takes care of the null values, therefore the if statements and returns are redundant, it is sufficient to bind these stamenetns with && operator
| } | ||
|
|
||
| public int hashCode() { | ||
| final int prime = 31; |
There was a problem hiding this comment.
to complicated, it is sufficient to have the hascode based on the ones of locality, streetaddress and hasGeo, and also it is sufficient to sumup the hasCodes of the 3 fields if they exists (they are uniquely identifying the place, and if not, the equals is used anyway)
|
|
||
| public static String getEntityUriFromName(List<CountryMapping> list ,String name) { | ||
| for(CountryMapping cm : list) { | ||
| List<String> splittedAndTrimmed = Arrays.stream(cm.getZohoLabel().split(",")).map(String::trim).toList(); |
There was a problem hiding this comment.
is this method used anywehre? If not, please remove
There was a problem hiding this comment.
it is used (you can see it by right click on the method name in eclipse and then "open call hierarchy")
There was a problem hiding this comment.
well, I don't have the branch yet in eclipse ..
| @@ -170,7 +170,12 @@ private String getEntityFromURL(String urlToRead) throws WikidataAccessException | |||
| } | |||
|
|
|||
| private WikidataOrganization parse(String xml) throws JAXBException { | |||
| //this commented out code caused exception, i.e. Caused by: org.xml.sax.SAXParseException: Premature end of file. | |||
There was a problem hiding this comment.
please remove the commented out code
| // if (StringUtils.isBlank(organizationCountry)) { | ||
| // return null; | ||
| // } else { | ||
| // String isoCode = null; |
There was a problem hiding this comment.
this implementation doesn't look correct, please completely remove the code
| .accept(MediaType.APPLICATION_JSON)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.id", is(entityId))) | ||
| .andExpect(jsonPath("$.type", is(EntityTypes.Organization.getEntityType()))) | ||
| .andExpect(jsonPath("$.sameAs").isNotEmpty()); | ||
| .andExpect(jsonPath("$.sameAs").isNotEmpty()) | ||
| .andExpect(jsonPath("$.countryId").isNotEmpty()) |
There was a problem hiding this comment.
please use the exact value for comparison instead of not empty
| .andExpect(jsonPath("$.sameAs").isNotEmpty()); | ||
| .andExpect(jsonPath("$.sameAs").isNotEmpty()) | ||
| .andExpect(jsonPath("$.countryId").isNotEmpty()) | ||
| .andExpect(jsonPath("$.countryPlace").isNotEmpty()); |
There was a problem hiding this comment.
please use the exact value for comparison instead of not empty
| @@ -504,12 +509,14 @@ public void retrieveOrganizationJsonExternalShouldBeSuccessful() throws Exceptio | |||
| mockMvc | |||
| .perform( | |||
| get(IntegrationTestUtils.BASE_SERVICE_URL + "/" + requestPath + ".jsonld") | |||
| .param(WebEntityConstants.QUERY_PARAM_PROFILE, "external") | |||
| .param(WebEntityConstants.QUERY_PARAM_PROFILE, "external, dereference") | |||
There was a problem hiding this comment.
please ad a test without dereference profile and verify that the dereferencing is not performed
improved sonar issues; 3) other code refactorings
be serialized, Country should not be stored in mongo #EA-3641
zoho data generator #EA-3641
|
the integration tests are working locally, but fails to load country mapping in github, still it should load from test country mapping and not from resources, will need to fix this later |
No description provided.