Skip to content

Commit df33d7b

Browse files
Camelia-Orcidamontenegro
Camelia-Orcid
and
amontenegro
authored
Added the subdomain fix when processing email domains (#7239)
* Added the subdomain fix when processing email domains * Update DBUnitTest.java * removed commented statements * Fixed the failing test * Fixed verified domain not being deleted when an email address with a subdomain was deleted. * remove system.out lines --------- Co-authored-by: Angel Montenegro <a.montenegro@orcid.org>
1 parent 5e0cc91 commit df33d7b

File tree

5 files changed

+93
-35
lines changed

5 files changed

+93
-35
lines changed

‎orcid-core/src/main/java/org/orcid/core/manager/v3/ProfileEmailDomainManager.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.orcid.core.manager.v3;
22

33
import org.orcid.core.manager.v3.read_only.ProfileEmailDomainManagerReadOnly;
4+
import org.orcid.jaxb.model.v3.release.record.Emails;
45
import org.orcid.persistence.jpa.entities.EmailEntity;
56

67
import java.util.List;
@@ -11,7 +12,7 @@
1112
*
1213
*/
1314
public interface ProfileEmailDomainManager extends ProfileEmailDomainManagerReadOnly {
14-
void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails emails);
15+
void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails emails, org.orcid.jaxb.model.v3.release.record.Emails updatedEmailSet);
1516
void processDomain(String orcid, String email);
1617
void removeAllEmailDomains(String orcid);
1718
void moveEmailDomainToAnotherAccount(String emailDomain, String deprecatedOrcid, String primaryOrcid);

‎orcid-core/src/main/java/org/orcid/core/manager/v3/impl/ProfileEmailDomainManagerImpl.java

+38-22
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
package org.orcid.core.manager.v3.impl;
22

3-
3+
import org.apache.commons.lang3.StringUtils;
4+
import org.orcid.core.common.manager.EmailDomainManager;
45
import org.orcid.core.manager.ProfileEntityCacheManager;
56
import org.orcid.core.manager.v3.ProfileEmailDomainManager;
67
import org.orcid.core.manager.v3.read_only.impl.ProfileEmailDomainManagerReadOnlyImpl;
8+
import org.orcid.core.utils.emailDomain.EmailDomainValidator;
79
import org.orcid.jaxb.model.v3.release.common.Visibility;
10+
import org.orcid.jaxb.model.v3.release.record.Emails;
811
import org.orcid.persistence.dao.EmailDao;
912
import org.orcid.persistence.dao.EmailDomainDao;
1013
import org.orcid.persistence.dao.ProfileDao;
@@ -13,12 +16,11 @@
1316
import org.orcid.persistence.jpa.entities.EmailEntity;
1417
import org.orcid.persistence.jpa.entities.ProfileEmailDomainEntity;
1518
import org.orcid.persistence.jpa.entities.ProfileEntity;
19+
import org.orcid.pojo.ajaxForm.Email;
1620
import org.slf4j.Logger;
1721
import org.slf4j.LoggerFactory;
1822
import org.springframework.transaction.annotation.Transactional;
1923

20-
import liquibase.repackaged.org.apache.commons.lang3.StringUtils;
21-
2224
import javax.annotation.Resource;
2325
import java.util.*;
2426

@@ -42,8 +44,11 @@ public class ProfileEmailDomainManagerImpl extends ProfileEmailDomainManagerRead
4244
@Resource
4345
private ProfileEntityCacheManager profileEntityCacheManager;
4446

47+
@Resource
48+
private EmailDomainManager emailDomainManager;
49+
4550
@Transactional
46-
public void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails newEmails) {
51+
public void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails newEmails, org.orcid.jaxb.model.v3.release.record.Emails updatedEmailSet) {
4752
if (orcid == null || orcid.isBlank()) {
4853
throw new IllegalArgumentException("ORCID must not be empty");
4954
}
@@ -53,18 +58,29 @@ public void updateEmailDomains(String orcid, org.orcid.pojo.ajaxForm.Emails newE
5358
// VISIBILITY UPDATE FOR EXISTING DOMAINS
5459
for (org.orcid.pojo.ajaxForm.ProfileEmailDomain emailDomain : newEmails.getEmailDomains()) {
5560
for (ProfileEmailDomainEntity existingEmailDomain : existingEmailDomains) {
56-
if (existingEmailDomain.getEmailDomain().equals(emailDomain.getValue())) {
57-
if (!existingEmailDomain.getVisibility().equals(emailDomain.getVisibility())) {
61+
if (StringUtils.equals(existingEmailDomain.getEmailDomain(), emailDomain.getValue())) {
62+
if (!StringUtils.equals(existingEmailDomain.getVisibility(), emailDomain.getVisibility())) {
5863
profileEmailDomainDao.updateVisibility(orcid, emailDomain.getValue(), emailDomain.getVisibility());
5964
}
6065
}
6166
}
6267
}
68+
6369
// REMOVE DOMAINS
70+
Set<EmailDomainEntity> edSet = new HashSet<EmailDomainEntity>();
71+
if (updatedEmailSet != null && updatedEmailSet.getEmails() != null) {
72+
for (org.orcid.jaxb.model.v3.release.record.Email email : updatedEmailSet.getEmails()) {
73+
if (email.isVerified() && StringUtils.isNotBlank(email.getEmail())) {
74+
edSet.addAll(emailDomainManager.findByEmailDomain(email.getEmail().split("@")[1]));
75+
}
76+
}
77+
}
78+
6479
for (ProfileEmailDomainEntity existingEmailDomain : existingEmailDomains) {
6580
boolean deleteEmail = true;
66-
for (org.orcid.pojo.ajaxForm.ProfileEmailDomain emailDomain : newEmails.getEmailDomains()) {
67-
if (existingEmailDomain.getEmailDomain().equals(emailDomain.getValue())) {
81+
for (EmailDomainEntity emailDomain : edSet) {
82+
if (StringUtils.equals(existingEmailDomain.getEmailDomain(), emailDomain.getEmailDomain())
83+
&& StringUtils.equals(emailDomain.getCategory().name(), EmailDomainEntity.DomainCategory.PROFESSIONAL.name())) {
6884
deleteEmail = false;
6985
break;
7086
}
@@ -85,28 +101,28 @@ public void processDomain(String orcid, String email) {
85101
}
86102

87103
String domain = email.split("@")[1];
88-
List<EmailDomainEntity> domainsInfo = emailDomainDao.findByEmailDomain(domain);
104+
List<EmailDomainEntity> domainsInfo = emailDomainManager.findByEmailDomain(domain);
89105
String category = EmailDomainEntity.DomainCategory.UNDEFINED.name();
90-
91-
106+
92107
// Check if email is professional
93108
if (domainsInfo != null) {
94-
for(EmailDomainEntity domainInfo: domainsInfo) {
109+
for (EmailDomainEntity domainInfo : domainsInfo) {
95110
category = domainInfo.getCategory().name();
96-
if(StringUtils.equalsIgnoreCase(category, EmailDomainEntity.DomainCategory.PROFESSIONAL.name())) {
111+
if (StringUtils.equalsIgnoreCase(category, EmailDomainEntity.DomainCategory.PROFESSIONAL.name())) {
112+
domain = domainInfo.getEmailDomain();
97113
break;
98114
}
99115
}
100-
if(StringUtils.equalsIgnoreCase(category, EmailDomainEntity.DomainCategory.PROFESSIONAL.name())) {
101-
ProfileEmailDomainEntity existingDomain = profileEmailDomainDaoReadOnly.findByEmailDomain(orcid, domain);
102-
// ADD NEW DOMAIN IF ONE DOESN'T EXIST
103-
if (existingDomain == null) {
104-
// Verify the user doesn't have more emails with that domain
105-
ProfileEntity profile = profileEntityCacheManager.retrieve(orcid);
106-
String domainVisibility = profile.getActivitiesVisibilityDefault();
107-
profileEmailDomainDao.addEmailDomain(orcid, domain, domainVisibility);
116+
if (StringUtils.equalsIgnoreCase(category, EmailDomainEntity.DomainCategory.PROFESSIONAL.name())) {
117+
ProfileEmailDomainEntity existingDomain = profileEmailDomainDaoReadOnly.findByEmailDomain(orcid, domain);
118+
// ADD NEW DOMAIN IF ONE DOESN'T EXIST
119+
if (existingDomain == null) {
120+
// Verify the user doesn't have more emails with that domain
121+
ProfileEntity profile = profileEntityCacheManager.retrieve(orcid);
122+
String domainVisibility = profile.getActivitiesVisibilityDefault();
123+
profileEmailDomainDao.addEmailDomain(orcid, domain, domainVisibility);
124+
}
108125
}
109-
}
110126
}
111127
}
112128

‎orcid-core/src/test/java/org/orcid/core/manager/v3/ProfileEmailDomainManagerTest.java

+44-9
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@
77
import static org.mockito.Mockito.verify;
88
import static org.mockito.Mockito.when;
99

10+
import java.util.ArrayList;
1011
import java.util.Date;
1112
import java.util.List;
1213

1314
import org.junit.Before;
1415
import org.junit.Test;
1516
import org.mockito.Mock;
1617
import org.mockito.MockitoAnnotations;
18+
import org.orcid.core.common.manager.EmailDomainManager;
1719
import org.orcid.core.manager.ProfileEntityCacheManager;
1820
import org.orcid.core.manager.v3.impl.ProfileEmailDomainManagerImpl;
1921
import org.orcid.jaxb.model.v3.release.common.Visibility;
@@ -38,6 +40,9 @@ public class ProfileEmailDomainManagerTest {
3840

3941
@Mock
4042
private EmailDomainDao emailDomainDaoMock;
43+
44+
@Mock
45+
private EmailDomainManager emailDomainManagerMock;
4146

4247
ProfileEmailDomainManager pedm = new ProfileEmailDomainManagerImpl();
4348

@@ -56,6 +61,7 @@ public void before() {
5661
TargetProxyHelper.injectIntoProxy(pedm, "profileEmailDomainDaoReadOnly", profileEmailDomainDaoReadOnlyMock);
5762
TargetProxyHelper.injectIntoProxy(pedm, "emailDomainDao", emailDomainDaoMock);
5863
TargetProxyHelper.injectIntoProxy(pedm, "profileEntityCacheManager", profileEntityCacheManagerMock);
64+
TargetProxyHelper.injectIntoProxy(pedm, "emailDomainManager", emailDomainManagerMock);
5965

6066
ProfileEmailDomainEntity ped1 = new ProfileEmailDomainEntity();
6167
ProfileEmailDomainEntity ped2 = new ProfileEmailDomainEntity();
@@ -72,7 +78,16 @@ public void before() {
7278
ped3.setEmailDomain(EMAIL_DOMAIN);
7379
ped3.setOrcid(ORCID_TWO);
7480
ped3.setVisibility(Visibility.PUBLIC.value());
75-
81+
82+
EmailDomainEntity e1 = new EmailDomainEntity();
83+
e1.setEmailDomain(EMAIL_DOMAIN);
84+
e1.setCategory(DomainCategory.PROFESSIONAL);
85+
86+
EmailDomainEntity e2 = new EmailDomainEntity();
87+
e2.setEmailDomain(EMAIL_DOMAIN_TWO);
88+
e2.setCategory(DomainCategory.PROFESSIONAL);
89+
90+
7691
when(profileEmailDomainDaoReadOnlyMock.findByEmailDomain(eq(ORCID), eq(EMAIL_DOMAIN))).thenReturn(ped1);
7792
when(profileEmailDomainDaoReadOnlyMock.findByEmailDomain(eq(ORCID), eq(EMAIL_DOMAIN_TWO))).thenReturn(ped2);
7893
when(profileEmailDomainDaoReadOnlyMock.findByEmailDomain(eq(ORCID_TWO), eq(EMAIL_DOMAIN))).thenReturn(ped3);
@@ -87,6 +102,9 @@ public void before() {
87102
when(profileEmailDomainDaoMock.addEmailDomain(eq(ORCID), eq(EMAIL_DOMAIN_TWO), eq(Visibility.LIMITED.value()))).thenReturn(ped2);
88103

89104
when(profileEmailDomainDaoMock.updateVisibility(eq(ORCID), eq(EMAIL_DOMAIN_TWO), eq(Visibility.LIMITED.value()))).thenReturn(true);
105+
106+
when(emailDomainManagerMock.findByEmailDomain(eq(EMAIL_DOMAIN))).thenReturn(List.of(e1));
107+
when(emailDomainManagerMock.findByEmailDomain(eq(EMAIL_DOMAIN_TWO))).thenReturn(List.of(e2));
90108

91109
ProfileEntity profile = new ProfileEntity();
92110
profile.setActivitiesVisibilityDefault(Visibility.PUBLIC.value());
@@ -108,15 +126,15 @@ public void processDomain_domainAlreadyAdded() {
108126
EmailDomainEntity professionalEmailDomain = new EmailDomainEntity();
109127
professionalEmailDomain.setCategory(DomainCategory.PROFESSIONAL);
110128
professionalEmailDomain.setEmailDomain(EMAIL_DOMAIN);
111-
when(emailDomainDaoMock.findByEmailDomain(eq(EMAIL_DOMAIN))).thenReturn(List.of(professionalEmailDomain));
129+
when(emailDomainManagerMock.findByEmailDomain(eq(EMAIL_DOMAIN))).thenReturn(List.of(professionalEmailDomain));
112130
pedm.processDomain(ORCID, "email@orcid.org");
113131
verify(profileEmailDomainDaoReadOnlyMock, times(1)).findByEmailDomain(eq(ORCID), eq(EMAIL_DOMAIN));
114132
verify(profileEmailDomainDaoMock, never()).addEmailDomain(anyString(), anyString(), anyString());
115133
}
116134

117135
@Test
118136
public void processDomain_doNotAddUnknownDomain() {
119-
when(emailDomainDaoMock.findByEmailDomain(eq(EMAIL_DOMAIN))).thenReturn(null);
137+
when(emailDomainManagerMock.findByEmailDomain(eq(EMAIL_DOMAIN))).thenReturn(null);
120138
pedm.processDomain(ORCID, "email@orcid.org");
121139
verify(profileEmailDomainDaoReadOnlyMock, never()).findByEmailDomain(anyString(), anyString());
122140
verify(profileEmailDomainDaoMock, never()).addEmailDomain(anyString(), anyString(), anyString());
@@ -127,7 +145,7 @@ public void processDomain_doNotAddPersonalDomain() {
127145
EmailDomainEntity professionalEmailDomain = new EmailDomainEntity();
128146
professionalEmailDomain.setCategory(DomainCategory.PERSONAL);
129147
professionalEmailDomain.setEmailDomain(EMAIL_DOMAIN);
130-
when(emailDomainDaoMock.findByEmailDomain(eq(EMAIL_DOMAIN))).thenReturn(List.of(professionalEmailDomain));
148+
when(emailDomainManagerMock.findByEmailDomain(eq(EMAIL_DOMAIN))).thenReturn(List.of(professionalEmailDomain));
131149
pedm.processDomain(ORCID, "email@orcid.org");
132150
verify(profileEmailDomainDaoReadOnlyMock, never()).findByEmailDomain(anyString(), anyString());
133151
verify(profileEmailDomainDaoMock, never()).addEmailDomain(anyString(), anyString(), anyString());
@@ -138,15 +156,15 @@ public void processDomain_addDomain() {
138156
EmailDomainEntity professionalEmailDomain = new EmailDomainEntity();
139157
professionalEmailDomain.setCategory(DomainCategory.PROFESSIONAL);
140158
professionalEmailDomain.setEmailDomain(EMAIL_DOMAIN_THREE);
141-
when(emailDomainDaoMock.findByEmailDomain(eq(EMAIL_DOMAIN_THREE))).thenReturn(List.of(professionalEmailDomain));
159+
when(emailDomainManagerMock.findByEmailDomain(eq(EMAIL_DOMAIN_THREE))).thenReturn(List.of(professionalEmailDomain));
142160
pedm.processDomain(ORCID, "email@domain.net");
143161
verify(profileEmailDomainDaoReadOnlyMock, times(1)).findByEmailDomain(eq(ORCID), eq(EMAIL_DOMAIN_THREE));
144162
verify(profileEmailDomainDaoMock, times(1)).addEmailDomain(ORCID, EMAIL_DOMAIN_THREE, Visibility.PUBLIC.value());
145163
}
146164

147165
@Test(expected = IllegalArgumentException.class)
148166
public void updateEmailDomains_nullOrcid() {
149-
pedm.updateEmailDomains(null, new org.orcid.pojo.ajaxForm.Emails());
167+
pedm.updateEmailDomains(null, new org.orcid.pojo.ajaxForm.Emails(), new org.orcid.jaxb.model.v3.release.record.Emails());
150168
}
151169

152170
@Test
@@ -159,7 +177,18 @@ public void updateEmailDomains_updateVisibility() {
159177
ed2.setVisibility(Visibility.PRIVATE.value());
160178
ed2.setValue(EMAIL_DOMAIN_TWO);
161179
emails.setEmailDomains(List.of(ed1, ed2));
162-
pedm.updateEmailDomains(ORCID, emails);
180+
181+
//current emails
182+
org.orcid.jaxb.model.v3.release.record.Emails recordEmails = new org.orcid.jaxb.model.v3.release.record.Emails();
183+
org.orcid.jaxb.model.v3.release.record.Email email1 = new org.orcid.jaxb.model.v3.release.record.Email();
184+
email1.setEmail("one@" + EMAIL_DOMAIN);
185+
email1.setVerified(true);
186+
org.orcid.jaxb.model.v3.release.record.Email email2 = new org.orcid.jaxb.model.v3.release.record.Email();
187+
email2.setEmail("two@" + EMAIL_DOMAIN_TWO);
188+
email2.setVerified(true);
189+
recordEmails.setEmails(List.of(email1, email2));
190+
pedm.updateEmailDomains(ORCID, emails, recordEmails);
191+
163192
verify(profileEmailDomainDaoMock, times(1)).updateVisibility(ORCID, EMAIL_DOMAIN, Visibility.LIMITED.value());
164193
verify(profileEmailDomainDaoMock, times(1)).updateVisibility(ORCID, EMAIL_DOMAIN_TWO, Visibility.PRIVATE.value());
165194
verify(profileEmailDomainDaoMock, never()).removeEmailDomain(anyString(), anyString());
@@ -173,7 +202,13 @@ public void updateEmailDomains_makeNoChanges() {
173202
ed1.setVisibility(Visibility.PUBLIC.value());
174203
ed1.setValue(EMAIL_DOMAIN);
175204
emails.setEmailDomains(List.of(ed1));
176-
pedm.updateEmailDomains(ORCID_TWO, emails);
205+
//current emails
206+
org.orcid.jaxb.model.v3.release.record.Emails recordEmails = new org.orcid.jaxb.model.v3.release.record.Emails();
207+
org.orcid.jaxb.model.v3.release.record.Email email1 = new org.orcid.jaxb.model.v3.release.record.Email();
208+
email1.setEmail("one@" + EMAIL_DOMAIN);
209+
email1.setVerified(true);
210+
recordEmails.setEmails(List.of(email1));
211+
pedm.updateEmailDomains(ORCID_TWO, emails, recordEmails);
177212
verify(profileEmailDomainDaoMock, never()).updateVisibility(anyString(), anyString(), anyString());
178213
verify(profileEmailDomainDaoMock, never()).removeEmailDomain(anyString(), anyString());
179214
}
@@ -182,7 +217,7 @@ public void updateEmailDomains_makeNoChanges() {
182217
public void updateEmailDomains_removeDomain() {
183218
org.orcid.pojo.ajaxForm.Emails emails = new org.orcid.pojo.ajaxForm.Emails();
184219
emails.setEmailDomains(List.of(new ProfileEmailDomain()));
185-
pedm.updateEmailDomains(ORCID, emails);
220+
pedm.updateEmailDomains(ORCID, emails, new org.orcid.jaxb.model.v3.release.record.Emails());
186221
verify(profileEmailDomainDaoMock, never()).updateVisibility(anyString(), anyString(), anyString());
187222
verify(profileEmailDomainDaoMock, times(1)).removeEmailDomain(ORCID, EMAIL_DOMAIN);
188223
verify(profileEmailDomainDaoMock, times(1)).removeEmailDomain(ORCID, EMAIL_DOMAIN_TWO);

‎orcid-test/src/main/java/org/orcid/test/DBUnitTest.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import java.sql.Connection;
66
import java.sql.SQLException;
7+
import java.sql.Statement;
78
import java.util.List;
89

910
import org.dbunit.DatabaseUnitException;
@@ -116,13 +117,18 @@ private static void clearCaches(CacheManager cacheManager) {
116117
}
117118

118119
private static void cleanClientSourcedProfiles(IDatabaseConnection connection) throws AmbiguousTableNameException, DatabaseUnitException, SQLException {
120+
119121
QueryDataSet grandChildTableSet = new QueryDataSet(connection);
120122
grandChildTableSet.addTable("research_resource_item_org");
123+
grandChildTableSet.addTable("client_secret");
124+
grandChildTableSet.addTable("external_identifier");
125+
grandChildTableSet.addTable("email");
121126
DatabaseOperation.DELETE.execute(connection, grandChildTableSet);
122127

123128
QueryDataSet childTableSet = new QueryDataSet(connection);
124129
childTableSet.addTable("research_resource_item");
125130
childTableSet.addTable("research_resource_org");
131+
childTableSet.addTable("profile_email_domain");
126132
DatabaseOperation.DELETE.execute(connection, childTableSet);
127133

128134
QueryDataSet dataSet = new QueryDataSet(connection);
@@ -135,7 +141,6 @@ private static void cleanClientSourcedProfiles(IDatabaseConnection connection) t
135141
dataSet.addTable("work");
136142
dataSet.addTable("profile_event");
137143
dataSet.addTable("researcher_url");
138-
dataSet.addTable("email");
139144
dataSet.addTable("email_domain");
140145
dataSet.addTable("email_event");
141146
dataSet.addTable("external_identifier");
@@ -168,6 +173,7 @@ private static void cleanClientSourcedProfiles(IDatabaseConnection connection) t
168173
theRest.addTable("client_secret");
169174
theRest.addTable("custom_email");
170175
DatabaseOperation.DELETE.execute(connection, theRest);
176+
171177
}
172178

173179
private static void cleanAll(IDatabaseConnection connection) throws DatabaseUnitException, SQLException {

‎orcid-web/src/main/java/org/orcid/frontend/web/controllers/ManageProfileController.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -606,13 +606,13 @@ public ModelAndView confirmDeactivateOrcidAccount(HttpServletRequest request, Ht
606606
}
607607

608608
for (org.orcid.jaxb.model.v3.release.record.Email deletedEmail : deletedEmails) {
609-
deleteEmailJson ( deletedEmail.getEmail() );
609+
deleteEmailJson ( deletedEmail.getEmail() );
610610
}
611611

612612
Emails updatedSet = emailManager.getEmails(getCurrentUserOrcid());
613613
List<ProfileEmailDomainEntity> updatedDomains = null;
614614
if (Features.EMAIL_DOMAINS.isActive()) {
615-
profileEmailDomainManager.updateEmailDomains(orcid, newEmailSet);
615+
profileEmailDomainManager.updateEmailDomains(orcid, newEmailSet, updatedSet);
616616
updatedDomains = profileEmailDomainManagerReadOnly.getEmailDomains(getCurrentUserOrcid());
617617
}
618618
org.orcid.pojo.ajaxForm.Emails emailsResponse = org.orcid.pojo.ajaxForm.Emails.valueOf(updatedSet, updatedDomains);

0 commit comments

Comments
 (0)