diff --git a/NEWS.md b/NEWS.md index 87ba64c0..1a4de7e6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,6 +8,7 @@ * [MODAUD-271](https://folio-org.atlassian.net/browse/MODAUD-271) - Fix cancellation reason deserialization with consortium source field * [MODAUD-257](https://folio-org.atlassian.net/browse/MODAUD-257) - Replace EOL net.mguenther.kafka:kafka-junit with testcontainers KafkaContainer * [MODAUD-297](https://folio-org.atlassian.net/browse/MODAUD-297) - Consume user domain events and store audit history +* [MODAUD-299](https://folio-org.atlassian.net/browse/MODAUD-299) - Delete user audit records on disable ## 2.11.1 2025-04-15 * [MODAUD-250](https://folio-org.atlassian.net/browse/MODAUD-250) - Version history of "MARC" records is not tracked diff --git a/mod-audit-server/src/main/java/org/folio/dao/configuration/SettingDao.java b/mod-audit-server/src/main/java/org/folio/dao/configuration/SettingDao.java index c67f79e7..cbf5a71d 100644 --- a/mod-audit-server/src/main/java/org/folio/dao/configuration/SettingDao.java +++ b/mod-audit-server/src/main/java/org/folio/dao/configuration/SettingDao.java @@ -20,6 +20,7 @@ import io.vertx.sqlclient.Tuple; import java.util.List; import java.util.function.Function; +import org.folio.rest.persist.Conn; import org.folio.util.DbUtils; import org.folio.util.PostgresClientFactory; import org.springframework.stereotype.Repository; @@ -56,11 +57,10 @@ public Future> getAllByGroupId(String groupId, String tenant return promise.future().map(this::mapToSettingList); } - public Future exists(String settingId, String tenantId) { - var promise = Promise.>promise(); + public Future exists(String settingId, Conn conn, String tenantId) { var query = prepareSql(EXIST_BY_ID_SQL, tenantId); - pgClientFactory.createInstance(tenantId).select(query, Tuple.of(settingId), promise); - return promise.future().map(rowSet -> rowSet.iterator().hasNext()); + return conn.execute(query, Tuple.of(settingId)) + .map(rowSet -> rowSet.iterator().hasNext()); } public Future getById(String settingId, String tenantId) { @@ -70,13 +70,11 @@ public Future getById(String settingId, String tenantId) { return promise.future().map(rowSet -> settingMapper().apply(rowSet.iterator().next())); } - public Future update(String settingId, SettingEntity entity, String tenantId) { - var promise = Promise.>promise(); + public Future update(String settingId, SettingEntity entity, Conn conn, String tenantId) { var query = prepareSql(UPDATE_SQL, tenantId).replace(TYPE_CAST_PLACEHOLDER, getTypeCast(entity.getType())); var params = Tuple.of(entity.getKey(), entity.getValue(), entity.getType().value(), entity.getDescription(), entity.getUpdatedByUserId(), entity.getUpdatedDate(), settingId); - pgClientFactory.createInstance(tenantId).execute(query, params, promise); - return promise.future().mapEmpty(); + return conn.execute(query, params).mapEmpty(); } private String prepareSql(String sql, String tenantId) { diff --git a/mod-audit-server/src/main/java/org/folio/dao/user/UserEventDao.java b/mod-audit-server/src/main/java/org/folio/dao/user/UserEventDao.java index 563b8cda..c8688c31 100644 --- a/mod-audit-server/src/main/java/org/folio/dao/user/UserEventDao.java +++ b/mod-audit-server/src/main/java/org/folio/dao/user/UserEventDao.java @@ -6,6 +6,7 @@ import java.sql.Timestamp; import java.util.List; import java.util.UUID; +import org.folio.rest.persist.Conn; public interface UserEventDao { @@ -48,6 +49,8 @@ public interface UserEventDao { */ Future deleteByUserId(UUID userId, String tenantId); + Future deleteAll(Conn conn, String tenantId); + /** * Returns audit table name * diff --git a/mod-audit-server/src/main/java/org/folio/dao/user/impl/UserEventDaoImpl.java b/mod-audit-server/src/main/java/org/folio/dao/user/impl/UserEventDaoImpl.java index e70cfa6a..f01780dc 100644 --- a/mod-audit-server/src/main/java/org/folio/dao/user/impl/UserEventDaoImpl.java +++ b/mod-audit-server/src/main/java/org/folio/dao/user/impl/UserEventDaoImpl.java @@ -26,6 +26,7 @@ import org.folio.dao.user.UserAuditEntity; import org.folio.dao.user.UserEventDao; import org.folio.domain.diff.ChangeRecordDto; +import org.folio.rest.persist.Conn; import org.folio.util.PostgresClientFactory; import org.springframework.stereotype.Repository; @@ -45,6 +46,8 @@ public class UserEventDaoImpl implements UserEventDao { WHERE user_id = $1 """; + private static final String DELETE_ALL_SQL = "DELETE FROM %s"; + private static final String SELECT_SQL = """ SELECT * FROM %s WHERE user_id = $1 %s @@ -104,6 +107,14 @@ public Future deleteByUserId(UUID userId, String tenantId) { .mapEmpty(); } + @Override + public Future deleteAll(Conn conn, String tenantId) { + LOGGER.debug("deleteAll:: Deleting all user audit records with [tenantId: {}]", tenantId); + var table = formatDBTableName(tenantId, tableName()); + var query = DELETE_ALL_SQL.formatted(table); + return conn.execute(query).mapEmpty(); + } + @Override public String tableName() { return USER_AUDIT_TABLE; diff --git a/mod-audit-server/src/main/java/org/folio/services/configuration/ConfigurationService.java b/mod-audit-server/src/main/java/org/folio/services/configuration/ConfigurationService.java index c495c168..d10d7f7d 100644 --- a/mod-audit-server/src/main/java/org/folio/services/configuration/ConfigurationService.java +++ b/mod-audit-server/src/main/java/org/folio/services/configuration/ConfigurationService.java @@ -5,6 +5,7 @@ import io.vertx.core.Future; import java.time.LocalDateTime; import java.time.ZoneOffset; +import java.util.List; import java.util.UUID; import javax.ws.rs.NotFoundException; import lombok.RequiredArgsConstructor; @@ -14,6 +15,8 @@ import org.folio.rest.jaxrs.model.Setting; import org.folio.rest.jaxrs.model.SettingCollection; import org.folio.rest.jaxrs.model.SettingGroupCollection; +import org.folio.rest.persist.Conn; +import org.folio.util.PostgresClientFactory; import org.springframework.stereotype.Service; @Service @@ -24,6 +27,8 @@ public class ConfigurationService { private final SettingGroupDao settingGroupDao; private final SettingMappers settingMappers; private final SettingValidationService validationService; + private final List changeHandlers; + private final PostgresClientFactory pgClientFactory; public Future getAllSettingGroups(String tenantId) { return settingGroupDao.getAll(tenantId) @@ -47,10 +52,12 @@ public Future updateSetting(String groupId, String settingKey, Setting set entity.setUpdatedDate(LocalDateTime.now(ZoneOffset.UTC)); entity.setUpdatedByUserId(userId == null ? null : UUID.fromString(userId)); var settingId = entity.getId(); - return settingDao.exists(settingId, tenantId) - .compose(exists -> failSettingIfNotExist(groupId, settingKey, exists)) - .compose(o -> settingDao.update(settingId, entity, tenantId)) - .mapEmpty(); + return pgClientFactory.createInstance(tenantId).withTrans(conn -> + settingDao.exists(settingId, conn, tenantId) + .compose(exists -> failSettingIfNotExist(groupId, settingKey, exists)) + .compose(ignored -> settingDao.update(settingId, entity, conn, tenantId)) + .compose(ignored -> notifyHandlers(groupId, settingKey, entity.getValue(), conn, tenantId)) + ); } public Future getSetting(org.folio.services.configuration.Setting setting, String tenantId) { @@ -76,4 +83,15 @@ private Future failNotFound(boolean exists, String settingKey) { ? Future.succeededFuture(null) : Future.failedFuture(new NotFoundException(settingKey)); } + + private Future notifyHandlers(String groupId, String settingKey, + Object newValue, Conn conn, String tenantId) { + var result = Future.succeededFuture(); + for (var handler : changeHandlers) { + if (handler.isResponsible(groupId, settingKey)) { + result = result.compose(ignored -> handler.onSettingChanged(newValue, conn, tenantId)); + } + } + return result; + } } diff --git a/mod-audit-server/src/main/java/org/folio/services/configuration/SettingChangeHandler.java b/mod-audit-server/src/main/java/org/folio/services/configuration/SettingChangeHandler.java new file mode 100644 index 00000000..36732324 --- /dev/null +++ b/mod-audit-server/src/main/java/org/folio/services/configuration/SettingChangeHandler.java @@ -0,0 +1,21 @@ +package org.folio.services.configuration; + +import io.vertx.core.Future; +import org.folio.rest.persist.Conn; + +/** + * Handler invoked when a configuration setting is updated. + * + *

Implementations are called within the active write transaction managed by + * {@link ConfigurationService#updateSetting}. The provided {@code conn} may be used for + * transactional DB operations that must be atomic with the setting update itself. + * + *

Implementations must not commit or roll back {@code conn}. + * Non-DB side effects (HTTP calls, Kafka messages, etc.) should not be performed via this callback. + */ +public interface SettingChangeHandler { + + boolean isResponsible(String groupId, String settingKey); + + Future onSettingChanged(Object newValue, Conn conn, String tenantId); +} diff --git a/mod-audit-server/src/main/java/org/folio/services/user/UserAuditPurgeHandler.java b/mod-audit-server/src/main/java/org/folio/services/user/UserAuditPurgeHandler.java new file mode 100644 index 00000000..84019139 --- /dev/null +++ b/mod-audit-server/src/main/java/org/folio/services/user/UserAuditPurgeHandler.java @@ -0,0 +1,35 @@ +package org.folio.services.user; + +import io.vertx.core.Future; +import lombok.RequiredArgsConstructor; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.folio.dao.user.UserEventDao; +import org.folio.rest.persist.Conn; +import org.folio.services.configuration.SettingChangeHandler; +import org.folio.services.configuration.SettingGroup; +import org.folio.services.configuration.SettingKey; +import org.springframework.stereotype.Service; + +@Service +@RequiredArgsConstructor +public class UserAuditPurgeHandler implements SettingChangeHandler { + + private static final Logger LOGGER = LogManager.getLogger(); + + private final UserEventDao userEventDao; + + @Override + public boolean isResponsible(String groupId, String settingKey) { + return SettingGroup.USER.getId().equals(groupId) && SettingKey.ENABLED.getValue().equals(settingKey); + } + + @Override + public Future onSettingChanged(Object newValue, Conn conn, String tenantId) { + if (Boolean.FALSE.equals(newValue)) { + LOGGER.info("onSettingChanged:: User audit disabled, deleting all user audit records [tenantId: {}]", tenantId); + return userEventDao.deleteAll(conn, tenantId); + } + return Future.succeededFuture(); + } +} diff --git a/mod-audit-server/src/test/java/org/folio/TestSuite.java b/mod-audit-server/src/test/java/org/folio/TestSuite.java index 31b53eb1..b85053e0 100644 --- a/mod-audit-server/src/test/java/org/folio/TestSuite.java +++ b/mod-audit-server/src/test/java/org/folio/TestSuite.java @@ -55,6 +55,7 @@ import org.folio.services.OrderLineAuditEventsServiceTest; import org.folio.services.OrganizationAuditEventsServiceTest; import org.folio.services.PieceAuditEventsServiceTest; +import org.folio.services.configuration.ConfigurationServiceTransactionTest; import org.folio.services.marc.impl.MarcAuditServiceTest; import org.folio.util.marc.MarcUtilTest; import org.junit.jupiter.api.AfterAll; @@ -284,6 +285,10 @@ class InventoryAuditApiNestedTest extends InventoryAuditApiTest { class AuditDataCleanupApiNestedTest extends AuditDataCleanupApiTest { } + @Nested + class ConfigurationServiceTransactionNestedTest extends ConfigurationServiceTransactionTest { + } + @Nested class UserAuditApiNestedTest extends UserAuditApiTest { } diff --git a/mod-audit-server/src/test/java/org/folio/dao/configuration/SettingDaoTest.java b/mod-audit-server/src/test/java/org/folio/dao/configuration/SettingDaoTest.java index 4dbde94a..16d38e63 100644 --- a/mod-audit-server/src/test/java/org/folio/dao/configuration/SettingDaoTest.java +++ b/mod-audit-server/src/test/java/org/folio/dao/configuration/SettingDaoTest.java @@ -4,12 +4,16 @@ import static org.folio.utils.EntityUtils.createSettingEntity; import static org.folio.utils.MockUtils.mockPostgresExecutionSuccess; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.vertx.core.Future; import io.vertx.junit5.VertxExtension; import io.vertx.junit5.VertxTestContext; import io.vertx.sqlclient.Row; @@ -18,6 +22,7 @@ import java.time.LocalDateTime; import java.util.UUID; import java.util.stream.Stream; +import org.folio.rest.persist.Conn; import org.folio.rest.persist.PostgresClient; import org.folio.util.PostgresClientFactory; import org.folio.utils.MockUtils; @@ -42,19 +47,11 @@ class SettingDaoTest { @Mock private PostgresClient postgresClient; @InjectMocks - private SettingDao instanceEventDao; - - private static Stream updateTestData() { - return Stream.of( - Arguments.of(1, SettingValueType.INTEGER, "integer"), - Arguments.of("string", SettingValueType.STRING, "text"), - Arguments.of(true, SettingValueType.BOOLEAN, "boolean") - ); - } + private SettingDao settingDao; @BeforeEach public void setUp() { - when(postgresClientFactory.createInstance(TENANT_ID)).thenReturn(postgresClient); + lenient().when(postgresClientFactory.createInstance(TENANT_ID)).thenReturn(postgresClient); } @Test @@ -66,27 +63,30 @@ void getAllByGroupId_positive() { mockPostgresExecutionSuccess(2).when(postgresClient).select(eq(query), captor.capture(), any()); // when - instanceEventDao.getAllByGroupId(groupId, TENANT_ID); + settingDao.getAllByGroupId(groupId, TENANT_ID); // then assertEquals(groupId, captor.getValue().getString(0)); verify(postgresClient).select(eq(query), any(Tuple.class), any()); } + @SuppressWarnings("unchecked") @Test void exists_positive() { // given var settingId = "settingId"; - var query = "SELECT 1 FROM diku_mod_audit.setting WHERE id = $1"; - var captor = ArgumentCaptor.forClass(Tuple.class); - mockPostgresExecutionSuccess(2).when(postgresClient).select(eq(query), captor.capture(), any()); + var conn = mock(Conn.class); + var rowSet = mock(RowSet.class); + when(rowSet.iterator()).thenReturn(MockUtils.mockRowIterator(mock(Row.class))); + when(conn.execute(anyString(), any(Tuple.class))).thenReturn(Future.succeededFuture(rowSet)); // when - instanceEventDao.exists(settingId, TENANT_ID); + var result = settingDao.exists(settingId, conn, TENANT_ID); // then - assertEquals(settingId, captor.getValue().getString(0)); - verify(postgresClient).select(eq(query), any(Tuple.class), any()); + assertTrue(result.succeeded()); + assertTrue(result.result()); + verify(conn).execute(anyString(), any(Tuple.class)); } @Test @@ -101,7 +101,7 @@ void getById_positive(VertxTestContext ctx) { .when(postgresClient).select(eq(query), captor.capture(), any()); // when - instanceEventDao.getById(settingId, TENANT_ID) + settingDao.getById(settingId, TENANT_ID) .onComplete(ctx.succeeding(result -> { assertEquals(settingId, captor.getValue().getString(0)); assertEquals(10, result.getValue()); @@ -109,9 +109,18 @@ void getById_positive(VertxTestContext ctx) { })); } + private static Stream updateTestData() { + return Stream.of( + Arguments.of(1, SettingValueType.INTEGER, "integer"), + Arguments.of("string", SettingValueType.STRING, "text"), + Arguments.of(true, SettingValueType.BOOLEAN, "boolean") + ); + } + + @SuppressWarnings("unchecked") @ParameterizedTest @MethodSource("updateTestData") - void update_positive(Object value, SettingValueType type, String typeValue) { + void update_withConn_bindsCorrectParameters(Object value, SettingValueType type, String typeValue) { // given var entity = new SettingEntity("group.key", "key", value, type, "description", "group", LocalDateTime.now(), UUID.randomUUID(), LocalDateTime.now(), UUID.randomUUID()); @@ -124,13 +133,16 @@ void update_positive(Object value, SettingValueType type, String typeValue) { updated_by = $5, updated_date = $6 WHERE id = $7""".formatted(typeValue); + var conn = mock(Conn.class); var captor = ArgumentCaptor.forClass(Tuple.class); - mockPostgresExecutionSuccess(2).when(postgresClient).execute(eq(query), captor.capture(), any()); + when(conn.execute(eq(query), captor.capture())) + .thenReturn(Future.succeededFuture(mock(RowSet.class))); // when - instanceEventDao.update(entity.getId(), entity, TENANT_ID); + var result = settingDao.update(entity.getId(), entity, conn, TENANT_ID); // then + assertTrue(result.succeeded()); var captorValue = captor.getValue(); assertEquals(entity.getKey(), captorValue.getString(0)); assertEquals(entity.getValue(), captorValue.getValue(1)); diff --git a/mod-audit-server/src/test/java/org/folio/dao/user/impl/UserEventDaoImplTest.java b/mod-audit-server/src/test/java/org/folio/dao/user/impl/UserEventDaoImplTest.java index 4cfc2192..cb46016b 100644 --- a/mod-audit-server/src/test/java/org/folio/dao/user/impl/UserEventDaoImplTest.java +++ b/mod-audit-server/src/test/java/org/folio/dao/user/impl/UserEventDaoImplTest.java @@ -10,13 +10,17 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import io.vertx.core.Future; import io.vertx.junit5.VertxExtension; import io.vertx.junit5.VertxTestContext; +import io.vertx.sqlclient.RowSet; import io.vertx.sqlclient.Tuple; +import org.folio.rest.persist.Conn; import org.folio.rest.persist.PostgresClient; import org.folio.util.PostgresClientFactory; import org.folio.utils.UnitTest; @@ -89,6 +93,19 @@ void shouldDeleteByUserId(VertxTestContext ctx) { })); } + @SuppressWarnings("unchecked") + @Test + void shouldDeleteAll(VertxTestContext ctx) { + var conn = mock(Conn.class); + when(conn.execute(anyString())).thenReturn(Future.succeededFuture(mock(RowSet.class))); + + userEventDao.deleteAll(conn, TENANT_ID) + .onComplete(ctx.succeeding(result -> { + verify(conn, times(1)).execute(anyString()); + ctx.completeNow(); + })); + } + @Test void shouldReturnCorrectTableName() { assertEquals("user_audit", userEventDao.tableName()); diff --git a/mod-audit-server/src/test/java/org/folio/rest/impl/AuditDataCleanupApiTest.java b/mod-audit-server/src/test/java/org/folio/rest/impl/AuditDataCleanupApiTest.java index 0e77730f..6d618f34 100644 --- a/mod-audit-server/src/test/java/org/folio/rest/impl/AuditDataCleanupApiTest.java +++ b/mod-audit-server/src/test/java/org/folio/rest/impl/AuditDataCleanupApiTest.java @@ -134,7 +134,9 @@ private void setRetentionToOneDay(Setting setting, SettingGroup settingGroup) { .type(SettingValueType.INTEGER) .groupId(settingGroup.getId()) .build(); - settingDao.update(settingEntity.getId(), settingEntity, TENANT_ID).toCompletionStage().toCompletableFuture().get(); + postgresClientFactory.createInstance(TENANT_ID) + .withTrans(conn -> settingDao.update(settingEntity.getId(), settingEntity, conn, TENANT_ID)) + .toCompletionStage().toCompletableFuture().get(); } @SneakyThrows diff --git a/mod-audit-server/src/test/java/org/folio/rest/impl/InventoryAuditApiTest.java b/mod-audit-server/src/test/java/org/folio/rest/impl/InventoryAuditApiTest.java index 1c2fa039..c228f2eb 100644 --- a/mod-audit-server/src/test/java/org/folio/rest/impl/InventoryAuditApiTest.java +++ b/mod-audit-server/src/test/java/org/folio/rest/impl/InventoryAuditApiTest.java @@ -120,7 +120,7 @@ void shouldReturnPaginatedInventoryEvents(InventoryResourceType resourceType, St .build(); var entityId = UUID.randomUUID().toString(); - settingDao.update(settingEntity.getId(), settingEntity, TENANT_ID).toCompletionStage().toCompletableFuture().get(); + updateSetting(settingEntity); // Create InventoryAuditEntity entities with a day date difference for (int i = 0; i < 5; i++) { @@ -182,6 +182,13 @@ void shouldReturn400ForInvalidEntityId(String apiPath) { .body("errors[0].message", containsString("Invalid UUID string")); } + @SneakyThrows + private void updateSetting(SettingEntity settingEntity) { + postgresClientFactory.createInstance(TENANT_ID) + .withTrans(conn -> settingDao.update(settingEntity.getId(), settingEntity, conn, TENANT_ID)) + .toCompletionStage().toCompletableFuture().get(); + } + private static Stream provideResourceTypeAndPath() { return Stream.of( Arguments.of(InventoryResourceType.INSTANCE, INVENTORY_INSTANCE_AUDIT_PATH), diff --git a/mod-audit-server/src/test/java/org/folio/rest/impl/UserAuditApiTest.java b/mod-audit-server/src/test/java/org/folio/rest/impl/UserAuditApiTest.java index 9ef15d01..1e78cea5 100644 --- a/mod-audit-server/src/test/java/org/folio/rest/impl/UserAuditApiTest.java +++ b/mod-audit-server/src/test/java/org/folio/rest/impl/UserAuditApiTest.java @@ -89,7 +89,9 @@ void shouldReturnPaginatedUserEvents() { .build(); var userId = UUID.randomUUID().toString(); - settingDao.update(settingEntity.getId(), settingEntity, TENANT_ID).toCompletionStage().toCompletableFuture().get(); + postgresClientFactory.createInstance(TENANT_ID) + .withTrans(conn -> settingDao.update(settingEntity.getId(), settingEntity, conn, TENANT_ID)) + .toCompletionStage().toCompletableFuture().get(); for (int i = 0; i < 5; i++) { var changeRecordDto = new ChangeRecordDto(); diff --git a/mod-audit-server/src/test/java/org/folio/services/configuration/ConfigurationServiceTest.java b/mod-audit-server/src/test/java/org/folio/services/configuration/ConfigurationServiceTest.java index ea119450..41530c75 100644 --- a/mod-audit-server/src/test/java/org/folio/services/configuration/ConfigurationServiceTest.java +++ b/mod-audit-server/src/test/java/org/folio/services/configuration/ConfigurationServiceTest.java @@ -8,13 +8,17 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import io.vertx.core.Future; import java.util.Collections; +import java.util.List; import java.util.UUID; +import java.util.function.Function; import javax.ws.rs.NotFoundException; import org.folio.dao.configuration.SettingDao; import org.folio.dao.configuration.SettingEntity; @@ -25,11 +29,14 @@ import org.folio.mapper.configuration.SettingMapper; import org.folio.mapper.configuration.SettingMappers; import org.folio.rest.jaxrs.model.Setting; +import org.folio.rest.persist.Conn; +import org.folio.rest.persist.PostgresClient; +import org.folio.util.PostgresClientFactory; import org.folio.utils.UnitTest; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -50,10 +57,27 @@ class ConfigurationServiceTest { private SettingMappers settingMappers; @Mock private SettingValidationService validationService; + @Mock + private SettingChangeHandler changeHandler; + @Mock + private PostgresClientFactory pgClientFactory; + @Mock + private PostgresClient postgresClient; + @Mock + private Conn conn; - @InjectMocks private ConfigurationService configurationService; + @BeforeEach + void setUp() { + configurationService = new ConfigurationService(settingDao, settingGroupDao, settingMappers, + validationService, List.of(changeHandler), pgClientFactory); + lenient().when(pgClientFactory.createInstance(TENANT_ID)).thenReturn(postgresClient); + lenient().doAnswer(invocation -> invocation.>>getArgument(0).apply(conn)) + .when(postgresClient).withTrans(any()); + lenient().when(changeHandler.isResponsible(GROUP_ID, SETTING_KEY)).thenReturn(true); + } + @Test void getAllSettingGroups_shouldReturnSettingGroupCollection() { when(settingGroupDao.getAll(anyString())).thenReturn(Future.succeededFuture(Collections.emptyList())); @@ -80,13 +104,16 @@ void getAllSettingsByGroupId_shouldReturnSettingCollection() { } @Test - void updateSetting_shouldUpdateSetting() { + void updateSetting_shouldUpdateSettingAndNotifyHandlers() { var setting = getSetting(); var argumentCaptor = ArgumentCaptor.forClass(SettingEntity.class); var userId = UUID.randomUUID(); - when(settingDao.exists(anyString(), anyString())).thenReturn(Future.succeededFuture(true)); - when(settingDao.update(anyString(), argumentCaptor.capture(), anyString())).thenReturn(Future.succeededFuture()); + when(settingDao.exists(anyString(), any(Conn.class), anyString())).thenReturn(Future.succeededFuture(true)); + when(settingDao.update(anyString(), argumentCaptor.capture(), any(Conn.class), anyString())) + .thenReturn(Future.succeededFuture()); when(settingMappers.getSettingEntityMapper()).thenReturn(new SettingEntityMapper()); + when(changeHandler.onSettingChanged("value", conn, TENANT_ID)) + .thenReturn(Future.succeededFuture()); var result = configurationService.updateSetting(GROUP_ID, SETTING_KEY, setting, userId.toString(), TENANT_ID); @@ -102,14 +129,46 @@ void updateSetting_shouldUpdateSetting() { assertNotNull(settingEntity.getUpdatedDate()); verify(validationService).validateSetting(setting, GROUP_ID, SETTING_KEY); - verify(settingDao).exists(SETTING_ID, TENANT_ID); - verify(settingDao).update(eq(SETTING_ID), any(), eq(TENANT_ID)); + verify(settingDao).exists(SETTING_ID, conn, TENANT_ID); + verify(settingDao).update(eq(SETTING_ID), any(), eq(conn), eq(TENANT_ID)); + verify(changeHandler).onSettingChanged("value", conn, TENANT_ID); + } + + @Test + void updateSetting_shouldNotNotifyHandlers_whenNoHandlerIsResponsible() { + var setting = getSetting(); + when(settingDao.exists(anyString(), any(Conn.class), anyString())).thenReturn(Future.succeededFuture(true)); + when(settingDao.update(anyString(), any(), any(Conn.class), anyString())).thenReturn(Future.succeededFuture()); + when(settingMappers.getSettingEntityMapper()).thenReturn(new SettingEntityMapper()); + when(changeHandler.isResponsible(GROUP_ID, SETTING_KEY)).thenReturn(false); + + var result = configurationService.updateSetting(GROUP_ID, SETTING_KEY, setting, null, TENANT_ID); + + assertTrue(result.succeeded()); + verify(changeHandler, never()).onSettingChanged(any(), any(Conn.class), any()); + } + + @Test + void updateSetting_shouldFailWhenHandlerFails() { + var setting = getSetting(); + when(settingDao.exists(anyString(), any(Conn.class), anyString())).thenReturn(Future.succeededFuture(true)); + when(settingDao.update(anyString(), any(), any(Conn.class), anyString())).thenReturn(Future.succeededFuture()); + when(settingMappers.getSettingEntityMapper()).thenReturn(new SettingEntityMapper()); + when(changeHandler.onSettingChanged("value", conn, TENANT_ID)) + .thenReturn(Future.failedFuture(new RuntimeException("handler error"))); + + var result = configurationService.updateSetting(GROUP_ID, SETTING_KEY, setting, null, TENANT_ID); + + assertTrue(result.failed()); + assertEquals("handler error", result.cause().getMessage()); + verify(settingDao).update(eq(SETTING_ID), any(), eq(conn), eq(TENANT_ID)); + verify(changeHandler).onSettingChanged("value", conn, TENANT_ID); } @Test void updateSetting_shouldThrowNotFoundException_whenSettingDoesNotExist() { var setting = getSetting(); - when(settingDao.exists(anyString(), anyString())).thenReturn(Future.succeededFuture(false)); + when(settingDao.exists(anyString(), any(Conn.class), anyString())).thenReturn(Future.succeededFuture(false)); when(settingMappers.getSettingEntityMapper()).thenReturn(new SettingEntityMapper()); var result = configurationService.updateSetting(GROUP_ID, SETTING_KEY, setting, null, TENANT_ID); @@ -117,7 +176,7 @@ void updateSetting_shouldThrowNotFoundException_whenSettingDoesNotExist() { assertTrue(result.failed()); assertEquals(NotFoundException.class, result.cause().getClass()); verify(validationService).validateSetting(setting, GROUP_ID, SETTING_KEY); - verify(settingDao).exists(SETTING_ID, TENANT_ID); + verify(settingDao).exists(SETTING_ID, conn, TENANT_ID); } @Test @@ -144,4 +203,4 @@ private Setting getSetting() { setting.setValue("value"); return setting; } -} \ No newline at end of file +} diff --git a/mod-audit-server/src/test/java/org/folio/services/configuration/ConfigurationServiceTransactionTest.java b/mod-audit-server/src/test/java/org/folio/services/configuration/ConfigurationServiceTransactionTest.java new file mode 100644 index 00000000..7cfc6813 --- /dev/null +++ b/mod-audit-server/src/test/java/org/folio/services/configuration/ConfigurationServiceTransactionTest.java @@ -0,0 +1,174 @@ +package org.folio.services.configuration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.folio.util.DbUtils.formatDBTableName; + +import io.vertx.core.Future; +import io.vertx.core.Vertx; +import java.sql.Timestamp; +import java.time.Instant; +import java.util.List; +import java.util.UUID; +import lombok.SneakyThrows; +import org.folio.dao.configuration.SettingDao; +import org.folio.dao.configuration.SettingEntity; +import org.folio.dao.configuration.SettingGroupDao; +import org.folio.dao.configuration.SettingValueType; +import org.folio.dao.user.UserAuditEntity; +import org.folio.dao.user.impl.UserEventDaoImpl; +import org.folio.mapper.configuration.SettingEntityMapper; +import org.folio.rest.persist.Conn; +import org.folio.mapper.configuration.SettingGroupMapper; +import org.folio.mapper.configuration.SettingMapper; +import org.folio.mapper.configuration.SettingMappers; +import org.folio.rest.impl.ApiTestBase; +import org.folio.services.user.UserAuditPurgeHandler; +import org.folio.util.PostgresClientFactory; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Spy; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +public class ConfigurationServiceTransactionTest extends ApiTestBase { + + private static final String TENANT_ID = "modaudittest"; + private static final String GROUP_ID = SettingGroup.USER.getId(); + private static final String SETTING_KEY = SettingKey.ENABLED.getValue(); + private static final String SETTING_ID = GROUP_ID + "." + SETTING_KEY; + + @InjectMocks + SettingDao settingDao; + @InjectMocks + SettingGroupDao settingGroupDao; + @InjectMocks + UserEventDaoImpl userEventDao; + @Spy + private PostgresClientFactory postgresClientFactory = new PostgresClientFactory(Vertx.vertx()); + + private SettingMappers settingMappers; + private SettingValidationService validationService; + + @BeforeEach + void setUp() { + settingMappers = new SettingMappers(new SettingGroupMapper(), new SettingMapper(), new SettingEntityMapper()); + validationService = new SettingValidationService(); + } + + @SneakyThrows + @Test + void shouldDeleteAllRecordsAndUpdateSetting_whenAuditDisabled() { + // given: enabled = true and some audit records exist + setEnabled(true); + insertUserAuditRecord(); + assertThat(countUserAuditRecords()).isGreaterThan(0); + + // when: disable audit via ConfigurationService with real cleanup handler + var cleanupHandler = new UserAuditPurgeHandler(userEventDao); + var service = new ConfigurationService(settingDao, settingGroupDao, settingMappers, + validationService, List.of(cleanupHandler), postgresClientFactory); + + service.updateSetting(GROUP_ID, SETTING_KEY, buildSettingPayload(false), null, TENANT_ID) + .toCompletionStage().toCompletableFuture().get(); + + // then: records deleted and setting changed to false + assertThat(countUserAuditRecords()).isZero(); + assertThat(getEnabledValue()).isEqualTo(false); + } + + @SneakyThrows + @Test + void shouldRollbackSettingUpdate_whenHandlerFails() { + // given: enabled = true and some audit records exist + setEnabled(true); + deleteAllUserAuditRecords(); + insertUserAuditRecord(); + var recordsBefore = countUserAuditRecords(); + assertThat(recordsBefore).isGreaterThan(0); + + // when: disable audit with a handler that fails + SettingChangeHandler failingHandler = new SettingChangeHandler() { + @Override + public boolean isResponsible(String groupId, String settingKey) { + return true; + } + + @Override + public Future onSettingChanged(Object newValue, Conn conn, String tenantId) { + return Future.failedFuture(new RuntimeException("simulated handler failure")); + } + }; + + var service = new ConfigurationService(settingDao, settingGroupDao, settingMappers, + validationService, List.of(failingHandler), postgresClientFactory); + + var result = service.updateSetting(GROUP_ID, SETTING_KEY, buildSettingPayload(false), null, TENANT_ID) + .toCompletionStage().toCompletableFuture(); + + assertThatThrownBy(result::get) + .hasMessageContaining("simulated handler failure"); + + // then: setting is still true (rolled back) and records still exist + assertThat(getEnabledValue()).isEqualTo(true); + assertThat(countUserAuditRecords()).isEqualTo(recordsBefore); + } + + @SneakyThrows + private void setEnabled(boolean value) { + var entity = SettingEntity.builder() + .id(SETTING_ID) + .key(SETTING_KEY) + .value(value) + .type(SettingValueType.BOOLEAN) + .groupId(GROUP_ID) + .build(); + postgresClientFactory.createInstance(TENANT_ID) + .withTrans(conn -> settingDao.update(entity.getId(), entity, conn, TENANT_ID)) + .toCompletionStage().toCompletableFuture().get(); + } + + @SneakyThrows + private Object getEnabledValue() { + return settingDao.getById(SETTING_ID, TENANT_ID) + .toCompletionStage().toCompletableFuture().get() + .getValue(); + } + + @SneakyThrows + private void insertUserAuditRecord() { + var entity = new UserAuditEntity( + UUID.randomUUID(), Timestamp.from(Instant.now()), UUID.randomUUID(), + "CREATE", UUID.randomUUID(), null); + userEventDao.save(entity, TENANT_ID) + .toCompletionStage().toCompletableFuture().get(); + } + + @SneakyThrows + private long countUserAuditRecords() { + var table = formatDBTableName(TENANT_ID, "user_audit"); + return postgresClientFactory.createInstance(TENANT_ID) + .execute("SELECT COUNT(*) FROM " + table) + .map(rs -> rs.iterator().next().getLong(0)) + .toCompletionStage().toCompletableFuture().get(); + } + + @SneakyThrows + private void deleteAllUserAuditRecords() { + var table = formatDBTableName(TENANT_ID, "user_audit"); + postgresClientFactory.createInstance(TENANT_ID) + .execute("DELETE FROM " + table) + .toCompletionStage().toCompletableFuture().get(); + } + + private org.folio.rest.jaxrs.model.Setting buildSettingPayload(boolean value) { + var setting = new org.folio.rest.jaxrs.model.Setting(); + setting.setKey(SETTING_KEY); + setting.setGroupId(GROUP_ID); + setting.setType(org.folio.rest.jaxrs.model.Setting.Type.BOOLEAN); + setting.setValue(value); + return setting; + } +} diff --git a/mod-audit-server/src/test/java/org/folio/services/user/UserAuditPurgeHandlerTest.java b/mod-audit-server/src/test/java/org/folio/services/user/UserAuditPurgeHandlerTest.java new file mode 100644 index 00000000..b7bfa8d1 --- /dev/null +++ b/mod-audit-server/src/test/java/org/folio/services/user/UserAuditPurgeHandlerTest.java @@ -0,0 +1,72 @@ +package org.folio.services.user; + +import static org.folio.utils.EntityUtils.TENANT_ID; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import io.vertx.core.Future; +import java.util.stream.Stream; +import org.folio.dao.user.UserEventDao; +import org.folio.rest.persist.Conn; +import org.folio.services.configuration.SettingGroup; +import org.folio.services.configuration.SettingKey; +import org.folio.utils.UnitTest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@UnitTest +@ExtendWith(MockitoExtension.class) +class UserAuditPurgeHandlerTest { + + private static final String USER_GROUP_ID = SettingGroup.USER.getId(); + private static final String ENABLED_KEY = SettingKey.ENABLED.getValue(); + private static final String PAGE_SIZE_KEY = SettingKey.RECORDS_PAGE_SIZE.getValue(); + + @Mock + private UserEventDao userEventDao; + @Mock + private Conn conn; + @InjectMocks + private UserAuditPurgeHandler purgeHandler; + + @Test + void onSettingChanged_shouldDeleteAll_whenSetToFalse() { + when(userEventDao.deleteAll(conn, TENANT_ID)).thenReturn(Future.succeededFuture()); + + var result = purgeHandler.onSettingChanged(false, conn, TENANT_ID); + + assertTrue(result.succeeded()); + verify(userEventDao).deleteAll(conn, TENANT_ID); + } + + @Test + void onSettingChanged_shouldNotDelete_whenSetToTrue() { + var result = purgeHandler.onSettingChanged(true, conn, TENANT_ID); + + assertTrue(result.succeeded()); + verify(userEventDao, never()).deleteAll(conn, TENANT_ID); + } + + @ParameterizedTest + @MethodSource("isResponsibleCases") + void isResponsible_shouldMatchOnlyUserEnabledSetting(String groupId, String settingKey, boolean expected) { + assertEquals(expected, purgeHandler.isResponsible(groupId, settingKey)); + } + + static Stream isResponsibleCases() { + return Stream.of( + Arguments.of(USER_GROUP_ID, ENABLED_KEY, true), + Arguments.of(USER_GROUP_ID, PAGE_SIZE_KEY, false), + Arguments.of(SettingGroup.INVENTORY.getId(), ENABLED_KEY, false) + ); + } +}