From e2845483dde09e1070ef85662e10ceb714958565 Mon Sep 17 00:00:00 2001 From: Thilo Schwarz Date: Sat, 13 Jul 2024 20:22:39 +0200 Subject: [PATCH 1/3] in preparation for issue #24 add cascade delete and simplify DatabaseRestoreHandler --- .../dyndrest/repository/UpdateLogRepo.java | 10 +++ .../service/DatabaseRestoreHandler.java | 79 +++++++++++------ .../dyndrest/service/HostZoneService.java | 9 ++ src/main/resources/h2/schema.sql | 4 +- .../dyndrest/repository/ConstraintTest.java | 86 +++++++++++++++++++ .../repository/UpdateLogRepoTest.java | 10 ++- .../dyndrest/service/HostZoneServiceTest.java | 11 ++- src/test/resources/h2/dump.sql | 4 +- 8 files changed, 178 insertions(+), 35 deletions(-) create mode 100644 src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java diff --git a/src/main/java/codes/thischwa/dyndrest/repository/UpdateLogRepo.java b/src/main/java/codes/thischwa/dyndrest/repository/UpdateLogRepo.java index 4428175..02c11de 100644 --- a/src/main/java/codes/thischwa/dyndrest/repository/UpdateLogRepo.java +++ b/src/main/java/codes/thischwa/dyndrest/repository/UpdateLogRepo.java @@ -46,4 +46,14 @@ public interface UpdateLogRepo + "join ZONE z on z.ID = h.ZONE_ID " + "order by u.CHANGED DESC") List findAllFullUpdateLogs(); + + @Query( + "select u.ID, u.HOST_ID, u.IPV4, u.IPV6, u.CHANGED, u.CHANGED_UPDATE, u.STATUS, " + + "concat(h.NAME, '.', z.NAME) host " + + "from UPDATE_LOG u " + + "join HOST h on h.ID = u.HOST_ID " + + "join ZONE z on z.ID = h.ZONE_ID " + + "where u.HOST_ID = :hostId " + + "order by u.CHANGED DESC") + List findByHostId(Integer hostId); } diff --git a/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java b/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java index 16dae0b..e4c2440 100644 --- a/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java +++ b/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java @@ -37,37 +37,54 @@ public class DatabaseRestoreHandler extends PostProcessor { @Override public void process(Collection wantedBeans) throws Exception { - AppConfig appConfig = (AppConfig) wantedBeans.stream() - .filter(AppConfig.class::isInstance) - .findFirst() - .orElseThrow(() -> new IllegalStateException("AppConfig not found.")); - DataSource dataSource = (DataSource) wantedBeans.stream() - .filter(DataSource.class::isInstance) - .findFirst() - .orElseThrow(() -> new IllegalStateException("Datasource not found.")); - AppConfig.Database databaseConfig = appConfig.database(); - this.jdbcTemplate = new JdbcTemplate(dataSource); + setupRestorationParams(wantedBeans); + if ((!dbExists && !restoreEnabled) || (dbExists && restoreEnabled)) { + restore(); + } + } + private void setupRestorationParams(Collection wantedBeans) { + AppConfig appConfig = getAppConfig(wantedBeans); + jdbcTemplate = new JdbcTemplate(getDataSource(wantedBeans)); dbExists = !isDatabaseEmpty(); + + AppConfig.Database databaseConfig = appConfig.database(); AppConfig.Database.Restore dbRestore = databaseConfig.restore(); - if (dbRestore == null) { - restoreEnabled = false; - return; + + if (dbRestore != null && dbRestore.enabled()) { + setupRestorePath(databaseConfig, dbRestore); } - if (dbRestore.enabled()) { - restorePath = Paths.get(dbRestore.path(), databaseConfig.dumpFile()).normalize(); - if (Files.exists(restorePath)) { - log.info("Database restore enabled and path exists: {}", restorePath); - restoreEnabled = true; - restorePathBak = - Paths.get(dbRestore.path(), databaseConfig.dumpFile() + ".bak").normalize(); - } else { - log.warn("Database restore is enabled, but path does not exist: {}", restorePath); - } + } + + private AppConfig getAppConfig(Collection wantedBeans) { + return (AppConfig) + wantedBeans.stream() + .filter(AppConfig.class::isInstance) + .findFirst() + .orElseThrow(() -> new IllegalStateException("AppConfig not found.")); + } + + private DataSource getDataSource(Collection wantedBeans) { + return (DataSource) + wantedBeans.stream() + .filter(DataSource.class::isInstance) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Datasource not found.")); + } + + private void setupRestorePath( + AppConfig.Database databaseConfig, AppConfig.Database.Restore dbRestore) { + restorePath = Paths.get(dbRestore.path(), databaseConfig.dumpFile()).normalize(); + + if (Files.exists(restorePath)) { + restoreEnabled = true; + restorePathBak = Paths.get(dbRestore.path(), databaseConfig.dumpFile() + ".bak").normalize(); + log.info("Database restore enabled and path exists: {}", restorePath); } else { + log.warn("Database restore is enabled, but path does not exist: {}", restorePath); restoreEnabled = false; + restorePath = null; } - restore(); } /** @@ -83,13 +100,21 @@ public void restore() throws Exception { if (restoreEnabled) { assert restorePath != null; assert restorePathBak != null; + log.info("Database exists and restore is enabled, try to restore it from the last dump."); populate(ds); renameDump(); } } else { - log.info("Embedded database is empty, try restore it from the last dump!"); - populate(ds); - renameDump(); + if (restoreEnabled) { + log.info( + "Embedded database doesn't exists and restore is enabled, try restore it from the last dump!"); + populate(ds); + renameDump(); + } else { + log.info( + "Embedded database doesn't exists and restore is disabled, try restore the schema!"); + populate(ds); + } } } diff --git a/src/main/java/codes/thischwa/dyndrest/service/HostZoneService.java b/src/main/java/codes/thischwa/dyndrest/service/HostZoneService.java index fa430e4..9d1d13c 100644 --- a/src/main/java/codes/thischwa/dyndrest/service/HostZoneService.java +++ b/src/main/java/codes/thischwa/dyndrest/service/HostZoneService.java @@ -178,4 +178,13 @@ public Optional> findHostsOfZone(String zoneName) { } return Optional.of(hostRepo.findByZoneId(zone.getId())); } + + /** + * Deletes a zone from the repository. + * + * @param zone the zone to be deleted + */ + public void deleteZone(Zone zone) { + zoneRepo.delete(zone); + } } diff --git a/src/main/resources/h2/schema.sql b/src/main/resources/h2/schema.sql index 94bb4b2..121d9fe 100644 --- a/src/main/resources/h2/schema.sql +++ b/src/main/resources/h2/schema.sql @@ -23,7 +23,7 @@ create table HOST constraint HOST_UNIQUE unique (ZONE_ID, NAME), constraint HOST_ZONE_ID___FK - foreign key (ZONE_ID) references ZONE + foreign key (ZONE_ID) references ZONE on delete cascade ); comment on column HOST.NAME is 'prefix of the full host name'; @@ -38,7 +38,7 @@ create table UPDATE_LOG CHANGED_UPDATE TIMESTAMP, STATUS ENUM ('success', 'failed'), constraint HOST_ID___FK - foreign key (HOST_ID) references PUBLIC.HOST + foreign key (HOST_ID) references HOST on delete cascade ); diff --git a/src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java b/src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java new file mode 100644 index 0000000..6aa4077 --- /dev/null +++ b/src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java @@ -0,0 +1,86 @@ +package codes.thischwa.dyndrest.repository; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import codes.thischwa.dyndrest.AbstractIntegrationTest; +import codes.thischwa.dyndrest.model.FullHost; +import codes.thischwa.dyndrest.model.Host; +import codes.thischwa.dyndrest.model.Zone; +import codes.thischwa.dyndrest.service.HostZoneService; +import java.time.LocalDateTime; +import java.util.List; +import java.util.Optional; + +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.DuplicateKeyException; +import org.springframework.data.relational.core.conversion.DbActionExecutionException; + +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +public class ConstraintTest extends AbstractIntegrationTest { + + @Autowired private HostZoneService service; + @Autowired private ZoneRepo zoneRepo; + @Autowired private HostRepo hostRepo; + @Autowired private UpdateLogRepo logRepo; + + @Test + void testDuplicateZone() { + Zone zone = new Zone(); + zone.setName("newzone1.info"); + zone.setNs("new1.ns.info"); + zone.setChanged(LocalDateTime.now()); + zoneRepo.save(zone); + + Zone duplicateZone = new Zone(); + duplicateZone.setName("newzone1.info"); + duplicateZone.setNs("new1.ns.info"); + duplicateZone.setChanged(LocalDateTime.now()); + try { + service.saveOrUpdate(zone); + } catch (DbActionExecutionException e) { + assertEquals(e.getCause().getClass(), DuplicateKeyException.class); + } + } + + @Test + void testDuplicateHost() { + Host host = new Host(); + host.setName("newhost1"); + host.setApiToken("token1"); + host.setZoneId(1); + host.setChanged(LocalDateTime.now()); + service.saveOrUpdate(host); + Host duplicateHost = new Host(); + duplicateHost.setName("newhost1"); + duplicateHost.setApiToken("token1"); + duplicateHost.setZoneId(1); + duplicateHost.setChanged(LocalDateTime.now()); + try { + service.saveOrUpdate(duplicateHost); + } catch (DbActionExecutionException e) { + assertEquals(e.getCause().getClass(), DuplicateKeyException.class); + } + } + + @Test + @Order(Integer.MAX_VALUE) + void testCascadeDelete() { + Zone zone = zoneRepo.findByName("dynhost0.info"); + Optional> fullHosts = service.findHostsOfZone(zone.getName()); + assertFalse(fullHosts.isEmpty()); + Integer hostId = fullHosts.get().get(0).getId(); + assertFalse(logRepo.findByHostId(hostId).isEmpty()); + + service.deleteZone(zone); + assertTrue(service.findHostsOfZone(zone.getName()).isEmpty()); + + assertTrue(logRepo.findByHostId(hostId).isEmpty()); + } +} diff --git a/src/test/java/codes/thischwa/dyndrest/repository/UpdateLogRepoTest.java b/src/test/java/codes/thischwa/dyndrest/repository/UpdateLogRepoTest.java index 0311e3b..9121a94 100644 --- a/src/test/java/codes/thischwa/dyndrest/repository/UpdateLogRepoTest.java +++ b/src/test/java/codes/thischwa/dyndrest/repository/UpdateLogRepoTest.java @@ -25,8 +25,14 @@ void testFindAllByStatus() { @Test void testFindById() { - List hosts = repo.findAllFullUpdateLogsByIds(List.of(41, 42)); - assertEquals(2, hosts.size()); + List logs = repo.findAllFullUpdateLogsByIds(List.of(41, 42)); + assertEquals(2, logs.size()); + } + + @Test + void testFindByHostId() { + List logs = repo.findByHostId(1); + assertFalse(logs.isEmpty()); } @Test diff --git a/src/test/java/codes/thischwa/dyndrest/service/HostZoneServiceTest.java b/src/test/java/codes/thischwa/dyndrest/service/HostZoneServiceTest.java index 7eae52d..21bcdcb 100644 --- a/src/test/java/codes/thischwa/dyndrest/service/HostZoneServiceTest.java +++ b/src/test/java/codes/thischwa/dyndrest/service/HostZoneServiceTest.java @@ -9,6 +9,8 @@ import java.time.LocalDateTime; import java.util.List; import java.util.Optional; + +import codes.thischwa.dyndrest.repository.UpdateLogRepo; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; @@ -22,6 +24,8 @@ class HostZoneServiceTest extends AbstractIntegrationTest { @Autowired private HostZoneService service; + @Autowired private UpdateLogRepo logRepo; + @Order(1) @Test void testGetConfiguredHosts() { @@ -40,7 +44,10 @@ void testGetConfiguredHosts() { void testGetConfiguredZones() { List zones = service.getConfiguredZones(); assertEquals(2, zones.size()); - Zone zone = zones.get(1); + Zone zone = zones.get(0); + assertEquals("dynhost0.info", zone.getName()); + assertEquals("ns0.domain.info", zone.getNs()); + zone = zones.get(1); assertEquals("dynhost1.info", zone.getName()); assertEquals("ns1.domain.info", zone.getNs()); } @@ -162,7 +169,7 @@ void testSaveFullHost() { @Order(10) @Test - void testSaveZone() { + void testSaveZoneDuplicate() { Zone zone = new Zone(); zone.setName("zone1.org"); zone.setNs("ns1.zone.org"); diff --git a/src/test/resources/h2/dump.sql b/src/test/resources/h2/dump.sql index 9756a12..f40ca4b 100644 --- a/src/test/resources/h2/dump.sql +++ b/src/test/resources/h2/dump.sql @@ -40,7 +40,7 @@ VALUES (4, 'test1', 2, '1234567890abcdex', TIMESTAMP '2024-01-28 12:06:29.821934 ALTER TABLE "HOST" ADD CONSTRAINT "HOST_UNIQUE" UNIQUE ("ZONE_ID", "NAME"); ALTER TABLE "HOST" - ADD CONSTRAINT "ZONE_ID___FK" FOREIGN KEY ("ZONE_ID") REFERENCES "ZONE" ("ID") NOCHECK; + ADD CONSTRAINT "ZONE_ID___FK" FOREIGN KEY ("ZONE_ID") REFERENCES "ZONE" ("ID") on delete cascade NOCHECK; CREATE TABLE "UPDATE_LOG" ( @@ -54,4 +54,4 @@ CREATE TABLE "UPDATE_LOG" ); ALTER TABLE "UPDATE_LOG" - ADD CONSTRAINT "HOST_ID___FK" FOREIGN KEY ("HOST_ID") REFERENCES "HOST" ("ID") NOCHECK \ No newline at end of file + ADD CONSTRAINT "HOST_ID___FK" FOREIGN KEY ("HOST_ID") REFERENCES "HOST" ("ID") ON DELETE CASCADE NOCHECK; From e45def58975e89acc49d78d44de83b28f1d62d46 Mon Sep 17 00:00:00 2001 From: Thilo Schwarz Date: Sun, 14 Jul 2024 11:31:18 +0200 Subject: [PATCH 2/3] reduce code smells --- .../{PostProcessor.java => BeanCollector.java} | 17 +++++++++++------ .../impl/domainrobot/DomainRobotProvider.java | 2 +- .../service/DatabaseRestoreHandler.java | 13 +++++-------- .../dyndrest/repository/ConstraintTest.java | 5 +---- 4 files changed, 18 insertions(+), 19 deletions(-) rename src/main/java/codes/thischwa/dyndrest/config/{PostProcessor.java => BeanCollector.java} (81%) diff --git a/src/main/java/codes/thischwa/dyndrest/config/PostProcessor.java b/src/main/java/codes/thischwa/dyndrest/config/BeanCollector.java similarity index 81% rename from src/main/java/codes/thischwa/dyndrest/config/PostProcessor.java rename to src/main/java/codes/thischwa/dyndrest/config/BeanCollector.java index 154cf65..ffd9450 100644 --- a/src/main/java/codes/thischwa/dyndrest/config/PostProcessor.java +++ b/src/main/java/codes/thischwa/dyndrest/config/BeanCollector.java @@ -10,23 +10,28 @@ import org.springframework.lang.Nullable; /** - * A Spring Bean post-processor that performs database restoration based on the provided - * configuration. This class is responsible for restoring the database if all the necessary - * conditions are met. + * BeanCollector is an abstract class that implements the BeanPostProcessor interface. It provides + * functionality for collecting and processing desired Spring managed beans. + * + *

During the bean post-processing, BeanCollector checks if the bean matches the desired types. + * Once all the desired beans are initialized, it triggers the processing logic by calling the + * process() method. The processBean() method is called internally to check and initialize the + * beans. */ @Slf4j -public abstract class PostProcessor implements BeanPostProcessor { +public abstract class BeanCollector implements BeanPostProcessor { private final Collection> wanted = new HashSet<>(); private final Collection initialized = new HashSet<>(); private boolean processed; - public PostProcessor() { + public BeanCollector() { Collections.addAll(wanted, getWanted()); } /** - * Retrieves the array of spring-managed bean classes that should be fetched by the post-processor. + * Retrieves the array of spring-managed bean classes that should be fetched by the + * post-processor. * * @return The array of classes. */ diff --git a/src/main/java/codes/thischwa/dyndrest/provider/impl/domainrobot/DomainRobotProvider.java b/src/main/java/codes/thischwa/dyndrest/provider/impl/domainrobot/DomainRobotProvider.java index a2b9d18..314f88b 100644 --- a/src/main/java/codes/thischwa/dyndrest/provider/impl/domainrobot/DomainRobotProvider.java +++ b/src/main/java/codes/thischwa/dyndrest/provider/impl/domainrobot/DomainRobotProvider.java @@ -74,7 +74,7 @@ private void checkZone(codes.thischwa.dyndrest.model.Zone myZone) private void checkHosts(Zone zone) throws IllegalArgumentException { Optional> opt = hostZoneService.findHostsOfZone(zone.getOrigin()); - if (opt.isEmpty()){ + if (opt.isEmpty()) { log.warn("No hosts found for zone: {}", zone.getOrigin()); return; } diff --git a/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java b/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java index e4c2440..57c6a0b 100644 --- a/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java +++ b/src/main/java/codes/thischwa/dyndrest/service/DatabaseRestoreHandler.java @@ -1,7 +1,7 @@ package codes.thischwa.dyndrest.service; import codes.thischwa.dyndrest.config.AppConfig; -import codes.thischwa.dyndrest.config.PostProcessor; +import codes.thischwa.dyndrest.config.BeanCollector; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -23,7 +23,7 @@ @Service @Profile("!test") @Slf4j -public class DatabaseRestoreHandler extends PostProcessor { +public class DatabaseRestoreHandler extends BeanCollector { private JdbcTemplate jdbcTemplate; @@ -118,13 +118,10 @@ public void restore() throws Exception { } } - private void renameDump() { - try { + private void renameDump() throws IOException { + assert restorePath != null; + assert restorePathBak != null; Files.move(restorePath, restorePathBak, StandardCopyOption.REPLACE_EXISTING); - log.info("Database restored successful, restore dump has moved to: {}", restorePathBak); - } catch (IOException e) { - throw new RuntimeException(e); - } } private boolean isDatabaseEmpty() { diff --git a/src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java b/src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java index 6aa4077..439e410 100644 --- a/src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java +++ b/src/test/java/codes/thischwa/dyndrest/repository/ConstraintTest.java @@ -2,7 +2,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import codes.thischwa.dyndrest.AbstractIntegrationTest; @@ -13,7 +12,6 @@ import java.time.LocalDateTime; import java.util.List; import java.util.Optional; - import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; @@ -23,11 +21,10 @@ import org.springframework.data.relational.core.conversion.DbActionExecutionException; @TestMethodOrder(MethodOrderer.OrderAnnotation.class) -public class ConstraintTest extends AbstractIntegrationTest { +class ConstraintTest extends AbstractIntegrationTest { @Autowired private HostZoneService service; @Autowired private ZoneRepo zoneRepo; - @Autowired private HostRepo hostRepo; @Autowired private UpdateLogRepo logRepo; @Test From d8e70be18295eb6968bf1591ab75c2eb64ad82b3 Mon Sep 17 00:00:00 2001 From: Thilo Schwarz Date: Sun, 14 Jul 2024 12:23:23 +0200 Subject: [PATCH 3/3] reduce code smells --- .../thischwa/dyndrest/config/BeanCollector.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/codes/thischwa/dyndrest/config/BeanCollector.java b/src/main/java/codes/thischwa/dyndrest/config/BeanCollector.java index ffd9450..5d16b58 100644 --- a/src/main/java/codes/thischwa/dyndrest/config/BeanCollector.java +++ b/src/main/java/codes/thischwa/dyndrest/config/BeanCollector.java @@ -25,7 +25,7 @@ public abstract class BeanCollector implements BeanPostProcessor { private final Collection initialized = new HashSet<>(); private boolean processed; - public BeanCollector() { + protected BeanCollector() { Collections.addAll(wanted, getWanted()); } @@ -54,9 +54,8 @@ public Object postProcessBeforeInitialization(Object bean, String beanName) /** * Performs post-processing after the initialization of a Spring bean. This method checks if the - * bean matches the desired types and initializes them. Once all the desired beans are - * initialized, it triggers the processing logic. This method should be implemented by a subclass - * of PostProcessor. + * bean matches the desired types. Once all the desired beans are initialized, it triggers the + * processing logic. This method should be implemented by a subclass of PostProcessor. * * @param bean The Spring bean instance to be post-processed. * @param beanName The name of the Spring bean. @@ -66,9 +65,9 @@ public Object postProcessBeforeInitialization(Object bean, String beanName) @Nullable @Override public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException { - processBean(bean); + collectBean(bean); if (!processed && wanted.isEmpty()) { - log.info("*** Wanted beans found!"); + log.info("*** Wanted beans found, start #process!"); try { process(initialized); } catch (Exception e) { @@ -81,7 +80,7 @@ public Object postProcessAfterInitialization(Object bean, String beanName) throw return BeanPostProcessor.super.postProcessAfterInitialization(bean, beanName); } - private void processBean(Object bean) { + private void collectBean(Object bean) { Collection> toBeRemoved = new HashSet<>(); for (Class wantedBean : wanted) { if (wantedBean.isInstance(bean) && !initialized.contains(bean)) {