Skip to content

Commit 37ff736

Browse files
rushtongrushtong
authored
DT-1662: Ensure single Library Card per User (#2536)
1 parent fc0ec26 commit 37ff736

23 files changed

+508
-479
lines changed

‎src/main/java/org/broadinstitute/consent/http/db/LibraryCardDAO.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ WITH daa_deletes AS (DELETE FROM lc_daa lc_daa WHERE lc_daa.lc_id = :libraryCard
8989
LEFT JOIN lc_daa ld ON lc.id = ld.lc_id
9090
WHERE lc.user_id = :userId
9191
""")
92-
List<LibraryCard> findLibraryCardsByUserId(@Bind("userId") Integer userId);
92+
LibraryCard findLibraryCardByUserId(@Bind("userId") Integer userId);
9393

9494
@RegisterBeanMapper(value = LibraryCard.class)
9595
@UseRowReducer(LibraryCardReducer.class)
@@ -115,7 +115,7 @@ WITH daa_deletes AS (DELETE FROM lc_daa lc_daa WHERE lc_daa.lc_id = :libraryCard
115115
@UseRowReducer(LibraryCardReducer.class)
116116
@SqlQuery("SELECT * FROM library_card " +
117117
"WHERE user_email = :email")
118-
List<LibraryCard> findAllLibraryCardsByUserEmail(@Bind("email") String email);
118+
LibraryCard findLibraryCardByUserEmail(@Bind("email") String email);
119119

120120
@SqlUpdate("DELETE FROM library_card WHERE user_id = :userId OR create_user_id = :userId OR update_user_id = :userId")
121121
void deleteAllLibraryCardsByUser(@Bind("userId") Integer userId);

‎src/main/java/org/broadinstitute/consent/http/db/UserDAO.java

Lines changed: 62 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,38 @@ public interface UserDAO extends Transactional<UserDAO> {
2828
@RegisterBeanMapper(value = User.class, prefix = "u")
2929
@RegisterBeanMapper(value = UserRole.class)
3030
@RegisterBeanMapper(value = Institution.class, prefix = "i")
31+
@RegisterBeanMapper(value = LibraryCard.class, prefix = "lc")
3132
@UseRowReducer(UserWithRolesReducer.class)
32-
@SqlQuery("SELECT "
33-
+ User.QUERY_FIELDS_WITH_U_PREFIX + QUERY_FIELD_SEPARATOR
34-
+ Institution.QUERY_FIELDS_WITH_I_PREFIX + QUERY_FIELD_SEPARATOR
35-
+ " ur.user_role_id, ur.user_id, ur.role_id, ur.dac_id, r.name "
36-
+ " FROM users u "
37-
+ " LEFT JOIN user_role ur ON ur.user_id = u.user_id "
38-
+ " LEFT JOIN roles r ON r.role_id = ur.role_id "
39-
+ " LEFT JOIN institution i ON u.institution_id = i.institution_id"
40-
+ " WHERE u.user_id = :userId")
33+
@SqlQuery("""
34+
SELECT
35+
u.user_id as u_user_id,
36+
u.email as u_email,
37+
u.display_name as u_display_name,
38+
u.create_date as u_create_date,
39+
u.email_preference as u_email_preference,
40+
u.institution_id as u_institution_id,
41+
u.era_commons_id as u_era_commons_id,
42+
i.institution_id as i_id,
43+
i.institution_name as i_name,
44+
i.it_director_name as i_it_director_name,
45+
i.it_director_email as i_it_director_email,
46+
i.create_date as i_create_date,
47+
i.update_date as i_update_date,
48+
ur.user_role_id as ur_user_role_id, ur.user_id as ur_user_id,
49+
ur.role_id as ur_role_id, ur.dac_id as ur_dac_id, r.name as ur_name,
50+
lc.id AS lc_id, lc.user_id AS lc_user_id,
51+
lc.user_name AS lc_user_name, lc.user_email AS lc_user_email,
52+
lc.create_user_id AS lc_create_user_id, lc.create_date AS lc_create_date,
53+
lc.update_user_id AS lc_update_user_id,
54+
ld.daa_id as lc_daa_id
55+
FROM users u
56+
LEFT JOIN user_role ur ON ur.user_id = u.user_id
57+
LEFT JOIN roles r ON r.role_id = ur.role_id
58+
LEFT JOIN institution i ON u.institution_id = i.institution_id
59+
LEFT JOIN library_card lc ON lc.user_id = u.user_id
60+
LEFT JOIN lc_daa ld ON lc.id = ld.lc_id
61+
WHERE u.user_id = :userId
62+
""")
4163
User findUserById(@Bind("userId") Integer userId);
4264

4365
@RegisterBeanMapper(value = User.class, prefix = "u")
@@ -93,17 +115,38 @@ public interface UserDAO extends Transactional<UserDAO> {
93115
@RegisterBeanMapper(value = User.class, prefix = "u")
94116
@RegisterBeanMapper(value = UserRole.class, prefix = "ur")
95117
@RegisterBeanMapper(value = Institution.class, prefix = "i")
118+
@RegisterBeanMapper(value = LibraryCard.class, prefix = "lc")
96119
@UseRowReducer(UserWithRolesReducer.class)
97-
@SqlQuery("SELECT "
98-
+ User.QUERY_FIELDS_WITH_U_PREFIX + QUERY_FIELD_SEPARATOR
99-
+ Institution.QUERY_FIELDS_WITH_I_PREFIX + QUERY_FIELD_SEPARATOR
100-
+ " ur.user_role_id as ur_user_role_id, ur.user_id as ur_user_id, "
101-
+ " ur.role_id as ur_role_id, ur.dac_id as ur_dac_id, r.name as ur_name "
102-
+ " FROM users u "
103-
+ " LEFT JOIN user_role ur ON ur.user_id = u.user_id "
104-
+ " LEFT JOIN roles r ON r.role_id = ur.role_id "
105-
+ " LEFT JOIN institution i ON u.institution_id = i.institution_id"
106-
+ " WHERE LOWER(u.email) = LOWER(:email)")
120+
@SqlQuery("""
121+
SELECT
122+
u.user_id as u_user_id,
123+
u.email as u_email,
124+
u.display_name as u_display_name,
125+
u.create_date as u_create_date,
126+
u.email_preference as u_email_preference,
127+
u.institution_id as u_institution_id,
128+
u.era_commons_id as u_era_commons_id,
129+
i.institution_id as i_id,
130+
i.institution_name as i_name,
131+
i.it_director_name as i_it_director_name,
132+
i.it_director_email as i_it_director_email,
133+
i.create_date as i_create_date,
134+
i.update_date as i_update_date,
135+
ur.user_role_id as ur_user_role_id, ur.user_id as ur_user_id,
136+
ur.role_id as ur_role_id, ur.dac_id as ur_dac_id, r.name as ur_name,
137+
lc.id AS lc_id, lc.user_id AS lc_user_id,
138+
lc.user_name AS lc_user_name, lc.user_email AS lc_user_email,
139+
lc.create_user_id AS lc_create_user_id, lc.create_date AS lc_create_date,
140+
lc.update_user_id AS lc_update_user_id,
141+
ld.daa_id as lc_daa_id
142+
FROM users u
143+
LEFT JOIN user_role ur ON ur.user_id = u.user_id
144+
LEFT JOIN roles r ON r.role_id = ur.role_id
145+
LEFT JOIN institution i ON u.institution_id = i.institution_id
146+
LEFT JOIN library_card lc ON lc.user_id = u.user_id
147+
LEFT JOIN lc_daa ld ON lc.id = ld.lc_id
148+
WHERE LOWER(u.email) = LOWER(:email)
149+
""")
107150
User findUserByEmail(@Bind("email") String email);
108151

109152
@RegisterBeanMapper(value = User.class, prefix = "u")
@@ -176,19 +219,6 @@ Integer insertUser(@Bind("email") String email,
176219
List<User> describeUsersByRoleAndEmailPreference(@Bind("roleName") String roleName,
177220
@Bind("emailPreference") Boolean emailPreference);
178221

179-
@RegisterBeanMapper(value = User.class, prefix = "u")
180-
@RegisterBeanMapper(value = UserRole.class)
181-
@UseRowReducer(UserWithRolesReducer.class)
182-
@SqlQuery("SELECT "
183-
+ User.QUERY_FIELDS_WITH_U_PREFIX + QUERY_FIELD_SEPARATOR
184-
+ " ur.user_role_id, ur.user_id, ur.role_id, ur.dac_id, r.name "
185-
+ " FROM users u "
186-
+ " LEFT JOIN user_role ur ON ur.user_id = u.user_id "
187-
+ " LEFT JOIN roles r ON r.role_id = ur.role_id "
188-
+ " WHERE LOWER(u.email) = LOWER(:email) "
189-
+ " AND r.role_id = :roleId")
190-
User findUserByEmailAndRoleId(@Bind("email") String email, @Bind("roleId") Integer roleId);
191-
192222
@UseRowMapper(UserWithRolesMapper.class)
193223
@SqlQuery("select du.*, r.role_id, r.name, ur.user_role_id, ur.user_id, ur.role_id, ur.dac_id " +
194224
" from users du " +

‎src/main/java/org/broadinstitute/consent/http/db/mapper/DarCollectionReducer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public void accumulate(Map<Integer, DarCollection> map, RowView rowView) {
9494
user.setInstitution(institution);
9595
}
9696
if (Objects.nonNull(libraryCard)) {
97-
user.addLibraryCard(libraryCard);
97+
user.setLibraryCard(libraryCard);
9898
}
9999
if (Objects.nonNull(userProperty)) {
100100
user.addProperty(userProperty);

‎src/main/java/org/broadinstitute/consent/http/db/mapper/UserWithRolesReducer.java

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import java.util.Map;
44
import java.util.Objects;
5-
import java.util.Optional;
65
import org.broadinstitute.consent.http.models.Institution;
76
import org.broadinstitute.consent.http.models.LibraryCard;
87
import org.broadinstitute.consent.http.models.User;
@@ -33,19 +32,12 @@ public void accumulate(Map<Integer, User> map, RowView rowView) {
3332
id -> rowView.getRow(User.class));
3433

3534
try {
36-
// Some queries look for `user_role_id` while those that use a prefix look for `u_user_role_id`
37-
Integer userRoleId = null;
38-
if (hasNonZeroColumn(rowView, "user_role_id")) {
39-
userRoleId = rowView.getColumn("user_role_id", Integer.class);
40-
} else if (hasNonZeroColumn(rowView, "ur_user_role_id")) {
41-
userRoleId = rowView.getColumn("ur_user_role_id", Integer.class);
42-
}
43-
if (Objects.nonNull(userRoleId)) {
44-
UserRole ur = rowView.getRow(UserRole.class);
45-
user.addRole(ur);
35+
UserRole userRole = mapUserRoleFromRowView(rowView, userId);
36+
if (userRole != null) {
37+
user.addRole(userRole);
4638
}
4739
} catch (MappingException e) {
48-
// Ignore any attempt to map a column that doesn't exist
40+
logWarn("Error adding User Role to User", e);
4941
}
5042
try {
5143
if (Objects.nonNull(rowView.getColumn("i_id", Integer.class))) {
@@ -56,34 +48,56 @@ public void accumulate(Map<Integer, User> map, RowView rowView) {
5648
}
5749
}
5850
} catch (MappingException e) {
59-
//Ignore institution mapping errors, possible for new users to not have an institution
51+
logDebug("Error adding Institution to User: %s".formatted(e.getMessage()));
6052
}
6153
//user role join can cause duplication of data if done in tandem with joins on other tables
6254
//ex) The same LC can end up being repeated multiple times
6355
//Below only adds LC if not currently saved on the array
6456
try {
6557
if (rowView.getColumn("lc_id", Integer.class) != null) {
66-
int lcId = rowView.getColumn("lc_id", Integer.class);
67-
LibraryCard lc;
68-
Optional<LibraryCard> existingLibraryCard = user.getLibraryCards().stream()
69-
.filter(card -> card.getId().equals(lcId))
70-
.findFirst();
71-
lc = existingLibraryCard.orElseGet(() -> rowView.getRow(LibraryCard.class));
58+
LibraryCard lc = rowView.getRow(LibraryCard.class);
7259
if (rowView.getColumn("lc_daa_id", Integer.class) != null) {
7360
lc.addDaa(rowView.getColumn("lc_daa_id", Integer.class));
7461
}
75-
user.addLibraryCard(lc);
62+
user.setLibraryCard(lc);
7663
}
7764
} catch (MappingException e) {
78-
//Ignore exceptions here, user may not have a library card issued under this instiution
65+
logDebug("Error adding Library Card to User: %s".formatted(e.getMessage()));
7966
}
8067
try {
8168
if (Objects.nonNull(rowView.getColumn("up_property_id", Integer.class))) {
8269
UserProperty p = rowView.getRow(UserProperty.class);
8370
user.addProperty(p);
8471
}
8572
} catch (MappingException e) {
86-
// Ignore any attempt to map a column that doesn't exist
73+
logDebug("Error adding User Property to User: %s".formatted(e.getMessage()));
74+
}
75+
}
76+
77+
// Some queries look for `user_role_id` while those that use a prefix look for `u_user_role_id`
78+
private UserRole mapUserRoleFromRowView(RowView rowView, Integer userId) {
79+
Integer userRoleId;
80+
Integer roleId;
81+
String name;
82+
Integer dacId = null;
83+
// Some queries look for `user_role_id` while those that use a prefix look for `ur_user_role_id`
84+
if (hasNonZeroColumn(rowView, "user_role_id")) {
85+
userRoleId = rowView.getColumn("user_role_id", Integer.class);
86+
roleId = rowView.getColumn("role_id", Integer.class);
87+
name = rowView.getColumn("name", String.class);
88+
if (hasNonZeroColumn(rowView, "dac_id")) {
89+
dacId = rowView.getColumn("dac_id", Integer.class);
90+
}
91+
return new UserRole(userRoleId, userId, roleId, name, dacId);
92+
} else if (hasNonZeroColumn(rowView, "ur_user_role_id")) {
93+
userRoleId = rowView.getColumn("ur_user_role_id", Integer.class);
94+
roleId = rowView.getColumn("ur_role_id", Integer.class);
95+
name = rowView.getColumn("ur_name", String.class);
96+
if (hasNonZeroColumn(rowView, "ur_dac_id")) {
97+
dacId = rowView.getColumn("ur_dac_id", Integer.class);
98+
}
99+
return new UserRole(userRoleId, userId, roleId, name, dacId);
87100
}
101+
return null;
88102
}
89103
}

‎src/main/java/org/broadinstitute/consent/http/models/LibraryCard.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.util.ArrayList;
44
import java.util.Date;
55
import java.util.List;
6+
import java.util.Objects;
67
import org.apache.commons.lang3.builder.EqualsBuilder;
78

89
public class LibraryCard {
@@ -116,6 +117,12 @@ public boolean equals(Object libraryCard) {
116117
return new EqualsBuilder().append(id, other.getId()).isEquals();
117118
}
118119

120+
@Override
121+
public int hashCode() {
122+
return Objects.hash(id, userId, userName, userEmail, createDate, createUserId, updateDate,
123+
updateUserId, daaIds);
124+
}
125+
119126
public void addDaa(Integer daaId) {
120127
if (this.daaIds == null) {
121128
this.daaIds = new ArrayList<>();

‎src/main/java/org/broadinstitute/consent/http/models/User.java

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public class User {
5757

5858
private Institution institution;
5959

60-
private List<LibraryCard> libraryCards;
60+
private LibraryCard libraryCard;
6161

6262
public User() {
6363
}
@@ -101,7 +101,7 @@ public User(String json) {
101101
// Nor do we need to retrieve the full institution object from user-provided data.
102102
JsonObject filteredUserJsonObject = filterFields(
103103
userJsonObject,
104-
Arrays.asList("createDate", "institution", "libraryCards"));
104+
Arrays.asList("createDate", "institution", "libraryCard"));
105105
User u = gson.fromJson(filteredUserJsonObject.toString(), User.class);
106106
setUserId(u);
107107
setEmail(u);
@@ -278,15 +278,12 @@ public Institution getInstitution() {
278278
return institution;
279279
}
280280

281-
public void setLibraryCards(List<LibraryCard> cards) {
282-
this.libraryCards = cards;
281+
public LibraryCard getLibraryCard() {
282+
return libraryCard;
283283
}
284284

285-
public List<LibraryCard> getLibraryCards() {
286-
if (this.libraryCards == null) {
287-
this.libraryCards = new ArrayList<>();
288-
}
289-
return this.libraryCards;
285+
public void setLibraryCard(LibraryCard libraryCard) {
286+
this.libraryCard = libraryCard;
290287
}
291288

292289
public void addRole(UserRole userRole) {
@@ -308,15 +305,6 @@ public void addProperty(UserProperty userProp) {
308305
}
309306
}
310307

311-
public void addLibraryCard(LibraryCard card) {
312-
if (this.libraryCards == null) {
313-
this.libraryCards = new ArrayList<>();
314-
}
315-
if (!this.libraryCards.contains(card)) {
316-
this.libraryCards.add(card);
317-
}
318-
}
319-
320308
@Override
321309
public int hashCode() {
322310
return this.getUserId();

0 commit comments

Comments
 (0)