Skip to content

Commit

Permalink
PP-11205 Add DAO method to delete users (#2192)
Browse files Browse the repository at this point in the history 
* PP-11205 Add DAO method to delete users

- Added DAO method to delete historic users not associated with any service
  • Loading branch information
@kbottla
kbottla authored Oct 26, 2023
1 parent a45a342 commit ddc962b
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 11 deletions.
4 changes: 2 additions & 2 deletions .secrets.baseline
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@
"filename": "src/test/java/uk/gov/pay/adminusers/fixtures/UserDbFixture.java",
"hashed_secret": "95578813f8acdd659caf14acd19b09c875addbb6",
"is_verified": false,
"line_number": 26
"line_number": 27
}
],
"src/test/java/uk/gov/pay/adminusers/pact/ContractTest.java": [
Expand Down Expand Up @@ -466,5 +466,5 @@
}
]
},
"generated_at": "2023-09-13T11:11:05Z"
"generated_at": "2023-10-26T09:06:36Z"
}
21 changes: 19 additions & 2 deletions src/main/java/uk/gov/pay/adminusers/model/User.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public class User {
@Schema(example = "2022-04-06T23:03:41.665Z")
@JsonSerialize(using = ApiResponseDateTimeSerializer.class)
private ZonedDateTime lastLoggedInAt;
@JsonIgnore
private ZonedDateTime createdAt;
private List<Link> links = new ArrayList<>();
private Integer sessionVersion = 0;

Expand All @@ -58,7 +60,15 @@ public static User from(Integer id, String externalId, String password, String e
SecondFactorMethod secondFactor, String provisionalOtpKey,
ZonedDateTime provisionalOtpKeyCreatedAt, ZonedDateTime lastLoggedInAt) {
return new User(id, externalId, password, email, otpKey, telephoneNumber, serviceRoles, features,
secondFactor, provisionalOtpKey, provisionalOtpKeyCreatedAt, lastLoggedInAt);
secondFactor, provisionalOtpKey, provisionalOtpKeyCreatedAt, lastLoggedInAt, null);
}

public static User from(Integer id, String externalId, String password, String email, String otpKey,
String telephoneNumber, List<ServiceRole> serviceRoles, String features,
SecondFactorMethod secondFactor, String provisionalOtpKey,
ZonedDateTime provisionalOtpKeyCreatedAt, ZonedDateTime lastLoggedInAt, ZonedDateTime createdAt) {
return new User(id, externalId, password, email, otpKey, telephoneNumber, serviceRoles, features,
secondFactor, provisionalOtpKey, provisionalOtpKeyCreatedAt, lastLoggedInAt, createdAt);
}

private User(Integer id, @JsonProperty("external_id") String externalId, @JsonProperty("password") String password, @JsonProperty("email") String email,
Expand All @@ -67,7 +77,9 @@ private User(Integer id, @JsonProperty("external_id") String externalId, @JsonPr
@JsonProperty("second_factor") SecondFactorMethod secondFactor,
@JsonProperty("provisional_otp_key") String provisionalOtpKey,
@JsonProperty("provisional_otp_key_created_at") ZonedDateTime provisionalOtpKeyCreatedAt,
@JsonProperty("last_logged_in_at") ZonedDateTime lastLoggedInAt) {
@JsonProperty("last_logged_in_at") ZonedDateTime lastLoggedInAt,
ZonedDateTime createdAt
) {
this.id = id;
this.externalId = externalId;
this.password = password;
Expand All @@ -80,6 +92,7 @@ private User(Integer id, @JsonProperty("external_id") String externalId, @JsonPr
this.provisionalOtpKey = provisionalOtpKey;
this.provisionalOtpKeyCreatedAt = provisionalOtpKeyCreatedAt;
this.lastLoggedInAt = lastLoggedInAt;
this.createdAt = createdAt;
}

@JsonIgnore
Expand Down Expand Up @@ -173,6 +186,10 @@ public void setLastLoggedInAt(ZonedDateTime lastLoggedInAt) {
this.lastLoggedInAt = lastLoggedInAt;
}

public ZonedDateTime getCreatedAt() {
return createdAt;
}

@JsonProperty("_links")
public List<Link> getLinks() {
return links;
Expand Down
23 changes: 21 additions & 2 deletions src/main/java/uk/gov/pay/adminusers/persistence/dao/UserDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
import javax.inject.Inject;
import javax.persistence.EntityManager;
import javax.persistence.Query;
import java.time.Instant;
import java.util.AbstractMap.SimpleEntry;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static java.util.Date.from;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toUnmodifiableList;
import static java.util.stream.Collectors.toUnmodifiableMap;
Expand Down Expand Up @@ -47,7 +49,7 @@ public List<UserEntity> findByExternalIds(List<String> externalIds) {
.setParameter("externalIds", lowerCaseExternalIds)
.getResultList();
}

public Map<String, List<String>> getAdminUserEmailsForGatewayAccountIds(List<String> gatewayAccountIds) {
if (gatewayAccountIds.size() > 0) {
String positionalParams = IntStream.rangeClosed(1, gatewayAccountIds.size()).mapToObj(Integer::toString)
Expand Down Expand Up @@ -80,7 +82,7 @@ public Map<String, List<String>> getAdminUserEmailsForGatewayAccountIds(List<Str
return Map.of();
}
}

public Optional<UserEntity> findByEmail(String email) {
String query = "SELECT u FROM UserEntity u " +
"WHERE LOWER(u.email) = LOWER(:email)";
Expand All @@ -103,4 +105,21 @@ public List<UserEntity> findByServiceId(Integer serviceId) {
.map(ServiceRoleEntity::getUser)
.collect(toUnmodifiableList());
}

public int deleteUsersNotAssociatedWithAnyService(Instant deleteRecordsBeforeDate) {
String query = "DELETE FROM users u" +
" WHERE u.id in (" +
" SELECT u.id FROM users u" +
" LEFT OUTER JOIN user_services_roles usr " +
" ON u.id = usr.user_id" +
" WHERE usr.user_id is null" +
" AND (last_logged_in_at < ?1 OR (\"createdAt\" < ?2 AND last_logged_in_at IS null))" +
" )";

return entityManager.get()
.createNativeQuery(query)
.setParameter(1, from(deleteRecordsBeforeDate))
.setParameter(2, from(deleteRecordsBeforeDate))
.executeUpdate();
}
}
15 changes: 14 additions & 1 deletion src/test/java/uk/gov/pay/adminusers/fixtures/UserDbFixture.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import uk.gov.pay.adminusers.model.User;
import uk.gov.pay.adminusers.utils.DatabaseTestHelper;

import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -29,6 +30,8 @@ public class UserDbFixture {
private String features = "FEATURE_1, FEATURE_2";
private String provisionalOtpKey;
private SecondFactorMethod secondFactorMethod = SecondFactorMethod.SMS;
private ZonedDateTime createdAt;
private ZonedDateTime lastLoggedInAt;

private UserDbFixture(DatabaseTestHelper databaseTestHelper) {
this.databaseTestHelper = databaseTestHelper;
Expand All @@ -44,7 +47,7 @@ public User insertUser() {
.collect(toUnmodifiableList());

User user = User.from(randomInt(), externalId, password, email, otpKey, telephoneNumber,
serviceRoles, features, secondFactorMethod, provisionalOtpKey, null, null);
serviceRoles, features, secondFactorMethod, provisionalOtpKey, null, lastLoggedInAt, createdAt);

databaseTestHelper.add(user);
serviceRoles.forEach(serviceRole ->
Expand Down Expand Up @@ -103,4 +106,14 @@ public UserDbFixture withSecondFactorMethod(SecondFactorMethod secondFactorMetho
this.secondFactorMethod = secondFactorMethod;
return this;
}

public UserDbFixture withCreatedAt(ZonedDateTime createdAt) {
this.createdAt = createdAt;
return this;
}

public UserDbFixture withLastLoggedInAt(ZonedDateTime lastLoggedInAt) {
this.lastLoggedInAt = lastLoggedInAt;
return this;
}
}
81 changes: 80 additions & 1 deletion src/test/java/uk/gov/pay/adminusers/persistence/dao/UserDaoIT.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package uk.gov.pay.adminusers.persistence.dao;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import uk.gov.pay.adminusers.model.Role;
import uk.gov.pay.adminusers.model.SecondFactorMethod;
Expand All @@ -18,11 +19,14 @@
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import static java.lang.String.valueOf;
import static java.time.ZonedDateTime.parse;
import static java.util.stream.Collectors.toUnmodifiableList;
import static org.apache.commons.lang3.RandomUtils.nextInt;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyOrNullString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -50,7 +54,7 @@ public void before() {
serviceDao = env.getInstance(ServiceDao.class);
roleDao = env.getInstance(RoleDao.class);
}

@Test
void getAdminUserEmailsForGatewayAccountIds_should_return_empty_map() {
Map<String, List<String>> map = userDao.getAdminUserEmailsForGatewayAccountIds(List.of());
Expand Down Expand Up @@ -377,4 +381,79 @@ public void shouldNotCreateAUserWithDifferentCaseEmail() {
var thrown = assertThrows(javax.persistence.RollbackException.class, () -> userDao.persist(userEntity));
assertThat(thrown.getMessage(), containsString("ERROR: duplicate key value violates unique constraint \"lower_case_email_index\""));
}

@Nested
class TestDeleteUsersNotAssociatedWithAnyService {

@Test
void shouldDeleteUsersWithLastLoggedInAtDateCorrectly() {
ZonedDateTime deleteUsersUpToDate = parse("2023-01-31T00:00:00Z");
Integer userId = userDbFixture(databaseHelper)
.withLastLoggedInAt(deleteUsersUpToDate.minusDays(1))
.insertUser()
.getId();
Integer userIdThatShouldNotDeleted = userDbFixture(databaseHelper)
.withLastLoggedInAt(deleteUsersUpToDate.plusDays(1))
.insertUser()
.getId();

int recordsDeleted = userDao.deleteUsersNotAssociatedWithAnyService(deleteUsersUpToDate.toInstant());

assertThat(recordsDeleted, is(1));

Optional<UserEntity> userEntity = userDao.findById(userId);
assertThat(userEntity.isPresent(), is(false));

userEntity = userDao.findById(userIdThatShouldNotDeleted);
assertThat(userEntity.isPresent(), is(true));
}

@Test
void shouldDeleteUsersWithoutLastLoggedInAtDateCorrectlyButCreatedDateBeforeTheExpungeDate() {
ZonedDateTime deleteUsersUpToDate = parse("2023-01-31T00:00:00Z");
Integer userId = userDbFixture(databaseHelper)
.withLastLoggedInAt(null)
.withCreatedAt(deleteUsersUpToDate.minusDays(1))
.insertUser()
.getId();
Integer userIdThatShouldNotDeleted = userDbFixture(databaseHelper)
.withLastLoggedInAt(null)
.withCreatedAt(deleteUsersUpToDate.plusDays(1))
.insertUser()
.getId();

int recordsDeleted = userDao.deleteUsersNotAssociatedWithAnyService(deleteUsersUpToDate.toInstant());

assertThat(recordsDeleted, is(1));

Optional<UserEntity> userEntity = userDao.findById(userId);
assertThat(userEntity.isPresent(), is(false));

userEntity = userDao.findById(userIdThatShouldNotDeleted);
assertThat(userEntity.isPresent(), is(true));
}

@Test
void shouldNotDeleteUsersWithLoggedInOrCreatedIfDateIsAfterTheExpungingDate() {
ZonedDateTime deleteUsersUpToDate = parse("2023-01-31T00:00:00Z");
String externalId1 = userDbFixture(databaseHelper)
.withLastLoggedInAt(deleteUsersUpToDate.plusDays(1))
.insertUser()
.getExternalId();
String externalId2 = userDbFixture(databaseHelper)
.withLastLoggedInAt(null)
.withCreatedAt(deleteUsersUpToDate.plusDays(1))
.insertUser()
.getExternalId();

int recordsDeleted = userDao.deleteUsersNotAssociatedWithAnyService(deleteUsersUpToDate.toInstant());

assertThat(recordsDeleted, is(0));

List<UserEntity> users = userDao.findByExternalIds(List.of(externalId1, externalId2));

assertThat(users.size(), is(2));
assertThat(users.stream().map(UserEntity::getExternalId).collect(Collectors.toSet()), containsInAnyOrder(externalId1, externalId2));
}
}
}
7 changes: 4 additions & 3 deletions src/test/java/uk/gov/pay/adminusers/utils/DatabaseTestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ public DatabaseTestHelper add(User user) {
.createUpdate("INSERT INTO users(" +
"id, external_id, password, email, otp_key, telephone_number, " +
"second_factor, disabled, login_counter, version, " +
"\"createdAt\", \"updatedAt\", session_version, provisional_otp_key) " +
"\"createdAt\", \"updatedAt\", session_version, provisional_otp_key, last_logged_in_at) " +
"VALUES (:id, :externalId, :password, :email, :otpKey, :telephoneNumber, " +
":secondFactor, :disabled, :loginCounter, :version, :createdAt, :updatedAt, :session_version, :provisionalOtpKey)")
":secondFactor, :disabled, :loginCounter, :version, :createdAt, :updatedAt, :session_version, :provisionalOtpKey, :lastLoggedInAt)")
.bind("id", user.getId())
.bind("externalId", user.getExternalId())
.bind("password", user.getPassword())
Expand All @@ -131,9 +131,10 @@ public DatabaseTestHelper add(User user) {
.bind("loginCounter", user.getLoginCounter())
.bind("version", 0)
.bind("session_version", user.getSessionVersion())
.bind("createdAt", now)
.bind("createdAt", user.getCreatedAt() == null ? now : user.getCreatedAt())
.bind("updatedAt", now)
.bind("provisionalOtpKey", user.getProvisionalOtpKey())
.bind("lastLoggedInAt", user.getLastLoggedInAt())
.execute()
);
return this;
Expand Down

0 comments on commit ddc962b

Please sign in to comment.