From 95b2d335fa10cf1462c84463f5302ac53e4931a4 Mon Sep 17 00:00:00 2001 From: jensp Date: Fri, 3 Feb 2017 07:40:13 +0000 Subject: [PATCH] CCM NG: Some repositories still returned null instead of Optional git-svn-id: https://svn.libreccm.org/ccm/ccm_ng@4550 8810af33-2d31-482b-a856-94f89814c4df --- .../cms/ContentCenterAppCreator.java | 2 +- .../contentsection/ContentSectionCreator.java | 2 +- .../applications/AdminApplicationCreator.java | 2 +- .../ApplicationsAdministrationTab.java | 8 +- .../ui/login/LoginApplicationCreator.java | 2 +- .../admin/ui/AdminJsfApplicationCreator.java | 2 +- .../categorization/CategoryRepository.java | 45 +++++------ .../categorization/DomainRepository.java | 7 +- .../libreccm/web/ApplicationRepository.java | 7 +- .../categorization/CategoryManagerTest.java | 2 +- .../CategoryRepositoryTest.java | 75 +++++++++++-------- .../ShortcutsApplicationCreator.java | 2 +- 12 files changed, 86 insertions(+), 70 deletions(-) diff --git a/ccm-cms/src/main/java/com/arsdigita/cms/ContentCenterAppCreator.java b/ccm-cms/src/main/java/com/arsdigita/cms/ContentCenterAppCreator.java index 557a4c0c4..8e5f1004d 100644 --- a/ccm-cms/src/main/java/com/arsdigita/cms/ContentCenterAppCreator.java +++ b/ccm-cms/src/main/java/com/arsdigita/cms/ContentCenterAppCreator.java @@ -46,7 +46,7 @@ public class ContentCenterAppCreator implements ApplicationCreator application = appRepo .retrieveApplicationForPath(selectedKey); final LegacyApplicationInstancePane pane; - if (application != null) { + if (application.isPresent()) { pane = instancePanes.get(application.getClass(). getName()); if (pane != null) { - pane.setApplication(application); + pane.setApplication(application.get()); } } else { pane = null; diff --git a/ccm-core/src/main/java/com/arsdigita/ui/login/LoginApplicationCreator.java b/ccm-core/src/main/java/com/arsdigita/ui/login/LoginApplicationCreator.java index eaa7dc99b..236541c79 100644 --- a/ccm-core/src/main/java/com/arsdigita/ui/login/LoginApplicationCreator.java +++ b/ccm-core/src/main/java/com/arsdigita/ui/login/LoginApplicationCreator.java @@ -47,7 +47,7 @@ public class LoginApplicationCreator + "which is mounted at /login"); } - return appRepository.retrieveApplicationForPath(primaryUrl); + return appRepository.retrieveApplicationForPath(primaryUrl).get(); } } diff --git a/ccm-core/src/main/java/org/libreccm/admin/ui/AdminJsfApplicationCreator.java b/ccm-core/src/main/java/org/libreccm/admin/ui/AdminJsfApplicationCreator.java index 398bcd20a..aa27b8484 100644 --- a/ccm-core/src/main/java/org/libreccm/admin/ui/AdminJsfApplicationCreator.java +++ b/ccm-core/src/main/java/org/libreccm/admin/ui/AdminJsfApplicationCreator.java @@ -45,7 +45,7 @@ public class AdminJsfApplicationCreator implements ApplicationCreator } @Transactional(Transactional.TxType.REQUIRED) - public Category findByPath(final String path) { + public Optional findByPath(final String path) { if (path == null || path.isEmpty()) { throw new IllegalArgumentException("Path can't be null or empty."); } @@ -102,18 +102,18 @@ public class CategoryRepository extends AbstractEntityRepository + "Valid path format: domainKey:path"); } - final Domain domain = domainRepo.findByDomainKey(tokens[0]); - if (domain == null) { + final Optional domain = domainRepo.findByDomainKey(tokens[0]); + if (domain.isPresent()) { + return findByPath(domain.get(), tokens[1]); + } else { throw new InvalidCategoryPathException(String.format( "No domain identified by the key '%s' found.", tokens[0])); } - - return findByPath(domain, tokens[1]); } @Transactional(Transactional.TxType.REQUIRED) - public Category findByPath(final Domain domain, final String path) { + public Optional findByPath(final Domain domain, final String path) { if (domain == null) { throw new IllegalArgumentException("Domain can't be null."); } @@ -133,37 +133,40 @@ public class CategoryRepository extends AbstractEntityRepository } LOGGER.debug("Trying to find category with path \"{}\" in " - + "domain \"{}\".", + + "domain \"{}\".", normalizedPath, domain.getDomainKey()); final String[] tokens = normalizedPath.split("/"); Category current = domain.getRoot(); for (final String token : tokens) { if (current.getSubCategories() == null) { - return null; + return Optional.empty(); } final Optional result = current.getSubCategories() .stream() - .filter((c) -> { - LOGGER.debug("#findByPath(Domain, String): c = {}", - c.toString()); - LOGGER.debug( - "#findByPath(Domain, String): c.getName = \"{}\"", - c.getName()); - LOGGER.debug("#findByPath(Domain, String): token = \"{}\"", - token); - return c.getName().equals(token); - }) + .filter(c -> filterCategoryByName(c, token)) .findFirst(); - + if (result.isPresent()) { current = result.get(); } else { - return null; + return Optional.empty(); } } - return current; + return Optional.of(current); + } + + private boolean filterCategoryByName(final Category category, + final String name) { + LOGGER.debug("#findByPath(Domain, String): c = {}", + category.toString()); + LOGGER.debug( + "#findByPath(Domain, String): c.getName = \"{}\"", + category.getName()); + LOGGER.debug("#findByPath(Domain, String): token = \"{}\"", + name); + return category.getName().equals(name); } @AuthorizationRequired diff --git a/ccm-core/src/main/java/org/libreccm/categorization/DomainRepository.java b/ccm-core/src/main/java/org/libreccm/categorization/DomainRepository.java index 976be720d..591bc2d76 100644 --- a/ccm-core/src/main/java/org/libreccm/categorization/DomainRepository.java +++ b/ccm-core/src/main/java/org/libreccm/categorization/DomainRepository.java @@ -24,6 +24,7 @@ import org.libreccm.security.RequiresPrivilege; import java.net.URI; import java.util.List; +import java.util.Optional; import java.util.UUID; import javax.enterprise.context.RequestScoped; @@ -76,7 +77,7 @@ public class DomainRepository extends AbstractEntityRepository { * @return The {@code Domain} identified by {@code domainKey} or * {@code null} if there is no such {@code Domain}. */ - public Domain findByDomainKey(final String domainKey) { + public Optional findByDomainKey(final String domainKey) { final TypedQuery query = entityManager.createNamedQuery( "Domain.findByKey", Domain.class); query.setParameter("key", domainKey); @@ -86,9 +87,9 @@ public class DomainRepository extends AbstractEntityRepository { query.setHint("javax.persistence.fetchgraph", graph); try { - return query.getSingleResult(); + return Optional.of(query.getSingleResult()); } catch (NoResultException ex) { - return null; + return Optional.empty(); } } diff --git a/ccm-core/src/main/java/org/libreccm/web/ApplicationRepository.java b/ccm-core/src/main/java/org/libreccm/web/ApplicationRepository.java index 068efe3f4..35d01cd9d 100644 --- a/ccm-core/src/main/java/org/libreccm/web/ApplicationRepository.java +++ b/ccm-core/src/main/java/org/libreccm/web/ApplicationRepository.java @@ -24,6 +24,7 @@ import org.libreccm.security.AuthorizationRequired; import org.libreccm.security.RequiresPrivilege; import java.util.List; +import java.util.Optional; import javax.enterprise.context.RequestScoped; import javax.persistence.NoResultException; @@ -57,15 +58,15 @@ public class ApplicationRepository * is no application mounted at that {@code path}. */ @Transactional(Transactional.TxType.REQUIRED) - public CcmApplication retrieveApplicationForPath(final String path) { + public Optional retrieveApplicationForPath(final String path) { final TypedQuery query = getEntityManager() .createNamedQuery("CcmApplication.retrieveApplicationForPath", CcmApplication.class); query.setParameter("path", path); try { - return query.getSingleResult(); + return Optional.of(query.getSingleResult()); } catch (NoResultException ex) { - return null; + return Optional.empty(); } } diff --git a/ccm-core/src/test/java/org/libreccm/categorization/CategoryManagerTest.java b/ccm-core/src/test/java/org/libreccm/categorization/CategoryManagerTest.java index 4c971fb06..4c0487778 100644 --- a/ccm-core/src/test/java/org/libreccm/categorization/CategoryManagerTest.java +++ b/ccm-core/src/test/java/org/libreccm/categorization/CategoryManagerTest.java @@ -659,7 +659,7 @@ public class CategoryManagerTest { public void createMultipleCategories() { shiro.getSystemUser().execute(() -> { - final Domain domain = domainRepo.findByDomainKey("test"); + final Domain domain = domainRepo.findByDomainKey("test").get(); final Category root = domain.getRoot(); final Category com = new Category(); diff --git a/ccm-core/src/test/java/org/libreccm/categorization/CategoryRepositoryTest.java b/ccm-core/src/test/java/org/libreccm/categorization/CategoryRepositoryTest.java index 3a73a6171..0ae631afb 100644 --- a/ccm-core/src/test/java/org/libreccm/categorization/CategoryRepositoryTest.java +++ b/ccm-core/src/test/java/org/libreccm/categorization/CategoryRepositoryTest.java @@ -54,8 +54,10 @@ import static org.libreccm.testutils.DependenciesHelpers.*; import org.jboss.arquillian.persistence.CleanupUsingScript; +import java.util.Optional; + /** - * + * * @author Jens Pelzetter */ @org.junit.experimental.categories.Category(IntegrationTest.class) @@ -160,23 +162,26 @@ public class CategoryRepositoryTest { "datasets/org/libreccm/categorization/CategoryRepositoryTest/data.yml") @InSequence(1100) public void findByPathString() { - final Category category1 = categoryRepo.findByPath("test:/foo/bar/"); - final Category category2 = categoryRepo.findByPath("test:/foo/bar"); - final Category category3 = categoryRepo.findByPath("test:/foo/"); + final Optional category1 = categoryRepo.findByPath( + "test:/foo/bar/"); + final Optional category2 = categoryRepo.findByPath( + "test:/foo/bar"); + final Optional category3 = categoryRepo.findByPath( + "test:/foo/"); - final Category notFound = categoryRepo + final Optional notFound = categoryRepo .findByPath("test:/does/not/exist"); - assertThat(category1, is(not(nullValue()))); - assertThat(category1.getName(), is(equalTo("bar"))); + assertThat(category1.isPresent(), is(true)); + assertThat(category1.get().getName(), is(equalTo("bar"))); - assertThat(category2, is(not(nullValue()))); - assertThat(category2.getName(), is(equalTo("bar"))); + assertThat(category2.isPresent(), is(true)); + assertThat(category2.get().getName(), is(equalTo("bar"))); - assertThat(category3, is(not(nullValue()))); - assertThat(category3.getName(), is(equalTo("foo"))); + assertThat(category3.isPresent(), is(true)); + assertThat(category3.get().getName(), is(equalTo("foo"))); - assertThat(notFound, is(nullValue())); + assertThat(notFound.isPresent(), is(false)); } @Test @@ -184,10 +189,10 @@ public class CategoryRepositoryTest { "datasets/org/libreccm/categorization/CategoryRepositoryTest/data.yml") @InSequence(1150) public void findByPathStringNotExisting() { - final Category doesNotExist = categoryRepo.findByPath( + final Optional doesNotExist = categoryRepo.findByPath( "test:/does/not/exist"); - assertThat(doesNotExist, is(nullValue())); + assertThat(doesNotExist.isPresent(), is(false)); } @Test(expected = InvalidCategoryPathException.class) @@ -204,29 +209,33 @@ public class CategoryRepositoryTest { "datasets/org/libreccm/categorization/CategoryRepositoryTest/data.yml") @InSequence(2100) public void findByPathDomainString() { - final Domain domain = domainRepo.findByDomainKey("test"); + final Domain domain = domainRepo.findByDomainKey("test").get(); - final Category category1 = categoryRepo.findByPath(domain, "/foo/bar/"); - final Category category2 = categoryRepo.findByPath(domain, "foo/bar/"); - final Category category3 = categoryRepo.findByPath(domain, "/foo/bar"); - final Category category4 = categoryRepo.findByPath(domain, "foo/bar"); + final Optional category1 = categoryRepo.findByPath( + domain, "/foo/bar/"); + final Optional category2 = categoryRepo.findByPath( + domain, "foo/bar/"); + final Optional category3 = categoryRepo.findByPath( + domain, "/foo/bar"); + final Optional category4 = categoryRepo.findByPath( + domain, "foo/bar"); - final Category notFound = categoryRepo.findByPath(domain, - "/does/not/exist"); + final Optional notFound = categoryRepo.findByPath( + domain, "/does/not/exist"); - assertThat(category1, is(not(nullValue()))); - assertThat(category1.getName(), is(equalTo("bar"))); + assertThat(category1.isPresent(), is(true)); + assertThat(category1.get().getName(), is(equalTo("bar"))); - assertThat(category2, is(not(nullValue()))); - assertThat(category2.getName(), is(equalTo("bar"))); + assertThat(category2.isPresent(), is(true)); + assertThat(category2.get().getName(), is(equalTo("bar"))); - assertThat(category3, is(not(nullValue()))); - assertThat(category3.getName(), is(equalTo("bar"))); + assertThat(category3.isPresent(), is(true)); + assertThat(category3.get().getName(), is(equalTo("bar"))); - assertThat(category4, is(not(nullValue()))); - assertThat(category4.getName(), is(equalTo("bar"))); + assertThat(category4.isPresent(), is(true)); + assertThat(category4.get().getName(), is(equalTo("bar"))); - assertThat(notFound, is(nullValue())); + assertThat(notFound.isPresent(), is(false)); } @Test @@ -234,12 +243,12 @@ public class CategoryRepositoryTest { "datasets/org/libreccm/categorization/CategoryRepositoryTest/data.yml") @InSequence(1150) public void findByPathDomainStringNotExisting() { - final Domain domain = domainRepo.findByDomainKey("test"); + final Domain domain = domainRepo.findByDomainKey("test").get(); - final Category doesNotExist = categoryRepo.findByPath(domain, + final Optional doesNotExist = categoryRepo.findByPath(domain, "/does/not/exist"); - assertThat(doesNotExist, is(nullValue())); + assertThat(doesNotExist.isPresent(), is(false)); } @Test diff --git a/ccm-shortcuts/src/main/java/org/libreccm/shortcuts/ShortcutsApplicationCreator.java b/ccm-shortcuts/src/main/java/org/libreccm/shortcuts/ShortcutsApplicationCreator.java index 30e9ff2e2..17ef799bb 100644 --- a/ccm-shortcuts/src/main/java/org/libreccm/shortcuts/ShortcutsApplicationCreator.java +++ b/ccm-shortcuts/src/main/java/org/libreccm/shortcuts/ShortcutsApplicationCreator.java @@ -47,7 +47,7 @@ public class ShortcutsApplicationCreator + "/shortcuts"); } - return appRepository.retrieveApplicationForPath(primaryUrl); + return appRepository.retrieveApplicationForPath(primaryUrl).get(); }