diff --git a/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepository.java b/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepository.java index 29bd7d6b..67e10487 100644 --- a/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepository.java +++ b/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepository.java @@ -2,6 +2,7 @@ import java.util.List; +import eu.erasmuswithoutpaper.registryclient.RegistryClient; import org.springframework.beans.factory.DisposableBean; import org.eclipse.jgit.api.errors.GitAPIException; @@ -151,7 +152,7 @@ class ConfigurationException extends Exception { * @param contents A string with the catalogue contents to be stored. * @return true if the new content differs from the previous one. */ - boolean putCatalogue(String contents); + boolean putCatalogue(String contents, RegistryClient client); /** * Store a new filtered version of the manifest. diff --git a/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryImpl.java b/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryImpl.java index 411ec9c6..0ac1f89c 100644 --- a/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryImpl.java +++ b/src/main/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryImpl.java @@ -27,7 +27,6 @@ import eu.erasmuswithoutpaper.registryclient.RegistryClient.RefreshFailureException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; -import org.springframework.context.annotation.Lazy; import org.springframework.stereotype.Service; import com.google.common.collect.Iterables; @@ -60,9 +59,6 @@ */ @Service @SuppressFBWarnings("SIC_INNER_SHOULD_BE_STATIC_ANON") -// FIXME @Lazy is for preventing cycle in dependencies with RealInternet i production profile. -// This should be unraveled. -@Lazy @ConditionalOnWebApplication public class ManifestRepositoryImpl implements ManifestRepository { @@ -70,7 +66,6 @@ public class ManifestRepositoryImpl implements ManifestRepository { private final ManifestRepositoryImplProperties repoProperties; private final CatalogueDependantCache catcache; - private RegistryClient client = null; private final Git git; private final ReentrantReadWriteLock lock; @@ -105,7 +100,6 @@ public ManifestRepositoryImpl(ManifestRepositoryImplProperties repoProperties, this.index = this.loadIndex().get(); } else { this.index = new TreeSet<>(); - this.deleteAll(); this.flushIndex(); this.commit("Upgrade repository structure"); } @@ -158,7 +152,7 @@ public boolean commit(String message) { * Remember to {@link #commit(String)} the changes afterwards, for logging purposes. *

*/ - public final void deleteAll() { + public final void deleteAll(RegistryClient client) { this.lock.writeLock().lock(); try { Path path = this.repoProperties.getFileSystem().getPath(this.repoProperties.getPath()); @@ -202,7 +196,7 @@ public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOExce this.cachedCatalogueContent = null; this.index.clear(); this.flushIndex(); - this.onCatalogueContentChanged(); + this.onCatalogueContentChanged(client); } catch (IOException e) { throw new RuntimeException(e); } finally { @@ -393,13 +387,13 @@ public boolean push() throws TransportException, GitAPIException, ConfigurationE } @Override - public boolean putCatalogue(String contents) { + public boolean putCatalogue(String contents, RegistryClient client) { this.lock.writeLock().lock(); try { boolean changed = this.writeFile(this.getPathForCatalogue(), contents); this.cachedCatalogueContent = contents; if (changed) { - this.onCatalogueContentChanged(); + this.onCatalogueContentChanged(client); } return changed; } finally { @@ -443,18 +437,6 @@ public void releaseWriteLock() { this.lock.writeLock().unlock(); } - /** - * Once set, {@link ManifestRepositoryImpl} will refresh this {@link RegistryClient} whenever the - * catalogue is changed. - * - * @param client The client to be kept in sync. - */ - @Autowired - public void setRegistryClient(RegistryClient client) { - this.client = client; - this.onCatalogueContentChanged(); - } - private void addToIndex(String url) { this.lock.writeLock().lock(); try { @@ -526,14 +508,12 @@ private Optional> loadIndex() { return Optional.of(result); } - private void onCatalogueContentChanged() { + private void onCatalogueContentChanged(RegistryClient client) { this.catcache.clear(); - if (this.client != null) { - try { - this.client.refresh(); - } catch (RefreshFailureException e) { - logger.error("Local registry client refresh failed: " + e); - } + try { + client.refresh(); + } catch (RefreshFailureException e) { + logger.error("Local registry client refresh failed: " + e); } } diff --git a/src/main/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterImpl.java b/src/main/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterImpl.java index 85f11ca7..85b6d6cf 100644 --- a/src/main/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterImpl.java +++ b/src/main/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterImpl.java @@ -437,7 +437,7 @@ private void updateTheCatalogue(boolean commit) { // Store it. - this.repo.putCatalogue(catalogueXml); + this.repo.putCatalogue(catalogueXml, registryClient); if (commit) { this.repo.commit("Update catalogue"); } diff --git a/src/test/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryTest.java b/src/test/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryTest.java index 8d9e4686..0a079986 100644 --- a/src/test/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryTest.java +++ b/src/test/java/eu/erasmuswithoutpaper/registry/repository/ManifestRepositoryTest.java @@ -6,6 +6,7 @@ import java.nio.charset.StandardCharsets; import eu.erasmuswithoutpaper.registry.WRTest; +import eu.erasmuswithoutpaper.registryclient.RegistryClient; import org.springframework.beans.factory.annotation.Autowired; import org.junit.jupiter.api.AfterEach; @@ -26,6 +27,9 @@ public static void setUpClass() { manifestUrl2 = "https://example.com/manifest2.xml"; } + @Autowired + private RegistryClient client; + @Autowired private ManifestRepositoryImpl repo; @@ -34,26 +38,26 @@ public static void setUpClass() { @AfterEach public void tearDown() { - this.repo.deleteAll(); + this.repo.deleteAll(client); } @Test public void testCatalogueDependantCache() { - this.repo.deleteAll(); + this.repo.deleteAll(client); - this.repo.putCatalogue("1"); + this.repo.putCatalogue("1", client); this.catcache.putCoverageMatrixHtml("1"); assertThat(this.catcache.getCoverageMatrixHtml()).isEqualTo("1"); - this.repo.putCatalogue("1"); + this.repo.putCatalogue("1", client); assertThat(this.catcache.getCoverageMatrixHtml()).isEqualTo("1"); - this.repo.putCatalogue("2"); + this.repo.putCatalogue("2", client); assertThat(this.catcache.getCoverageMatrixHtml()).isNull(); this.catcache.putCoverageMatrixHtml("2"); assertThat(this.catcache.getCoverageMatrixHtml()).isEqualTo("2"); - this.repo.putCatalogue("1"); + this.repo.putCatalogue("1", client); assertThat(this.catcache.getCoverageMatrixHtml()).isNull(); } @@ -63,7 +67,7 @@ public void testCatalogueDependantCache() { @Test public void testCatalogueStorage() { - this.repo.deleteAll(); + this.repo.deleteAll(client); // No catalogue should exist before this test is run. @@ -84,10 +88,10 @@ public void testCatalogueStorage() { // should accept any string here. this.repo.commit("commit things which were changed before"); - this.repo.putCatalogue("a string with the catalogue"); + this.repo.putCatalogue("a string with the catalogue", client); boolean r = this.repo.commit("commit the catalogue"); assertThat(r).isTrue(); - this.repo.putCatalogue("a string with the catalogue"); + this.repo.putCatalogue("a string with the catalogue", client); r = this.repo.commit("try to commit unchanged catalogue"); assertThat(r).isFalse(); @@ -107,14 +111,14 @@ public void testCatalogueStorage() { * inconsistent. */ public void testDeleteAll() { - this.repo.deleteAll(); + this.repo.deleteAll(client); assertThat(this.repo.getAllFilePaths()).containsExactly("index.xml"); this.repo.putOriginalManifest(manifestUrl1, "some string".getBytes(StandardCharsets.UTF_8)); this.repo.putFilteredManifest(manifestUrl2, "some string"); assertThat(this.repo.getAllFilePaths()).containsExactlyInAnyOrder("index.xml", "manifests/com/example.com/51d9ca82ce863381a7648647ab688b966f3b2260-filtered.xml", "manifests/com/example.com/bb937788ce84767ff64935e70c3856bd8c7bd16d.xml"); - this.repo.deleteAll(); + this.repo.deleteAll(client); assertThat(this.repo.getAllFilePaths()).containsExactly("index.xml"); } @@ -125,7 +129,7 @@ public void testDeleteAll() { @Test public void testManifestStorage() { - this.repo.deleteAll(); + this.repo.deleteAll(client); // Attempt to get a nonexistent manifest from the repo. diff --git a/src/test/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterTest.java b/src/test/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterTest.java index d0745d9c..e60e9acf 100644 --- a/src/test/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterTest.java +++ b/src/test/java/eu/erasmuswithoutpaper/registry/updater/RegistryUpdaterTest.java @@ -139,7 +139,7 @@ public void checkIfOfficialExamplesAreConsistent() { @BeforeEach public void setUp() { this.sourceProvider.clearSources(); - this.repo.deleteAll(); + this.repo.deleteAll(regClient); this.internet.clearURLs(); this.internet.clearEmailsSent(); this.lastCatalogue = null; diff --git a/src/test/java/eu/erasmuswithoutpaper/registry/validators/AbstractApiTest.java b/src/test/java/eu/erasmuswithoutpaper/registry/validators/AbstractApiTest.java index ce28bb5d..f31fcecd 100644 --- a/src/test/java/eu/erasmuswithoutpaper/registry/validators/AbstractApiTest.java +++ b/src/test/java/eu/erasmuswithoutpaper/registry/validators/AbstractApiTest.java @@ -141,7 +141,7 @@ public void setUp() { * consistent with the certificates returned by the validator. */ this.sourceProvider.clearSources(); - this.repo.deleteAll(); + this.repo.deleteAll(client); this.internet.clearAll(); Map registryManifests = this.selfManifestProvider.getManifests(); diff --git a/src/test/java/eu/erasmuswithoutpaper/registry/web/ApiControllerIntegrationTest.java b/src/test/java/eu/erasmuswithoutpaper/registry/web/ApiControllerIntegrationTest.java index 02405f50..c68b224e 100644 --- a/src/test/java/eu/erasmuswithoutpaper/registry/web/ApiControllerIntegrationTest.java +++ b/src/test/java/eu/erasmuswithoutpaper/registry/web/ApiControllerIntegrationTest.java @@ -16,7 +16,7 @@ import eu.erasmuswithoutpaper.registry.sourceprovider.ManifestSourceFactory; import eu.erasmuswithoutpaper.registry.sourceprovider.TestManifestSourceProvider; import eu.erasmuswithoutpaper.registry.updater.RegistryUpdaterTest; - +import eu.erasmuswithoutpaper.registryclient.RegistryClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.http.HttpStatus; @@ -41,6 +41,9 @@ public class ApiControllerIntegrationTest extends WRIntegrationTest { @Autowired private EwpDocBuilder docBuilder; + @Autowired + private RegistryClient client; + @Autowired private ManifestRepositoryImpl repo; @@ -104,7 +107,7 @@ public void respondsValid500() { */ @Test public void servesTheCatalogue() { - this.repo.putCatalogue(""); + this.repo.putCatalogue("", client); ResponseEntity response = this.template.getForEntity(this.baseURL + "/catalogue-v1.xml", String.class); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); @@ -152,7 +155,7 @@ public void testScenario1() { Lists.newArrayList(new RestrictInstitutionsCovered("^uw\\.edu\\.pl$")))); this.manifestSourceProvider.addSource(manifestFactory.newRegularSource(urlSE, Lists.newArrayList(new RestrictInstitutionsCovered("^.+\\.se$")))); - this.repo.deleteAll(); + this.repo.deleteAll(client); // this.internet.putURL(urlSelf, this.apiController.getSelfManifest().getBody()); /* Call the refresh URL and verify its response status. */ @@ -267,7 +270,7 @@ public void testScenario1() { } private int forceReload(String manifestUrl) { - MultiValueMap params = new LinkedMultiValueMap(); + MultiValueMap params = new LinkedMultiValueMap<>(); params.add("url", manifestUrl); ResponseEntity response = this.template.postForEntity(this.baseURL + "/reload", params, String.class);