From e4b42328e288b9fe05a951996996b9217324b6c6 Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Wed, 3 Feb 2016 17:48:37 +0100 Subject: [PATCH 01/11] Improve performance by using orElseGet when appropriate --- common/app/common/controllers/SunriseController.java | 8 +++++--- common/app/common/templates/CustomI18nHelper.java | 2 +- common/app/common/utils/PriceUtils.java | 2 +- common/app/shoppingcart/CartSessionUtils.java | 2 +- .../sdk/categories/CategoryTreeExtendedImpl.java | 3 +-- .../app/io/sphere/sdk/facets/SelectFacetImpl.java | 10 ++++++---- .../app/io/sphere/sdk/play/metrics/MetricAction.java | 1 + .../app/productcatalog/common/BreadcrumbData.java | 5 ++--- .../common/ProductCatalogController.java | 4 +--- .../app/productcatalog/common/ProductListData.java | 3 ++- .../productdetail/ProductDetailPageController.java | 6 +++--- .../productcatalog/productoverview/SearchCriteria.java | 5 +++-- .../checkout/address/CountriesFieldsBean.java | 4 ++-- .../app/shoppingcart/common/CartController.java | 4 ++-- 14 files changed, 31 insertions(+), 28 deletions(-) diff --git a/common/app/common/controllers/SunriseController.java b/common/app/common/controllers/SunriseController.java index 528a7344c..eeff44a97 100644 --- a/common/app/common/controllers/SunriseController.java +++ b/common/app/common/controllers/SunriseController.java @@ -7,7 +7,6 @@ import common.contexts.UserContext; import common.i18n.I18nResolver; import common.models.LocationSelector; -import common.models.MiniCart; import common.models.NavMenuData; import common.templates.TemplateService; import common.utils.PriceFormatter; @@ -24,7 +23,10 @@ import javax.annotation.Nullable; import javax.money.CurrencyUnit; import javax.money.Monetary; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Optional; import java.util.stream.IntStream; import static java.util.stream.Collectors.toList; @@ -150,7 +152,7 @@ private static CurrencyUnit currentCurrency(final CountryCode currentCountry, fi .map(countryCurrency -> { final CurrencyUnit currency = Monetary.getCurrency(countryCurrency.getCurrencyCode()); return projectContext.isCurrencyAccepted(currency) ? currency : projectContext.defaultCurrency(); - }).orElse(projectContext.defaultCurrency()); + }).orElseGet(projectContext::defaultCurrency); } private static List acceptedLocales(final String languageTag, final Http.Request request, diff --git a/common/app/common/templates/CustomI18nHelper.java b/common/app/common/templates/CustomI18nHelper.java index 519fff30c..2961f8326 100644 --- a/common/app/common/templates/CustomI18nHelper.java +++ b/common/app/common/templates/CustomI18nHelper.java @@ -31,7 +31,7 @@ public CharSequence apply(final String context, final Options options) throws IO private String resolveMessage(final Options options, final I18nIdentifier i18nIdentifier, final List locales) { return resolvePluralMessage(options, i18nIdentifier, locales) - .orElse(i18n.get(locales, i18nIdentifier.bundle, i18nIdentifier.key) + .orElseGet(() -> i18n.get(locales, i18nIdentifier.bundle, i18nIdentifier.key) .orElse(null)); } diff --git a/common/app/common/utils/PriceUtils.java b/common/app/common/utils/PriceUtils.java index eac9fb2d6..4f9aff66b 100644 --- a/common/app/common/utils/PriceUtils.java +++ b/common/app/common/utils/PriceUtils.java @@ -15,7 +15,7 @@ private PriceUtils() { public static MonetaryAmount calculateTotalPrice(final CartLike cartLike) { final Optional taxedPriceOpt = Optional.ofNullable(cartLike.getTaxedPrice()); - return taxedPriceOpt.map(TaxedPrice::getTotalGross).orElse(cartLike.getTotalPrice()); + return taxedPriceOpt.map(TaxedPrice::getTotalGross).orElseGet(cartLike::getTotalPrice); } public static Optional calculateSalesTax(final CartLike cartLike) { diff --git a/common/app/shoppingcart/CartSessionUtils.java b/common/app/shoppingcart/CartSessionUtils.java index 13fbcd2ae..d3d648260 100644 --- a/common/app/shoppingcart/CartSessionUtils.java +++ b/common/app/shoppingcart/CartSessionUtils.java @@ -19,7 +19,7 @@ private CartSessionUtils() { public static MiniCart getMiniCart(final Session session) { return Optional.ofNullable(session.get(CartSessionKeys.MINI_CART)) .map(miniCartAsJson -> SphereJsonUtils.readObject(miniCartAsJson, MiniCart.class)) - .orElse(new MiniCart()); + .orElseGet(MiniCart::new); } public static void overwriteCartSessionData(final Cart cart, final Session session, final UserContext userContext, diff --git a/move-to-sdk/app/io/sphere/sdk/categories/CategoryTreeExtendedImpl.java b/move-to-sdk/app/io/sphere/sdk/categories/CategoryTreeExtendedImpl.java index 0a7364f30..425d8a370 100644 --- a/move-to-sdk/app/io/sphere/sdk/categories/CategoryTreeExtendedImpl.java +++ b/move-to-sdk/app/io/sphere/sdk/categories/CategoryTreeExtendedImpl.java @@ -4,7 +4,6 @@ import java.util.*; -import static java.util.Collections.emptyList; import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toList; @@ -70,7 +69,7 @@ public Category getRootAncestor(final Category category) { private List getSiblings(final Category category) { return Optional.ofNullable(category.getParent()) .map(categoryTree::findChildren) - .orElse(emptyList()); + .orElseGet(Collections::emptyList); } private List getSubtreeAsFlatList(final Collection parentCategories) { diff --git a/move-to-sdk/app/io/sphere/sdk/facets/SelectFacetImpl.java b/move-to-sdk/app/io/sphere/sdk/facets/SelectFacetImpl.java index 1542fde25..1b6aca7a3 100644 --- a/move-to-sdk/app/io/sphere/sdk/facets/SelectFacetImpl.java +++ b/move-to-sdk/app/io/sphere/sdk/facets/SelectFacetImpl.java @@ -1,13 +1,15 @@ package io.sphere.sdk.facets; -import io.sphere.sdk.search.*; +import io.sphere.sdk.search.PagedSearchResult; +import io.sphere.sdk.search.TermFacetAndFilterExpression; +import io.sphere.sdk.search.TermFacetResult; import io.sphere.sdk.search.model.TermFacetAndFilterSearchModel; import javax.annotation.Nullable; +import java.util.Collections; import java.util.List; import java.util.Optional; -import static java.util.Collections.emptyList; import static java.util.stream.Collectors.toList; /** @@ -52,7 +54,7 @@ protected List getOptions() { .map(result -> result.getTerms().stream() .map(termStats -> FacetOption.ofTermStats(termStats, selectedValues)) .collect(toList())) - .orElse(emptyList()); + .orElseGet(Collections::emptyList); return mapper.apply(facetOptions); } @@ -69,7 +71,7 @@ public List getAllOptions() { @Override public List getLimitedOptions() { return limit.map(limit -> getOptions().stream().limit(limit).collect(toList())) - .orElse(getOptions()); + .orElseGet(this::getOptions); } @Override diff --git a/move-to-sdk/app/io/sphere/sdk/play/metrics/MetricAction.java b/move-to-sdk/app/io/sphere/sdk/play/metrics/MetricAction.java index 95790e874..9e306d935 100644 --- a/move-to-sdk/app/io/sphere/sdk/play/metrics/MetricAction.java +++ b/move-to-sdk/app/io/sphere/sdk/play/metrics/MetricAction.java @@ -27,6 +27,7 @@ public MetricAction(final Configuration configuration) { this.metricsEnabled = configuration.getBoolean(CONFIG_METRICS_ENABLED); } + @Override public F.Promise call(final Http.Context ctx) throws Throwable { if (metricsEnabled) { final List rawData = Collections.synchronizedList(new LinkedList<>()); diff --git a/product-catalog/app/productcatalog/common/BreadcrumbData.java b/product-catalog/app/productcatalog/common/BreadcrumbData.java index b12db5d48..c156e73f3 100644 --- a/product-catalog/app/productcatalog/common/BreadcrumbData.java +++ b/product-catalog/app/productcatalog/common/BreadcrumbData.java @@ -8,11 +8,10 @@ import io.sphere.sdk.products.ProductProjection; import io.sphere.sdk.products.ProductVariant; +import java.util.Collections; import java.util.List; -import java.util.Locale; import java.util.Optional; -import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static java.util.stream.Collectors.toList; @@ -38,7 +37,7 @@ public BreadcrumbData(final ProductProjection currentProduct, final ProductVaria this.links = currentProduct.getCategories().stream().findFirst() .flatMap(ref -> categoryTree.findById(ref.getId()) .map(currentCategory -> createCategoryBreadcrumb(currentCategory, categoryTree, userContext, reverseRouter)) - ).orElse(emptyList()); + ).orElseGet(Collections::emptyList); this.links.add(createProductLinkData(currentProduct, currentVariant, userContext, reverseRouter)); } diff --git a/product-catalog/app/productcatalog/common/ProductCatalogController.java b/product-catalog/app/productcatalog/common/ProductCatalogController.java index f4d1787fc..038be6ff6 100644 --- a/product-catalog/app/productcatalog/common/ProductCatalogController.java +++ b/product-catalog/app/productcatalog/common/ProductCatalogController.java @@ -10,8 +10,6 @@ import java.util.Collections; import java.util.List; -import static java.util.Collections.emptyList; - public abstract class ProductCatalogController extends SunriseController { private final ProductService productService; private final ProductDataConfig productDataConfig; @@ -27,7 +25,7 @@ public ProductCatalogController(final ControllerDependency controllerDependency, protected CategoryTree categoryTreeInNew() { final List categoriesInNew = newCategory() .map(Collections::singletonList) - .orElse(emptyList()); + .orElseGet(Collections::emptyList); return categoryTree().getSubtree(categoriesInNew); } diff --git a/product-catalog/app/productcatalog/common/ProductListData.java b/product-catalog/app/productcatalog/common/ProductListData.java index 34402bd72..b52d8b298 100644 --- a/product-catalog/app/productcatalog/common/ProductListData.java +++ b/product-catalog/app/productcatalog/common/ProductListData.java @@ -22,7 +22,8 @@ public ProductListData(final List productList, final ProductD final UserContext userContext, final ReverseRouter reverseRouter, final CategoryTree categoryTreeNew) { this.list = productList.stream() .map(product -> { - final ProductVariant matchingVariant = product.findFirstMatchingVariant().orElse(product.getMasterVariant()); + final ProductVariant matchingVariant = product.findFirstMatchingVariant() + .orElseGet(product::getMasterVariant); return new ProductThumbnailData(product, matchingVariant, productDataConfig, userContext, reverseRouter, categoryTreeNew); }) .collect(toList()); diff --git a/product-catalog/app/productcatalog/productdetail/ProductDetailPageController.java b/product-catalog/app/productcatalog/productdetail/ProductDetailPageController.java index de925d3c5..b60153af8 100644 --- a/product-catalog/app/productcatalog/productdetail/ProductDetailPageController.java +++ b/product-catalog/app/productcatalog/productdetail/ProductDetailPageController.java @@ -38,17 +38,17 @@ public F.Promise show(final String locale, final String slug, final Stri return productService().findProductBySlug(userContext.locale(), slug) .flatMap(productOpt -> productOpt .map(product -> renderProduct(userContext, slug, product, sku)) - .orElse(F.Promise.pure(notFound()))); + .orElseGet(() -> F.Promise.pure(notFound()))); } private F.Promise renderProduct(final UserContext userContext, final String slug, final ProductProjection product, final String sku) { - return product.findVariantBySky(sku) + return product.findVariantBySku(sku) .map(variant -> productService().getSuggestions(product, categoryTree(), numSuggestions).map(suggestions -> { final ProductDetailPageContent content = createPageContent(userContext, product, variant, suggestions); return (Result) ok(renderPage(userContext, content)); }) - ).orElse(redirectToMasterVariant(userContext, slug, product)); + ).orElseGet(() -> redirectToMasterVariant(userContext, slug, product)); } private F.Promise redirectToMasterVariant(final UserContext userContext, final String slug, diff --git a/product-catalog/app/productcatalog/productoverview/SearchCriteria.java b/product-catalog/app/productcatalog/productoverview/SearchCriteria.java index ddb73d042..0e5eabf97 100644 --- a/product-catalog/app/productcatalog/productoverview/SearchCriteria.java +++ b/product-catalog/app/productcatalog/productoverview/SearchCriteria.java @@ -18,6 +18,7 @@ import javax.annotation.Nullable; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Optional; @@ -65,7 +66,7 @@ private SearchCriteria(final Configuration configuration, final Http.Request req public List> boundFacets() { return categoryFacet() .map(categoryFacet -> asList(categoryFacet, colorFacet(), sizeFacet(), brandFacet())) - .orElse(asList(colorFacet(), sizeFacet(), brandFacet())); + .orElseGet(() -> asList(colorFacet(), sizeFacet(), brandFacet())); } public SortSelector boundSortSelector() { @@ -94,7 +95,7 @@ public List> selectedSort() { .filter(SortOption::isSelected) .map(SortOption::getSortExpressions) .findFirst() - .orElse(emptyList()); + .orElseGet(Collections::emptyList); } public Optional searchTerm() { diff --git a/shopping-cart/app/shoppingcart/checkout/address/CountriesFieldsBean.java b/shopping-cart/app/shoppingcart/checkout/address/CountriesFieldsBean.java index 557a2c7d1..306503f20 100644 --- a/shopping-cart/app/shoppingcart/checkout/address/CountriesFieldsBean.java +++ b/shopping-cart/app/shoppingcart/checkout/address/CountriesFieldsBean.java @@ -54,13 +54,13 @@ private void fill(final CountryCode selectedCountry, final UserContext userConte private CountryCode selectedCountry(final @Nullable String countryCode, final UserContext userContext) { return Optional.ofNullable(countryCode) .map(CountryCode::getByCode) - .orElse(userContext.country()); + .orElseGet(userContext::country); } private CountryCode selectedCountry(final @Nullable Address address, final UserContext userContext) { return Optional.ofNullable(address) .map(Address::getCountry) - .orElse(userContext.country()); + .orElseGet(userContext::country); } private SelectableData selectableDataFromCountry(final CountryCode countryCode, final boolean isSelected, final Locale locale) { diff --git a/shopping-cart/app/shoppingcart/common/CartController.java b/shopping-cart/app/shoppingcart/common/CartController.java index e0bae2edf..5b6321604 100644 --- a/shopping-cart/app/shoppingcart/common/CartController.java +++ b/shopping-cart/app/shoppingcart/common/CartController.java @@ -29,7 +29,7 @@ public CartController(final ControllerDependency controllerDependency) { protected F.Promise getOrCreateCart(final UserContext userContext, final Http.Session session) { return Optional.ofNullable(session(CartSessionKeys.CART_ID)) .map(this::fetchCart) - .orElse(createCart(userContext)) + .orElseGet(() -> createCart(userContext)) .flatMap(cart -> { CartSessionUtils.overwriteCartSessionData(cart, session, userContext, reverseRouter()); final boolean hasDifferentCountry = !userContext.country().equals(cart.getCountry()); @@ -61,7 +61,7 @@ private F.Promise updateCartCountry(final Cart cart, final CountryCode cou // TODO Handle case where some line items do not exist for this country final Address shippingAddress = Optional.ofNullable(cart.getShippingAddress()) .map(address -> address.withCountry(country)) - .orElse(Address.of(country)); + .orElseGet(() -> Address.of(country)); final CartUpdateCommand updateCommand = CartUpdateCommand.of(cart, asList(SetShippingAddress.of(shippingAddress), SetCountry.of(country))); return sphere().execute(updateCommand); From 24047bc978fdb5f194e1e906fbfd51a5c937a533 Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Thu, 4 Feb 2016 16:12:50 +0100 Subject: [PATCH 02/11] Split production modules into different concerns and in different packages --- .../CategoriesRefreshController.java | 2 +- app/categorytree/CategoryTreeProductionModule.java | 14 ++++++++++++++ .../CategoryTreeProvider.java | 4 ++-- .../RefreshableCategoryTree.java | 2 +- .../CtpClientProductionModule.java | 5 ++--- .../PlayJavaSphereClientProvider.java | 2 +- .../SphereClientProvider.java | 2 +- app/inject/ApplicationProductionModule.java | 6 +----- app/inject/CtpModelsProductionModule.java | 5 +---- app/inject/ProjectContextProvider.java | 1 + .../ReverseRouterImpl.java | 2 +- .../ReverseRouterProductionModule.java | 12 ++++++++++++ conf/routes | 2 +- 13 files changed, 39 insertions(+), 20 deletions(-) rename app/{controllers => categorytree}/CategoriesRefreshController.java (97%) create mode 100644 app/categorytree/CategoryTreeProductionModule.java rename app/{inject => categorytree}/CategoryTreeProvider.java (92%) rename app/{controllers => categorytree}/RefreshableCategoryTree.java (99%) rename app/{inject => ctpclient}/CtpClientProductionModule.java (87%) rename app/{inject => ctpclient}/PlayJavaSphereClientProvider.java (98%) rename app/{inject => ctpclient}/SphereClientProvider.java (99%) rename app/{pages => reverserouter}/ReverseRouterImpl.java (99%) create mode 100644 app/reverserouter/ReverseRouterProductionModule.java diff --git a/app/controllers/CategoriesRefreshController.java b/app/categorytree/CategoriesRefreshController.java similarity index 97% rename from app/controllers/CategoriesRefreshController.java rename to app/categorytree/CategoriesRefreshController.java index 64cc008ae..bcbb92837 100644 --- a/app/controllers/CategoriesRefreshController.java +++ b/app/categorytree/CategoriesRefreshController.java @@ -1,4 +1,4 @@ -package controllers; +package categorytree; import io.sphere.sdk.categories.CategoryTreeExtended; import play.mvc.Controller; diff --git a/app/categorytree/CategoryTreeProductionModule.java b/app/categorytree/CategoryTreeProductionModule.java new file mode 100644 index 000000000..eca54ecfc --- /dev/null +++ b/app/categorytree/CategoryTreeProductionModule.java @@ -0,0 +1,14 @@ +package categorytree; + +import com.google.inject.AbstractModule; +import io.sphere.sdk.categories.CategoryTreeExtended; + +import javax.inject.Singleton; + +public class CategoryTreeProductionModule extends AbstractModule { + + @Override + protected void configure() { + bind(CategoryTreeExtended.class).toProvider(CategoryTreeProvider.class).in(Singleton.class); + } +} diff --git a/app/inject/CategoryTreeProvider.java b/app/categorytree/CategoryTreeProvider.java similarity index 92% rename from app/inject/CategoryTreeProvider.java rename to app/categorytree/CategoryTreeProvider.java index 86c5f1547..66ff7078c 100644 --- a/app/inject/CategoryTreeProvider.java +++ b/app/categorytree/CategoryTreeProvider.java @@ -1,7 +1,7 @@ -package inject; +package categorytree; import com.google.inject.Provider; -import controllers.RefreshableCategoryTree; +import inject.SunriseInitializationException; import io.sphere.sdk.categories.CategoryTreeExtended; import io.sphere.sdk.client.SphereClient; import play.Logger; diff --git a/app/controllers/RefreshableCategoryTree.java b/app/categorytree/RefreshableCategoryTree.java similarity index 99% rename from app/controllers/RefreshableCategoryTree.java rename to app/categorytree/RefreshableCategoryTree.java index 0299c7829..494206258 100644 --- a/app/controllers/RefreshableCategoryTree.java +++ b/app/categorytree/RefreshableCategoryTree.java @@ -1,4 +1,4 @@ -package controllers; +package categorytree; import io.sphere.sdk.categories.Category; import io.sphere.sdk.categories.CategoryTree; diff --git a/app/inject/CtpClientProductionModule.java b/app/ctpclient/CtpClientProductionModule.java similarity index 87% rename from app/inject/CtpClientProductionModule.java rename to app/ctpclient/CtpClientProductionModule.java index 47a6baa37..966a51069 100644 --- a/app/inject/CtpClientProductionModule.java +++ b/app/ctpclient/CtpClientProductionModule.java @@ -1,4 +1,4 @@ -package inject; +package ctpclient; import com.google.inject.AbstractModule; import io.sphere.sdk.client.PlayJavaSphereClient; @@ -7,8 +7,7 @@ import javax.inject.Singleton; /** - * Configuration for the Guice {@link com.google.inject.Injector} which - * shall be used in production and integration tests. + * Configuration for the Guice {@link com.google.inject.Injector} which shall be used in production. */ public class CtpClientProductionModule extends AbstractModule { diff --git a/app/inject/PlayJavaSphereClientProvider.java b/app/ctpclient/PlayJavaSphereClientProvider.java similarity index 98% rename from app/inject/PlayJavaSphereClientProvider.java rename to app/ctpclient/PlayJavaSphereClientProvider.java index f2bc41d3e..3b46bee25 100644 --- a/app/inject/PlayJavaSphereClientProvider.java +++ b/app/ctpclient/PlayJavaSphereClientProvider.java @@ -1,4 +1,4 @@ -package inject; +package ctpclient; import com.google.inject.Provider; import io.sphere.sdk.client.PlayJavaSphereClient; diff --git a/app/inject/SphereClientProvider.java b/app/ctpclient/SphereClientProvider.java similarity index 99% rename from app/inject/SphereClientProvider.java rename to app/ctpclient/SphereClientProvider.java index 3d2cf15b8..a910c610a 100644 --- a/app/inject/SphereClientProvider.java +++ b/app/ctpclient/SphereClientProvider.java @@ -1,4 +1,4 @@ -package inject; +package ctpclient; import com.google.inject.Provider; import io.sphere.sdk.client.*; diff --git a/app/inject/ApplicationProductionModule.java b/app/inject/ApplicationProductionModule.java index ef5d21078..95168ec54 100644 --- a/app/inject/ApplicationProductionModule.java +++ b/app/inject/ApplicationProductionModule.java @@ -2,16 +2,13 @@ import com.google.inject.AbstractModule; import common.cms.CmsService; -import common.controllers.ReverseRouter; import common.i18n.I18nResolver; import common.templates.TemplateService; -import pages.ReverseRouterImpl; import javax.inject.Singleton; /** - * Configuration for the Guice {@link com.google.inject.Injector} which - * shall be used in production and integration tests. + * Configuration for the Guice {@link com.google.inject.Injector} which shall be used in production. */ public class ApplicationProductionModule extends AbstractModule { @@ -20,6 +17,5 @@ protected void configure() { bind(I18nResolver.class).toProvider(I18nResolverProvider.class).in(Singleton.class); bind(TemplateService.class).toProvider(TemplateServiceProvider.class).in(Singleton.class); bind(CmsService.class).toProvider(CmsServiceProvider.class).in(Singleton.class); - bind(ReverseRouter.class).toInstance(new ReverseRouterImpl()); } } diff --git a/app/inject/CtpModelsProductionModule.java b/app/inject/CtpModelsProductionModule.java index 10a7f6e1b..30b9f6062 100644 --- a/app/inject/CtpModelsProductionModule.java +++ b/app/inject/CtpModelsProductionModule.java @@ -3,22 +3,19 @@ import com.google.inject.AbstractModule; import common.contexts.ProjectContext; import common.models.ProductDataConfig; -import io.sphere.sdk.categories.CategoryTreeExtended; import productcatalog.services.ProductService; import shoppingcart.checkout.shipping.ShippingMethods; import javax.inject.Singleton; /** - * Configuration for the Guice {@link com.google.inject.Injector} which - * shall be used in production and integration tests. + * Configuration for the Guice {@link com.google.inject.Injector} which shall be used in production. */ public class CtpModelsProductionModule extends AbstractModule { @Override protected void configure() { bind(ProjectContext.class).toProvider(ProjectContextProvider.class).in(Singleton.class); - bind(CategoryTreeExtended.class).toProvider(CategoryTreeProvider.class).in(Singleton.class); bind(ProductDataConfig.class).toProvider(ProductDataConfigProvider.class).in(Singleton.class); bind(ProductService.class).toProvider(ProductServiceProvider.class).in(Singleton.class); bind(ShippingMethods.class).toProvider(ShippingMethodsProvider.class).in(Singleton.class); diff --git a/app/inject/ProjectContextProvider.java b/app/inject/ProjectContextProvider.java index 42b253cf2..20703efb3 100644 --- a/app/inject/ProjectContextProvider.java +++ b/app/inject/ProjectContextProvider.java @@ -3,6 +3,7 @@ import com.google.inject.Provider; import com.neovisionaries.i18n.CountryCode; import common.contexts.ProjectContext; +import inject.SunriseInitializationException; import io.sphere.sdk.client.SphereClient; import io.sphere.sdk.projects.Project; import io.sphere.sdk.projects.queries.ProjectGet; diff --git a/app/pages/ReverseRouterImpl.java b/app/reverserouter/ReverseRouterImpl.java similarity index 99% rename from app/pages/ReverseRouterImpl.java rename to app/reverserouter/ReverseRouterImpl.java index e9d2e1384..4902463f9 100644 --- a/app/pages/ReverseRouterImpl.java +++ b/app/reverserouter/ReverseRouterImpl.java @@ -1,4 +1,4 @@ -package pages; +package reverserouter; import common.controllers.ReverseRouter; import io.sphere.sdk.models.Base; diff --git a/app/reverserouter/ReverseRouterProductionModule.java b/app/reverserouter/ReverseRouterProductionModule.java new file mode 100644 index 000000000..ad4266842 --- /dev/null +++ b/app/reverserouter/ReverseRouterProductionModule.java @@ -0,0 +1,12 @@ +package reverserouter; + +import com.google.inject.AbstractModule; +import common.controllers.ReverseRouter; + +public class ReverseRouterProductionModule extends AbstractModule { + + @Override + protected void configure() { + bind(ReverseRouter.class).toInstance(new ReverseRouterImpl()); + } +} diff --git a/conf/routes b/conf/routes index 221ed242f..a3ab758ea 100644 --- a/conf/routes +++ b/conf/routes @@ -11,7 +11,7 @@ GET /assets/public/*file controllers.Asset GET /assets/$file<(css|js|fonts|img)/.*> controllers.WebJarAssets.at(file) # Endpoint to manually refresh category tree (otherwise an app restart is required) -GET /categories/refresh @controllers.CategoriesRefreshController.refresh() +GET /categories/refresh @categorytree.CategoriesRefreshController.refresh() # Shows the deployed version, artifactId and GIT SHA GET /version @controllers.StatusController.version() From 7bcc4212b84a4bf0714d878f6dec7c87368f51bc Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Thu, 4 Feb 2016 16:14:46 +0100 Subject: [PATCH 03/11] Implement basic auth --- app/basicauth/BasicAuth.java | 37 +++++++++++ app/basicauth/BasicAuthProductionModule.java | 16 +++++ app/basicauth/BasicAuthProvider.java | 40 ++++++++++++ app/basicauth/BasicAuthRequestHandler.java | 65 ++++++++++++++++++++ conf/application.conf | 10 ++- test/basicauth/BasicAuthTest.java | 32 ++++++++++ 6 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 app/basicauth/BasicAuth.java create mode 100644 app/basicauth/BasicAuthProductionModule.java create mode 100644 app/basicauth/BasicAuthProvider.java create mode 100644 app/basicauth/BasicAuthRequestHandler.java create mode 100644 test/basicauth/BasicAuthTest.java diff --git a/app/basicauth/BasicAuth.java b/app/basicauth/BasicAuth.java new file mode 100644 index 000000000..44f84b2f9 --- /dev/null +++ b/app/basicauth/BasicAuth.java @@ -0,0 +1,37 @@ +package basicauth; + +import java.util.Base64; + +/** + * Contains information related to the HTTP basic access authentication. + */ +public class BasicAuth { + private final String realm; + private final String encodedCredentials; + + private BasicAuth(final String realm, final String encodedCredentials) { + this.realm = realm; + this.encodedCredentials = encodedCredentials; + } + + public String getRealm() { + return realm; + } + + /** + * Decides whether the authentication header is valid, i.e. it is equal to "Basic username:password", + * where "username:password" is encoded in Base64 scheme. + * @param authorizationHeader the contents of the HTTP Authorization header + * @return true if the header complies with an Authorization header and contains the correct credentials, + * false otherwise + */ + public boolean isAuthorized(final String authorizationHeader) { + final String expectedAuthHeader = "Basic " + encodedCredentials; + return expectedAuthHeader.equals(authorizationHeader); + } + + public static BasicAuth of(final String realm, final String credentials) { + final String encodedCredentials = Base64.getEncoder().encodeToString(credentials.getBytes()); + return new BasicAuth(realm, encodedCredentials); + } +} diff --git a/app/basicauth/BasicAuthProductionModule.java b/app/basicauth/BasicAuthProductionModule.java new file mode 100644 index 000000000..3deca7cc9 --- /dev/null +++ b/app/basicauth/BasicAuthProductionModule.java @@ -0,0 +1,16 @@ +package basicauth; + +import com.google.inject.AbstractModule; + +import javax.inject.Singleton; + +/** + * Configuration for the Guice {@link com.google.inject.Injector} which shall be used in production. + */ +public class BasicAuthProductionModule extends AbstractModule { + + @Override + protected void configure() { + bind(BasicAuth.class).toProvider(BasicAuthProvider.class).in(Singleton.class); + } +} diff --git a/app/basicauth/BasicAuthProvider.java b/app/basicauth/BasicAuthProvider.java new file mode 100644 index 000000000..91fd2b385 --- /dev/null +++ b/app/basicauth/BasicAuthProvider.java @@ -0,0 +1,40 @@ +package basicauth; + +import inject.SunriseInitializationException; +import play.Configuration; +import play.Logger; + +import javax.annotation.Nullable; +import javax.inject.Inject; +import javax.inject.Provider; + +class BasicAuthProvider implements Provider { + public static final String CONFIG_REALM = "application.auth.realm"; + public static final String CONFIG_CREDENTIALS = "application.auth.credentials"; + public static final String REGEX_CREDENTIALS = "^[^ :]+:[^ :]+$"; + private final Configuration configuration; + + @Inject + public BasicAuthProvider(final Configuration configuration) { + this.configuration = configuration; + } + + @Nullable + @Override + public BasicAuth get() { + final String realm = configuration.getString(CONFIG_REALM, "Sunrise Authentication"); + final String credentials = configuration.getString(CONFIG_CREDENTIALS); + if (credentials != null && !credentials.isEmpty()) { + System.out.println(credentials); + if (credentials.matches(REGEX_CREDENTIALS)) { + Logger.debug("Basic authentication: enabled for realm \"{}\"", realm); + return BasicAuth.of(realm, credentials); + } else { + throw new SunriseInitializationException("Basic access authentication credentials must be of the form 'username:password', matching: " + REGEX_CREDENTIALS); + } + } else { + Logger.debug("Basic authentication: disabled"); + return null; + } + } +} diff --git a/app/basicauth/BasicAuthRequestHandler.java b/app/basicauth/BasicAuthRequestHandler.java new file mode 100644 index 000000000..57d1d8ce1 --- /dev/null +++ b/app/basicauth/BasicAuthRequestHandler.java @@ -0,0 +1,65 @@ +package basicauth; + +import play.Logger; +import play.http.DefaultHttpRequestHandler; +import play.libs.F; +import play.mvc.Action; +import play.mvc.Http; +import play.mvc.Result; + +import javax.annotation.Nullable; +import javax.inject.Inject; +import java.lang.reflect.Method; +import java.util.Optional; + +import static play.mvc.Http.HeaderNames.AUTHORIZATION; +import static play.mvc.Http.HeaderNames.WWW_AUTHENTICATE; + +/** + * Request handler that enables HTTP basic access authentication. + */ +public class BasicAuthRequestHandler extends DefaultHttpRequestHandler { + private final Optional basicAuth; + + @Inject + public BasicAuthRequestHandler(@Nullable final BasicAuth basicAuth) { + this.basicAuth = Optional.ofNullable(basicAuth); + } + + @Override + public Action createAction(final Http.Request request, final Method actionMethod) { + if (basicAuth.isPresent()) { + return authenticate(basicAuth.get()); + } else { + return super.createAction(request, actionMethod); + } + } + + private Action authenticate(final BasicAuth basicAuth) { + return new Action.Simple() { + + @Override + public F.Promise call(final Http.Context ctx) throws Throwable { + final boolean isAuthorized; + final String authorizationHeader = ctx.request().getHeader(AUTHORIZATION); + if (authorizationHeader != null) { + isAuthorized = basicAuth.isAuthorized(authorizationHeader); + } else { + isAuthorized = false; + ctx.response().setHeader(WWW_AUTHENTICATE, "Basic realm=\"" + basicAuth.getRealm() + "\""); + } + return authenticationResult(ctx, isAuthorized); + } + + private F.Promise authenticationResult(final Http.Context ctx, final boolean isAuthorized) throws Throwable { + if (isAuthorized) { + Logger.debug("Authorized"); + return delegate.call(ctx); + } else { + Logger.debug("Unauthorized"); + return F.Promise.pure(unauthorized()); + } + } + }; + } +} diff --git a/conf/application.conf b/conf/application.conf index 39eb6d524..fb06cd427 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -11,10 +11,18 @@ play.crypto.secret=${?APPLICATION_SECRET} # Injection configuration # ~~~~~ play.application.loader = "inject.SunriseApplicationLoader" -play.modules.enabled += "inject.CtpClientProductionModule" +play.modules.enabled += "ctpclient.CtpClientProductionModule" play.modules.enabled += "inject.CtpModelsProductionModule" play.modules.enabled += "inject.ApplicationProductionModule" +# Basic auth +# ~~~~~ +play.http.requestHandler = "basicauth.BasicAuthRequestHandler" # best to comment if you won't need basic auth + +application.auth.realm = "Sunrise Authentication" +application.auth.credentials = "username:password" # uncomment to enable HTTP basic access authentication with the given credentials + + # The application configuration # ~~~~~ application.metrics.enabled = true diff --git a/test/basicauth/BasicAuthTest.java b/test/basicauth/BasicAuthTest.java new file mode 100644 index 000000000..581b93d93 --- /dev/null +++ b/test/basicauth/BasicAuthTest.java @@ -0,0 +1,32 @@ +package basicauth; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class BasicAuthTest { + private static final String MY_REALM = "My Realm"; + private static final String CREDENTIALS = "username:password"; + private static final BasicAuth BASIC_AUTH = BasicAuth.of(MY_REALM, CREDENTIALS); + private static final String ENCODED_CREDENTIALS = "dXNlcm5hbWU6cGFzc3dvcmQ="; + + @Test + public void isAuthorizedWithCorrectCredentials() throws Exception { + assertThat(BASIC_AUTH.isAuthorized("Basic " + ENCODED_CREDENTIALS)).isTrue(); + } + + @Test + public void isNotAuthorizedWithDecodedCredentials() throws Exception { + assertThat(BASIC_AUTH.isAuthorized("Basic " + CREDENTIALS)).isFalse(); + } + + @Test + public void isNotAuthorizedWithInvalidCredentials() throws Exception { + assertThat(BASIC_AUTH.isAuthorized("Basic wrong:password")).isFalse(); + } + + @Test + public void isNotAuthorizedWithoutBasicHeader() throws Exception { + assertThat(BASIC_AUTH.isAuthorized(ENCODED_CREDENTIALS)).isFalse(); + } +} From b9be471bd8254c265f2a60e99178aa3c2c4ead62 Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Thu, 4 Feb 2016 16:25:10 +0100 Subject: [PATCH 04/11] Refactor test modules structure to enable override --- .../controllers/WithSunriseApplication.java | 126 ------------------ pt/basicauth/BasicAuthTestModule.java | 20 +++ pt/categorytree/CategoryTreeTestModule.java | 20 +++ pt/controllers/ApplicationControllerTest.java | 6 +- pt/controllers/StatusControllerTest.java | 18 ++- pt/controllers/WithSunriseApplication.java | 89 +++++++++++++ pt/ctpclient/CtpClientTestModule.java | 19 +++ pt/inject/ApplicationTestModule.java | 31 +++++ pt/inject/CtpModelsTestModule.java | 41 ++++++ pt/reverserouter/ReverseRouterTestModule.java | 17 +++ 10 files changed, 249 insertions(+), 138 deletions(-) delete mode 100644 common/test/common/controllers/WithSunriseApplication.java create mode 100644 pt/basicauth/BasicAuthTestModule.java create mode 100644 pt/categorytree/CategoryTreeTestModule.java create mode 100644 pt/controllers/WithSunriseApplication.java create mode 100644 pt/ctpclient/CtpClientTestModule.java create mode 100644 pt/inject/ApplicationTestModule.java create mode 100644 pt/inject/CtpModelsTestModule.java create mode 100644 pt/reverserouter/ReverseRouterTestModule.java diff --git a/common/test/common/controllers/WithSunriseApplication.java b/common/test/common/controllers/WithSunriseApplication.java deleted file mode 100644 index 870855e65..000000000 --- a/common/test/common/controllers/WithSunriseApplication.java +++ /dev/null @@ -1,126 +0,0 @@ -package common.controllers; - -import com.neovisionaries.i18n.CountryCode; -import common.cms.CmsService; -import common.contexts.ProjectContext; -import common.i18n.I18nResolver; -import common.models.ProductDataConfig; -import common.templates.TemplateService; -import io.sphere.sdk.categories.Category; -import io.sphere.sdk.categories.CategoryTreeExtended; -import io.sphere.sdk.categories.queries.CategoryQuery; -import io.sphere.sdk.client.PlayJavaSphereClient; -import io.sphere.sdk.client.SphereClient; -import io.sphere.sdk.producttypes.MetaProductType; -import io.sphere.sdk.producttypes.ProductType; -import io.sphere.sdk.producttypes.queries.ProductTypeQuery; -import io.sphere.sdk.queries.PagedQueryResult; -import org.apache.commons.lang3.RandomStringUtils; -import play.Application; -import play.Configuration; -import play.Environment; -import play.Mode; -import play.inject.guice.GuiceApplicationBuilder; -import play.libs.F; -import play.mvc.Controller; -import play.mvc.Http; - -import javax.money.Monetary; -import java.util.*; - -import static common.JsonUtils.readCtpObject; -import static java.util.Arrays.asList; -import static play.inject.Bindings.bind; -import static play.test.Helpers.running; - -public abstract class WithSunriseApplication { - public static final int ALLOWED_TIMEOUT = 1000; - - @FunctionalInterface - public interface CheckedConsumer { - - void apply(T t) throws Exception; - } - - protected final void run(final Application app, final Class controllerClass, - final CheckedConsumer test) { - running(app, () -> { - try { - test.apply(app.injector().instanceOf(controllerClass)); - } catch (Throwable t) { - throw new RuntimeException(t); - } - }); - } - - protected final void setContext(final Http.Request request) { - Http.Context.current.set(new Http.Context(request)); - } - - protected GuiceApplicationBuilder applicationBuilder(final SphereClient sphereClient) { - final PlayJavaSphereClient playJavaSphereClient = PlayJavaSphereClient.of(sphereClient); - return new GuiceApplicationBuilder() - .in(Environment.simple()) - .loadConfig(injectedConfiguration(baseConfiguration())) - .overrides(bind(SphereClient.class).toInstance(sphereClient)) - .overrides(bind(PlayJavaSphereClient.class).toInstance(playJavaSphereClient)) - .overrides(bind(I18nResolver.class).toInstance(injectedI18nResolver())) - .overrides(bind(TemplateService.class).toInstance(injectedTemplateService())) - .overrides(bind(CmsService.class).toInstance(injectedCmsService())) - .overrides(bind(ReverseRouter.class).toInstance(injectedReverseRouter())) - .overrides(bind(ProjectContext.class).toInstance(injectedProjectContext())) - .overrides(bind(CategoryTreeExtended.class).toInstance(injectedCategoryTree())) - .overrides(bind(ProductDataConfig.class).toInstance(injectedProductDataConfig())); - } - - protected Http.RequestBuilder requestBuilder() { - return new Http.RequestBuilder().id(1L); - } - - protected Configuration baseConfiguration() { - final Map additionalSettings = new HashMap<>(); - additionalSettings.put("application.metrics.enabled", false); - additionalSettings.put("application.settingsWidget.enabled", false); - additionalSettings.put("play.crypto.secret", RandomStringUtils.randomAlphanumeric(15)); - return new Configuration(additionalSettings); - } - - protected Configuration injectedConfiguration(final Configuration configuration) { - final Configuration testConfiguration = Configuration.load(new Environment(Mode.TEST)); - return configuration.withFallback(testConfiguration); - } - - private I18nResolver injectedI18nResolver() { - return ((locale, bundle, key, args) -> Optional.empty()); - } - - protected TemplateService injectedTemplateService() { - return ((templateName, pageData, locales) -> ""); - } - - protected CmsService injectedCmsService() { - return ((locale, pageKey) -> F.Promise.pure((messageKey, args) -> Optional.empty())); - } - - protected ReverseRouter injectedReverseRouter() { - return new TestableReverseRouter(); - } - - protected ProjectContext injectedProjectContext() { - return ProjectContext.of( - asList(Locale.ENGLISH, Locale.GERMAN), - asList(CountryCode.US, CountryCode.DE), - asList(Monetary.getCurrency("USD"), Monetary.getCurrency("EUR"))); - } - - protected CategoryTreeExtended injectedCategoryTree() { - final PagedQueryResult result = readCtpObject("data/categories.json", CategoryQuery.resultTypeReference()); - return CategoryTreeExtended.of(result.getResults()); - } - - protected ProductDataConfig injectedProductDataConfig() { - final PagedQueryResult result = readCtpObject("data/product-types.json", ProductTypeQuery.resultTypeReference()); - final MetaProductType metaProductType = MetaProductType.of(result.getResults()); - return ProductDataConfig.of(metaProductType, asList("foo", "bar")); - } -} \ No newline at end of file diff --git a/pt/basicauth/BasicAuthTestModule.java b/pt/basicauth/BasicAuthTestModule.java new file mode 100644 index 000000000..c4f7055bf --- /dev/null +++ b/pt/basicauth/BasicAuthTestModule.java @@ -0,0 +1,20 @@ +package basicauth; + +import com.google.inject.AbstractModule; +import com.google.inject.util.Providers; + +import javax.annotation.Nullable; + +public class BasicAuthTestModule extends AbstractModule { + @Nullable + private final BasicAuth basicAuth; + + public BasicAuthTestModule(@Nullable final BasicAuth basicAuth) { + this.basicAuth = basicAuth; + } + + @Override + protected void configure() { + bind(BasicAuth.class).toProvider(Providers.of(basicAuth)); + } +} \ No newline at end of file diff --git a/pt/categorytree/CategoryTreeTestModule.java b/pt/categorytree/CategoryTreeTestModule.java new file mode 100644 index 000000000..dd26b81bd --- /dev/null +++ b/pt/categorytree/CategoryTreeTestModule.java @@ -0,0 +1,20 @@ +package categorytree; + +import com.google.inject.AbstractModule; +import io.sphere.sdk.categories.Category; +import io.sphere.sdk.categories.CategoryTreeExtended; + +import java.util.List; + +public class CategoryTreeTestModule extends AbstractModule { + private final List categories; + + public CategoryTreeTestModule(final List categories) { + this.categories = categories; + } + + @Override + protected void configure() { + bind(CategoryTreeExtended.class).toInstance(CategoryTreeExtended.of(categories)); + } +} diff --git a/pt/controllers/ApplicationControllerTest.java b/pt/controllers/ApplicationControllerTest.java index 24c151e14..952f4fb3e 100644 --- a/pt/controllers/ApplicationControllerTest.java +++ b/pt/controllers/ApplicationControllerTest.java @@ -1,9 +1,6 @@ package controllers; -import common.controllers.TestableSphereClient; -import common.controllers.WithSunriseApplication; import org.junit.Test; -import play.Application; import play.mvc.Http; import play.mvc.Result; import productcatalog.home.HomeController; @@ -16,8 +13,7 @@ public class ApplicationControllerTest extends WithSunriseApplication { @Test public void homeIsAlive() { setContext(requestBuilder().build()); - final Application app = applicationBuilder(TestableSphereClient.ofEmptyResponse()).build(); - run(app, HomeController.class, controller -> { + run(app(), HomeController.class, controller -> { final Result result = controller.show("en").get(ALLOWED_TIMEOUT); assertThat(result.status()).isEqualTo(Http.Status.OK); assertThat(contentAsString(result)).isNullOrEmpty(); diff --git a/pt/controllers/StatusControllerTest.java b/pt/controllers/StatusControllerTest.java index 762831b56..aa525f0bb 100644 --- a/pt/controllers/StatusControllerTest.java +++ b/pt/controllers/StatusControllerTest.java @@ -1,7 +1,8 @@ package controllers; import common.controllers.TestableSphereClient; -import common.controllers.WithSunriseApplication; +import ctpclient.CtpClientTestModule; +import io.sphere.sdk.client.SphereClient; import io.sphere.sdk.products.ProductProjection; import io.sphere.sdk.products.search.ProductProjectionSearch; import io.sphere.sdk.search.PagedSearchResult; @@ -21,33 +22,32 @@ public class StatusControllerTest extends WithSunriseApplication { @Test public void showsHealthyCtp() throws Exception { final PagedSearchResult result = readCtpObject("data/products-search.json", ProductProjectionSearch.resultTypeReference()); - final Application app = applicationBuilder(TestableSphereClient.ofResponse(result)).build(); + final Application app = app(TestableSphereClient.ofResponse(result)); run(app, StatusController.class, this::rendersHealthyCtp); } @Test public void showsEmptyCtp() throws Exception { final PagedSearchResult result = readCtpObject("data/empty-products-search.json", ProductProjectionSearch.resultTypeReference()); - final Application app = applicationBuilder(TestableSphereClient.ofResponse(result)).build(); + final Application app = app(TestableSphereClient.ofResponse(result)); run(app, StatusController.class, this::rendersUnhealthyCtp); } @Test public void showsUnhealthyCtp() throws Exception { - final Application app = applicationBuilder(TestableSphereClient.ofUnhealthyCtp()).build(); + final Application app = app(TestableSphereClient.ofUnhealthyCtp()); run(app, StatusController.class, this::rendersUnhealthyCtp); } @Test public void showsUnresponsiveCtp() throws Exception { - final Application app = applicationBuilder(TestableSphereClient.ofUnresponsiveCtp()).build(); + final Application app = app(TestableSphereClient.ofUnresponsiveCtp()); run(app, StatusController.class, this::rendersUnhealthyCtp); } @Test public void showsVersion() throws Exception { - final Application app = applicationBuilder(TestableSphereClient.ofEmptyResponse()).build(); - run(app, StatusController.class, controller -> { + run(app(), StatusController.class, controller -> { final Result result = controller.version(); assertThat(result.status()).isEqualTo(Http.Status.OK); assertThat(result.contentType()).isEqualTo(Http.MimeTypes.JSON); @@ -55,6 +55,10 @@ public void showsVersion() throws Exception { }); } + private Application app(final SphereClient sphereClient) { + return app(new CtpClientTestModule(sphereClient)); + } + private void rendersHealthyCtp(final StatusController controller) throws IOException { final Result result = controller.health().get(ALLOWED_TIMEOUT); assertThat(result.status()).isEqualTo(Http.Status.OK); diff --git a/pt/controllers/WithSunriseApplication.java b/pt/controllers/WithSunriseApplication.java new file mode 100644 index 000000000..3f65c7e37 --- /dev/null +++ b/pt/controllers/WithSunriseApplication.java @@ -0,0 +1,89 @@ +package controllers; + +import basicauth.BasicAuthTestModule; +import categorytree.CategoryTreeTestModule; +import com.google.inject.Module; +import com.google.inject.util.Modules; +import common.controllers.TestableReverseRouter; +import common.controllers.TestableSphereClient; +import ctpclient.CtpClientTestModule; +import inject.ApplicationTestModule; +import inject.CtpModelsTestModule; +import io.sphere.sdk.categories.Category; +import io.sphere.sdk.categories.queries.CategoryQuery; +import org.apache.commons.lang3.RandomStringUtils; +import play.Application; +import play.Configuration; +import play.Environment; +import play.Mode; +import play.inject.guice.GuiceApplicationBuilder; +import play.mvc.Controller; +import play.mvc.Http; +import reverserouter.ReverseRouterTestModule; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static common.JsonUtils.readCtpObject; +import static play.test.Helpers.running; + +public abstract class WithSunriseApplication { + public static final int ALLOWED_TIMEOUT = 1000; + + @FunctionalInterface + public interface CheckedConsumer { + + void apply(T t) throws Exception; + } + + protected final void run(final Application app, final Class controllerClass, + final CheckedConsumer test) { + running(app, () -> { + try { + test.apply(app.injector().instanceOf(controllerClass)); + } catch (Throwable t) { + throw new RuntimeException(t); + } + }); + } + + protected final void setContext(final Http.Request request) { + Http.Context.current.set(new Http.Context(request)); + } + + protected Http.RequestBuilder requestBuilder() { + return new Http.RequestBuilder().id(1L); + } + + protected Application app(final Module ... modules) { + final Module testModule = Modules.override(testModule()).with(modules); + return appBuilder().overrides(testModule).build(); + } + + protected GuiceApplicationBuilder appBuilder() { + return new GuiceApplicationBuilder() + .in(Environment.simple()) + .loadConfig(testConfiguration()); + } + + protected Module testModule() { + final List categories = readCtpObject("data/categories.json", CategoryQuery.resultTypeReference()).getResults(); + return Modules.combine( + new CtpClientTestModule(TestableSphereClient.ofEmptyResponse()), + new ApplicationTestModule(), + new CtpModelsTestModule(), + new BasicAuthTestModule(null), + new ReverseRouterTestModule(new TestableReverseRouter()), + new CategoryTreeTestModule(categories)); + } + + protected Configuration testConfiguration() { + final Configuration configuration = Configuration.load(new Environment(Mode.TEST)); + final Map additionalSettings = new HashMap<>(); + additionalSettings.put("application.metrics.enabled", false); + additionalSettings.put("application.settingsWidget.enabled", false); + additionalSettings.put("play.crypto.secret", RandomStringUtils.randomAlphanumeric(5)); + return new Configuration(additionalSettings).withFallback(configuration); + } +} \ No newline at end of file diff --git a/pt/ctpclient/CtpClientTestModule.java b/pt/ctpclient/CtpClientTestModule.java new file mode 100644 index 000000000..befe10393 --- /dev/null +++ b/pt/ctpclient/CtpClientTestModule.java @@ -0,0 +1,19 @@ +package ctpclient; + +import com.google.inject.AbstractModule; +import io.sphere.sdk.client.PlayJavaSphereClient; +import io.sphere.sdk.client.SphereClient; + +public class CtpClientTestModule extends AbstractModule { + private final SphereClient sphereClient; + + public CtpClientTestModule(final SphereClient sphereClient) { + this.sphereClient = sphereClient; + } + + @Override + protected void configure() { + bind(SphereClient.class).toInstance(sphereClient); + bind(PlayJavaSphereClient.class).toInstance(PlayJavaSphereClient.of(sphereClient)); + } +} \ No newline at end of file diff --git a/pt/inject/ApplicationTestModule.java b/pt/inject/ApplicationTestModule.java new file mode 100644 index 000000000..84a9f9944 --- /dev/null +++ b/pt/inject/ApplicationTestModule.java @@ -0,0 +1,31 @@ +package inject; + +import com.google.inject.AbstractModule; +import common.cms.CmsService; +import common.i18n.I18nResolver; +import common.templates.TemplateService; +import play.libs.F; + +import java.util.Optional; + +public class ApplicationTestModule extends AbstractModule { + + @Override + protected void configure() { + bind(I18nResolver.class).toInstance(injectedI18nResolver()); + bind(TemplateService.class).toInstance(injectedTemplateService()); + bind(CmsService.class).toInstance(injectedCmsService()); + } + + private I18nResolver injectedI18nResolver() { + return ((locale, bundle, key, args) -> Optional.empty()); + } + + protected TemplateService injectedTemplateService() { + return ((templateName, pageData, locales) -> ""); + } + + protected CmsService injectedCmsService() { + return ((locale, pageKey) -> F.Promise.pure((messageKey, args) -> Optional.empty())); + } +} diff --git a/pt/inject/CtpModelsTestModule.java b/pt/inject/CtpModelsTestModule.java new file mode 100644 index 000000000..5e855234c --- /dev/null +++ b/pt/inject/CtpModelsTestModule.java @@ -0,0 +1,41 @@ +package inject; + +import com.google.inject.AbstractModule; +import com.neovisionaries.i18n.CountryCode; +import common.contexts.ProjectContext; +import common.models.ProductDataConfig; +import io.sphere.sdk.producttypes.MetaProductType; +import io.sphere.sdk.producttypes.ProductType; +import io.sphere.sdk.producttypes.queries.ProductTypeQuery; +import io.sphere.sdk.queries.PagedQueryResult; +import shoppingcart.checkout.shipping.ShippingMethods; + +import javax.money.Monetary; +import java.util.Collections; +import java.util.Locale; + +import static common.JsonUtils.readCtpObject; +import static java.util.Arrays.asList; + +public class CtpModelsTestModule extends AbstractModule { + + @Override + protected void configure() { + bind(ProjectContext.class).toInstance(injectedProjectContext()); + bind(ProductDataConfig.class).toInstance(injectedProductDataConfig()); + bind(ShippingMethods.class).toInstance(new ShippingMethods(Collections.emptyList())); + } + + protected ProjectContext injectedProjectContext() { + return ProjectContext.of( + asList(Locale.ENGLISH, Locale.GERMAN), + asList(CountryCode.US, CountryCode.DE), + asList(Monetary.getCurrency("USD"), Monetary.getCurrency("EUR"))); + } + + protected ProductDataConfig injectedProductDataConfig() { + final PagedQueryResult result = readCtpObject("data/product-types.json", ProductTypeQuery.resultTypeReference()); + final MetaProductType metaProductType = MetaProductType.of(result.getResults()); + return ProductDataConfig.of(metaProductType, asList("foo", "bar")); + } +} \ No newline at end of file diff --git a/pt/reverserouter/ReverseRouterTestModule.java b/pt/reverserouter/ReverseRouterTestModule.java new file mode 100644 index 000000000..0807d05ef --- /dev/null +++ b/pt/reverserouter/ReverseRouterTestModule.java @@ -0,0 +1,17 @@ +package reverserouter; + +import com.google.inject.AbstractModule; +import common.controllers.ReverseRouter; + +public class ReverseRouterTestModule extends AbstractModule { + private final ReverseRouter reverseRouter; + + public ReverseRouterTestModule(final ReverseRouter reverseRouter) { + this.reverseRouter = reverseRouter; + } + + @Override + protected void configure() { + bind(ReverseRouter.class).toInstance(reverseRouter); + } +} From 15d44310b8f2f9492c5416872f8fdecf8a483d82 Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Thu, 4 Feb 2016 16:25:32 +0100 Subject: [PATCH 05/11] Test basic auth process --- app/basicauth/BasicAuthProvider.java | 1 - conf/application.conf | 3 ++ pt/basicauth/BasicAuthRequestHandlerTest.java | 41 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 pt/basicauth/BasicAuthRequestHandlerTest.java diff --git a/app/basicauth/BasicAuthProvider.java b/app/basicauth/BasicAuthProvider.java index 91fd2b385..d322fd9bc 100644 --- a/app/basicauth/BasicAuthProvider.java +++ b/app/basicauth/BasicAuthProvider.java @@ -25,7 +25,6 @@ public BasicAuth get() { final String realm = configuration.getString(CONFIG_REALM, "Sunrise Authentication"); final String credentials = configuration.getString(CONFIG_CREDENTIALS); if (credentials != null && !credentials.isEmpty()) { - System.out.println(credentials); if (credentials.matches(REGEX_CREDENTIALS)) { Logger.debug("Basic authentication: enabled for realm \"{}\"", realm); return BasicAuth.of(realm, credentials); diff --git a/conf/application.conf b/conf/application.conf index fb06cd427..128629368 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -11,9 +11,12 @@ play.crypto.secret=${?APPLICATION_SECRET} # Injection configuration # ~~~~~ play.application.loader = "inject.SunriseApplicationLoader" +play.modules.enabled += "basicauth.BasicAuthProductionModule" play.modules.enabled += "ctpclient.CtpClientProductionModule" play.modules.enabled += "inject.CtpModelsProductionModule" play.modules.enabled += "inject.ApplicationProductionModule" +play.modules.enabled += "categorytree.CategoryTreeProductionModule" +play.modules.enabled += "reverserouter.ReverseRouterProductionModule" # Basic auth # ~~~~~ diff --git a/pt/basicauth/BasicAuthRequestHandlerTest.java b/pt/basicauth/BasicAuthRequestHandlerTest.java new file mode 100644 index 000000000..3deef3a00 --- /dev/null +++ b/pt/basicauth/BasicAuthRequestHandlerTest.java @@ -0,0 +1,41 @@ +package basicauth; + +import controllers.ApplicationController; +import controllers.WithSunriseApplication; +import org.junit.Ignore; +import org.junit.Test; +import play.Application; +import play.mvc.Http; +import play.mvc.Result; + +import static org.assertj.core.api.Assertions.assertThat; + +public class BasicAuthRequestHandlerTest extends WithSunriseApplication { + + @Test + public void allowsAccessWhenBasicAuthDisabled() throws Exception { + final Application app = appWithoutBasicAuth(); + run(app, ApplicationController.class, controller -> { + final Result result = controller.index(); + assertThat(result.status()).isLessThan(Http.Status.BAD_REQUEST); + }); + } + + @Ignore + @Test + public void unauthorizedWhenBasicAuthEnabled() throws Exception { + final Application app = appWithBasicAuth(BasicAuth.of("My Realm", "username:password")); + run(app, ApplicationController.class, controller -> { + final Result result = controller.index(); + assertThat(result.status()).isEqualTo(Http.Status.UNAUTHORIZED); + }); + } + + private Application appWithoutBasicAuth() { + return appWithBasicAuth(null); + } + + private Application appWithBasicAuth(final BasicAuth basicAuth) { + return app(new BasicAuthTestModule(basicAuth)); + } +} From 8c4c1dbfc194339659b3a03b5165dca9db1d409c Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Thu, 4 Feb 2016 18:09:03 +0100 Subject: [PATCH 06/11] Enable tests for basic auth --- app/basicauth/BasicAuth.java | 4 +- build.sbt | 3 +- conf/application.conf | 5 +- pt/basicauth/BasicAuthRequestHandlerTest.java | 52 +++++++++++++------ pt/controllers/WithSunriseApplication.java | 15 +++++- 5 files changed, 59 insertions(+), 20 deletions(-) diff --git a/app/basicauth/BasicAuth.java b/app/basicauth/BasicAuth.java index 44f84b2f9..b4da44914 100644 --- a/app/basicauth/BasicAuth.java +++ b/app/basicauth/BasicAuth.java @@ -1,11 +1,13 @@ package basicauth; +import io.sphere.sdk.models.Base; + import java.util.Base64; /** * Contains information related to the HTTP basic access authentication. */ -public class BasicAuth { +public class BasicAuth extends Base { private final String realm; private final String encodedCredentials; diff --git a/build.sbt b/build.sbt index ef0dbd34d..f3d69adf9 100644 --- a/build.sbt +++ b/build.sbt @@ -64,12 +64,12 @@ lazy val commonSettings = testSettings ++ releaseSettings ++ Seq ( Resolver.mavenLocal ), libraryDependencies ++= Seq ( + filters, "io.commercetools" % "commercetools-sunrise-design" % sunriseDesignVersion, "io.sphere.sdk.jvm" % "sphere-models" % sphereJvmSdkVersion, "io.sphere.sdk.jvm" % "sphere-play-2_4-java-client_2.10" % sphereJvmSdkVersion, "org.webjars" % "webjars-play_2.10" % "2.4.0-1", "com.github.jknack" % "handlebars" % "4.0.3", - filters, "commons-beanutils" % "commons-beanutils" % "1.9.2", "commons-io" % "commons-io" % "2.4" ), @@ -111,6 +111,7 @@ lazy val testScopes = "test,it,pt" lazy val testSettings = Defaults.itSettings ++ inConfig(PlayTest)(Defaults.testSettings) ++ testDirConfigs(IntegrationTest, "it") ++ testDirConfigs(PlayTest, "pt") ++ Seq ( testOptions += Tests.Argument(TestFrameworks.JUnit, "-v"), libraryDependencies ++= Seq ( + javaWs % "pt", "org.assertj" % "assertj-core" % "3.0.0" % testScopes, PlayImport.component("play-test") % "it,pt" ), diff --git a/conf/application.conf b/conf/application.conf index 128629368..e65742249 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -10,7 +10,9 @@ play.crypto.secret=${?APPLICATION_SECRET} # Injection configuration # ~~~~~ +play.http.requestHandler = "basicauth.BasicAuthRequestHandler" play.application.loader = "inject.SunriseApplicationLoader" + play.modules.enabled += "basicauth.BasicAuthProductionModule" play.modules.enabled += "ctpclient.CtpClientProductionModule" play.modules.enabled += "inject.CtpModelsProductionModule" @@ -20,10 +22,9 @@ play.modules.enabled += "reverserouter.ReverseRouterProductionModule" # Basic auth # ~~~~~ -play.http.requestHandler = "basicauth.BasicAuthRequestHandler" # best to comment if you won't need basic auth application.auth.realm = "Sunrise Authentication" -application.auth.credentials = "username:password" # uncomment to enable HTTP basic access authentication with the given credentials +#application.auth.credentials = "username:password" # uncomment to enable HTTP basic access authentication with the given credentials # The application configuration diff --git a/pt/basicauth/BasicAuthRequestHandlerTest.java b/pt/basicauth/BasicAuthRequestHandlerTest.java index 3deef3a00..47621a04a 100644 --- a/pt/basicauth/BasicAuthRequestHandlerTest.java +++ b/pt/basicauth/BasicAuthRequestHandlerTest.java @@ -1,33 +1,53 @@ package basicauth; -import controllers.ApplicationController; +import common.controllers.TestableReverseRouter; import controllers.WithSunriseApplication; -import org.junit.Ignore; import org.junit.Test; import play.Application; +import play.libs.ws.WSAuthScheme; +import play.libs.ws.WSRequest; +import play.libs.ws.WSResponse; import play.mvc.Http; -import play.mvc.Result; +import reverserouter.ReverseRouterTestModule; import static org.assertj.core.api.Assertions.assertThat; public class BasicAuthRequestHandlerTest extends WithSunriseApplication { @Test - public void allowsAccessWhenBasicAuthDisabled() throws Exception { - final Application app = appWithoutBasicAuth(); - run(app, ApplicationController.class, controller -> { - final Result result = controller.index(); - assertThat(result.status()).isLessThan(Http.Status.BAD_REQUEST); + public void allowsAccessWhenDisabled() throws Exception { + run(appWithoutBasicAuth(), "/", request -> { + final WSResponse response = request.get().get(ALLOWED_TIMEOUT); + assertThat(response.getStatus()).isEqualTo(Http.Status.OK); }); } - @Ignore @Test - public void unauthorizedWhenBasicAuthEnabled() throws Exception { - final Application app = appWithBasicAuth(BasicAuth.of("My Realm", "username:password")); - run(app, ApplicationController.class, controller -> { - final Result result = controller.index(); - assertThat(result.status()).isEqualTo(Http.Status.UNAUTHORIZED); + public void unauthorizedWhenEnabled() throws Exception { + final BasicAuth basicAuth = BasicAuth.of("My Realm", "username:password"); + run(appWithBasicAuth(basicAuth), "/", request -> { + final WSResponse response = request.get().get(ALLOWED_TIMEOUT); + assertThat(response.getStatus()).isEqualTo(Http.Status.UNAUTHORIZED); + }); + } + + @Test + public void authorizedWhenEnabledAndCredentialsProvided() throws Exception { + final BasicAuth basicAuth = BasicAuth.of("My Realm", "username:password"); + run(appWithBasicAuth(basicAuth), "/", request -> { + final WSRequest authenticatedRequest = request.setAuth("username", "password", WSAuthScheme.BASIC); + final WSResponse response = authenticatedRequest.get().get(ALLOWED_TIMEOUT); + assertThat(response.getStatus()).isEqualTo(Http.Status.OK); + }); + } + + @Test + public void unauthorizedWhenEnabledAndWrongCredentialsProvided() throws Exception { + final BasicAuth basicAuth = BasicAuth.of("My Realm", "username:password"); + run(appWithBasicAuth(basicAuth), "/", request -> { + final WSRequest authenticatedRequest = request.setAuth("username", "wrong", WSAuthScheme.BASIC); + final WSResponse response = authenticatedRequest.get().get(ALLOWED_TIMEOUT); + assertThat(response.getStatus()).isEqualTo(Http.Status.UNAUTHORIZED); }); } @@ -36,6 +56,8 @@ private Application appWithoutBasicAuth() { } private Application appWithBasicAuth(final BasicAuth basicAuth) { - return app(new BasicAuthTestModule(basicAuth)); + final TestableReverseRouter reverseRouter = new TestableReverseRouter(); + reverseRouter.setHomeUrl("/en/home"); + return app(new BasicAuthTestModule(basicAuth), new ReverseRouterTestModule(reverseRouter)); } } diff --git a/pt/controllers/WithSunriseApplication.java b/pt/controllers/WithSunriseApplication.java index 3f65c7e37..7fd1ec3fc 100644 --- a/pt/controllers/WithSunriseApplication.java +++ b/pt/controllers/WithSunriseApplication.java @@ -17,6 +17,8 @@ import play.Environment; import play.Mode; import play.inject.guice.GuiceApplicationBuilder; +import play.libs.ws.WS; +import play.libs.ws.WSRequest; import play.mvc.Controller; import play.mvc.Http; import reverserouter.ReverseRouterTestModule; @@ -27,9 +29,10 @@ import static common.JsonUtils.readCtpObject; import static play.test.Helpers.running; +import static play.test.Helpers.testServer; public abstract class WithSunriseApplication { - public static final int ALLOWED_TIMEOUT = 1000; + public static final int ALLOWED_TIMEOUT = 100000; @FunctionalInterface public interface CheckedConsumer { @@ -37,6 +40,16 @@ public interface CheckedConsumer { void apply(T t) throws Exception; } + protected final void run(final Application app, final String url, final CheckedConsumer test) { + running(testServer(3333, app), () -> { + try { + test.apply(WS.url("http://localhost:3333" + url)); + } catch (Throwable t) { + throw new RuntimeException(t); + } + }); + } + protected final void run(final Application app, final Class controllerClass, final CheckedConsumer test) { running(app, () -> { From f6754c017627796bdab5c0f9b7ba6f7ba2edfef7 Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Thu, 4 Feb 2016 18:21:16 +0100 Subject: [PATCH 07/11] Add configurable vars for basic auth --- app.json | 7 +++++++ conf/application.conf | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app.json b/app.json index 892764e83..9d1656315 100644 --- a/app.json +++ b/app.json @@ -19,6 +19,13 @@ "description": "The product attributes that allow to identify and select a product variant in your project. Enter the attribute keys separated by commas." }, + "BASIC_AUTH_CREDENTIALS": { + "description": "Leave empty if you do not want to enable basic access authentication! If you only want authorized users to access your website, please enter here the required credentials separated by colon ':'. For example, for a username 'john' and password 'secret', enter 'john:secret'" + }, + "BASIC_AUTH_REALM": { + "description": "Authentication realm that identifies your application. This is only necessary when basic authentication is enabled.", + "value": "Sunrise Demo" + }, "APPLICATION_SECRET": { "description": "The secret key is used to secure cryptographics functions. Anyone that can get access to the application secret will be able to generate any session they please, effectively allowing them to log in to your system as any user they please. If you deploy your application to several instances be sure to use the same key.", "generator": "secret" diff --git a/conf/application.conf b/conf/application.conf index e65742249..84bf2aa67 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -22,9 +22,11 @@ play.modules.enabled += "reverserouter.ReverseRouterProductionModule" # Basic auth # ~~~~~ +application.auth.realm = "Sunrise Demo" +application.auth.realm = ${?BASIC_AUTH_REALM} -application.auth.realm = "Sunrise Authentication" #application.auth.credentials = "username:password" # uncomment to enable HTTP basic access authentication with the given credentials +application.auth.credentials = ${?BASIC_AUTH_CREDENTIALS} # The application configuration From 4dbd26c40de713f036d874570fc13bf463d2d9ed Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Fri, 5 Feb 2016 10:59:00 +0100 Subject: [PATCH 08/11] Catch Exception instead of Throwable --- pt/controllers/WithSunriseApplication.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pt/controllers/WithSunriseApplication.java b/pt/controllers/WithSunriseApplication.java index 7fd1ec3fc..c599ca562 100644 --- a/pt/controllers/WithSunriseApplication.java +++ b/pt/controllers/WithSunriseApplication.java @@ -41,11 +41,12 @@ public interface CheckedConsumer { } protected final void run(final Application app, final String url, final CheckedConsumer test) { - running(testServer(3333, app), () -> { + final int port = 3333; + running(testServer(port, app), () -> { try { - test.apply(WS.url("http://localhost:3333" + url)); - } catch (Throwable t) { - throw new RuntimeException(t); + test.apply(WS.url("http://localhost:" + port + url)); + } catch (Exception e) { + throw new RuntimeException(e); } }); } @@ -55,8 +56,8 @@ protected final void run(final Application app, final Cla running(app, () -> { try { test.apply(app.injector().instanceOf(controllerClass)); - } catch (Throwable t) { - throw new RuntimeException(t); + } catch (Exception e) { + throw new RuntimeException(e); } }); } From 1d02ce28a79582852f9ca6cbc946c6a3b36a697f Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Fri, 5 Feb 2016 11:23:42 +0100 Subject: [PATCH 09/11] Improve tests readability --- pt/basicauth/BasicAuthRequestHandlerTest.java | 20 +++++++++++-------- test/basicauth/BasicAuthTest.java | 13 ++++++++---- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/pt/basicauth/BasicAuthRequestHandlerTest.java b/pt/basicauth/BasicAuthRequestHandlerTest.java index 47621a04a..04b4b32d5 100644 --- a/pt/basicauth/BasicAuthRequestHandlerTest.java +++ b/pt/basicauth/BasicAuthRequestHandlerTest.java @@ -14,6 +14,9 @@ public class BasicAuthRequestHandlerTest extends WithSunriseApplication { + public static final String USERNAME = "username"; + public static final String PASSWORD = "password"; + @Test public void allowsAccessWhenDisabled() throws Exception { run(appWithoutBasicAuth(), "/", request -> { @@ -24,8 +27,7 @@ public void allowsAccessWhenDisabled() throws Exception { @Test public void unauthorizedWhenEnabled() throws Exception { - final BasicAuth basicAuth = BasicAuth.of("My Realm", "username:password"); - run(appWithBasicAuth(basicAuth), "/", request -> { + run(appWithBasicAuth(), "/", request -> { final WSResponse response = request.get().get(ALLOWED_TIMEOUT); assertThat(response.getStatus()).isEqualTo(Http.Status.UNAUTHORIZED); }); @@ -33,9 +35,8 @@ public void unauthorizedWhenEnabled() throws Exception { @Test public void authorizedWhenEnabledAndCredentialsProvided() throws Exception { - final BasicAuth basicAuth = BasicAuth.of("My Realm", "username:password"); - run(appWithBasicAuth(basicAuth), "/", request -> { - final WSRequest authenticatedRequest = request.setAuth("username", "password", WSAuthScheme.BASIC); + run(appWithBasicAuth(), "/", request -> { + final WSRequest authenticatedRequest = request.setAuth(USERNAME, PASSWORD, WSAuthScheme.BASIC); final WSResponse response = authenticatedRequest.get().get(ALLOWED_TIMEOUT); assertThat(response.getStatus()).isEqualTo(Http.Status.OK); }); @@ -43,9 +44,8 @@ public void authorizedWhenEnabledAndCredentialsProvided() throws Exception { @Test public void unauthorizedWhenEnabledAndWrongCredentialsProvided() throws Exception { - final BasicAuth basicAuth = BasicAuth.of("My Realm", "username:password"); - run(appWithBasicAuth(basicAuth), "/", request -> { - final WSRequest authenticatedRequest = request.setAuth("username", "wrong", WSAuthScheme.BASIC); + run(appWithBasicAuth(), "/", request -> { + final WSRequest authenticatedRequest = request.setAuth(USERNAME, "wrong", WSAuthScheme.BASIC); final WSResponse response = authenticatedRequest.get().get(ALLOWED_TIMEOUT); assertThat(response.getStatus()).isEqualTo(Http.Status.UNAUTHORIZED); }); @@ -55,6 +55,10 @@ private Application appWithoutBasicAuth() { return appWithBasicAuth(null); } + private Application appWithBasicAuth() { + return appWithBasicAuth(BasicAuth.of("My Realm", USERNAME + ":" + PASSWORD)); + } + private Application appWithBasicAuth(final BasicAuth basicAuth) { final TestableReverseRouter reverseRouter = new TestableReverseRouter(); reverseRouter.setHomeUrl("/en/home"); diff --git a/test/basicauth/BasicAuthTest.java b/test/basicauth/BasicAuthTest.java index 581b93d93..55d624e6c 100644 --- a/test/basicauth/BasicAuthTest.java +++ b/test/basicauth/BasicAuthTest.java @@ -2,17 +2,18 @@ import org.junit.Test; +import java.util.Base64; + import static org.assertj.core.api.Assertions.assertThat; public class BasicAuthTest { private static final String MY_REALM = "My Realm"; private static final String CREDENTIALS = "username:password"; private static final BasicAuth BASIC_AUTH = BasicAuth.of(MY_REALM, CREDENTIALS); - private static final String ENCODED_CREDENTIALS = "dXNlcm5hbWU6cGFzc3dvcmQ="; @Test public void isAuthorizedWithCorrectCredentials() throws Exception { - assertThat(BASIC_AUTH.isAuthorized("Basic " + ENCODED_CREDENTIALS)).isTrue(); + assertThat(BASIC_AUTH.isAuthorized("Basic " + encode(CREDENTIALS))).isTrue(); } @Test @@ -22,11 +23,15 @@ public void isNotAuthorizedWithDecodedCredentials() throws Exception { @Test public void isNotAuthorizedWithInvalidCredentials() throws Exception { - assertThat(BASIC_AUTH.isAuthorized("Basic wrong:password")).isFalse(); + assertThat(BASIC_AUTH.isAuthorized("Basic " + encode("username:wrong"))).isFalse(); } @Test public void isNotAuthorizedWithoutBasicHeader() throws Exception { - assertThat(BASIC_AUTH.isAuthorized(ENCODED_CREDENTIALS)).isFalse(); + assertThat(BASIC_AUTH.isAuthorized(encode(CREDENTIALS))).isFalse(); + } + + private static String encode(final String credentials) { + return Base64.getEncoder().encodeToString(credentials.getBytes()); } } From 5dc5d2b6effaa1e2ed3ce19535b1bfeb009f549b Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Fri, 5 Feb 2016 12:15:10 +0100 Subject: [PATCH 10/11] Use static methods when possible for PT --- pt/basicauth/BasicAuthRequestHandlerTest.java | 25 +++++++++++++------ pt/controllers/StatusControllerTest.java | 8 +++--- pt/controllers/WithSunriseApplication.java | 24 ++++++++++-------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/pt/basicauth/BasicAuthRequestHandlerTest.java b/pt/basicauth/BasicAuthRequestHandlerTest.java index 04b4b32d5..d466630b3 100644 --- a/pt/basicauth/BasicAuthRequestHandlerTest.java +++ b/pt/basicauth/BasicAuthRequestHandlerTest.java @@ -4,18 +4,22 @@ import controllers.WithSunriseApplication; import org.junit.Test; import play.Application; +import play.Configuration; import play.libs.ws.WSAuthScheme; import play.libs.ws.WSRequest; import play.libs.ws.WSResponse; import play.mvc.Http; import reverserouter.ReverseRouterTestModule; +import java.util.Map; + +import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; public class BasicAuthRequestHandlerTest extends WithSunriseApplication { - - public static final String USERNAME = "username"; - public static final String PASSWORD = "password"; + private static final Configuration CONFIG = configurationWithHandleEnabled(); + private static final String USERNAME = "username"; + private static final String PASSWORD = "password"; @Test public void allowsAccessWhenDisabled() throws Exception { @@ -51,17 +55,24 @@ public void unauthorizedWhenEnabledAndWrongCredentialsProvided() throws Exceptio }); } - private Application appWithoutBasicAuth() { + private static Application appWithoutBasicAuth() { return appWithBasicAuth(null); } - private Application appWithBasicAuth() { + private static Application appWithBasicAuth() { return appWithBasicAuth(BasicAuth.of("My Realm", USERNAME + ":" + PASSWORD)); } - private Application appWithBasicAuth(final BasicAuth basicAuth) { + private static Application appWithBasicAuth(final BasicAuth basicAuth) { final TestableReverseRouter reverseRouter = new TestableReverseRouter(); reverseRouter.setHomeUrl("/en/home"); - return app(new BasicAuthTestModule(basicAuth), new ReverseRouterTestModule(reverseRouter)); + return appBuilder(new BasicAuthTestModule(basicAuth), new ReverseRouterTestModule(reverseRouter)) + .loadConfig(CONFIG) + .build(); + } + + private static Configuration configurationWithHandleEnabled() { + final Map configMap = singletonMap("play.http.requestHandler", "basicauth.BasicAuthRequestHandler"); + return new Configuration(configMap).withFallback(testConfiguration()); } } diff --git a/pt/controllers/StatusControllerTest.java b/pt/controllers/StatusControllerTest.java index aa525f0bb..f3352cf3c 100644 --- a/pt/controllers/StatusControllerTest.java +++ b/pt/controllers/StatusControllerTest.java @@ -55,10 +55,6 @@ public void showsVersion() throws Exception { }); } - private Application app(final SphereClient sphereClient) { - return app(new CtpClientTestModule(sphereClient)); - } - private void rendersHealthyCtp(final StatusController controller) throws IOException { final Result result = controller.health().get(ALLOWED_TIMEOUT); assertThat(result.status()).isEqualTo(Http.Status.OK); @@ -72,4 +68,8 @@ private void rendersUnhealthyCtp(final StatusController controller) throws IOExc assertThat(result.contentType()).isEqualTo(Http.MimeTypes.JSON); assertThat(contentAsString(result)).contains("\"healthy\" : false"); } + + private static Application app(final SphereClient sphereClient) { + return app(new CtpClientTestModule(sphereClient)); + } } \ No newline at end of file diff --git a/pt/controllers/WithSunriseApplication.java b/pt/controllers/WithSunriseApplication.java index c599ca562..2e5b19a74 100644 --- a/pt/controllers/WithSunriseApplication.java +++ b/pt/controllers/WithSunriseApplication.java @@ -40,7 +40,7 @@ public interface CheckedConsumer { void apply(T t) throws Exception; } - protected final void run(final Application app, final String url, final CheckedConsumer test) { + protected static void run(final Application app, final String url, final CheckedConsumer test) { final int port = 3333; running(testServer(port, app), () -> { try { @@ -51,7 +51,7 @@ protected final void run(final Application app, final String url, final CheckedC }); } - protected final void run(final Application app, final Class controllerClass, + protected static void run(final Application app, final Class controllerClass, final CheckedConsumer test) { running(app, () -> { try { @@ -62,26 +62,27 @@ protected final void run(final Application app, final Cla }); } - protected final void setContext(final Http.Request request) { + protected static void setContext(final Http.Request request) { Http.Context.current.set(new Http.Context(request)); } - protected Http.RequestBuilder requestBuilder() { + protected static Http.RequestBuilder requestBuilder() { return new Http.RequestBuilder().id(1L); } - protected Application app(final Module ... modules) { - final Module testModule = Modules.override(testModule()).with(modules); - return appBuilder().overrides(testModule).build(); + protected static Application app(final Module ... modules) { + return appBuilder(modules).build(); } - protected GuiceApplicationBuilder appBuilder() { + protected static GuiceApplicationBuilder appBuilder(final Module ... modules) { + final Module testModule = Modules.override(testModule()).with(modules); return new GuiceApplicationBuilder() .in(Environment.simple()) - .loadConfig(testConfiguration()); + .loadConfig(testConfiguration()) + .overrides(testModule); } - protected Module testModule() { + protected static Module testModule() { final List categories = readCtpObject("data/categories.json", CategoryQuery.resultTypeReference()).getResults(); return Modules.combine( new CtpClientTestModule(TestableSphereClient.ofEmptyResponse()), @@ -92,11 +93,12 @@ protected Module testModule() { new CategoryTreeTestModule(categories)); } - protected Configuration testConfiguration() { + protected static Configuration testConfiguration() { final Configuration configuration = Configuration.load(new Environment(Mode.TEST)); final Map additionalSettings = new HashMap<>(); additionalSettings.put("application.metrics.enabled", false); additionalSettings.put("application.settingsWidget.enabled", false); + additionalSettings.put("application.auth.credentials", null); additionalSettings.put("play.crypto.secret", RandomStringUtils.randomAlphanumeric(5)); return new Configuration(additionalSettings).withFallback(configuration); } From a3faa57e8066050501f03776b4efa761244bd0e9 Mon Sep 17 00:00:00 2001 From: Laura Luiz Date: Fri, 5 Feb 2016 12:15:27 +0100 Subject: [PATCH 11/11] Use a different logger for basic auth class --- app/basicauth/BasicAuthRequestHandler.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/basicauth/BasicAuthRequestHandler.java b/app/basicauth/BasicAuthRequestHandler.java index 57d1d8ce1..9077a951c 100644 --- a/app/basicauth/BasicAuthRequestHandler.java +++ b/app/basicauth/BasicAuthRequestHandler.java @@ -19,6 +19,7 @@ * Request handler that enables HTTP basic access authentication. */ public class BasicAuthRequestHandler extends DefaultHttpRequestHandler { + private static final Logger.ALogger LOGGER = Logger.of(BasicAuthRequestHandler.class); private final Optional basicAuth; @Inject @@ -53,10 +54,10 @@ public F.Promise call(final Http.Context ctx) throws Throwable { private F.Promise authenticationResult(final Http.Context ctx, final boolean isAuthorized) throws Throwable { if (isAuthorized) { - Logger.debug("Authorized"); + LOGGER.debug("Authorized"); return delegate.call(ctx); } else { - Logger.debug("Unauthorized"); + LOGGER.info("Unauthorized"); return F.Promise.pure(unauthorized()); } }