From 0061ebe19ba325c119cb76225ab39b9c0114768a Mon Sep 17 00:00:00 2001 From: Dirk Heuvels Date: Tue, 19 Jul 2022 15:19:30 +0200 Subject: [PATCH] APPNG-2442 Mapping logic and unit tests. * Removed ambiguity between name and hostname mappings. * Polished documentation. * Added tests for RequestUtil. --- .../main/java/org/appng/api/RequestUtil.java | 178 ++++++++++++------ .../main/java/org/appng/api/model/Site.java | 14 +- .../org/appng/api/RequestMappingTest.java | 155 +++++++++++++++ appng-template-parent/pom.xml | 2 +- 4 files changed, 279 insertions(+), 70 deletions(-) create mode 100644 appng-api/src/test/java/org/appng/api/RequestMappingTest.java diff --git a/appng-api/src/main/java/org/appng/api/RequestUtil.java b/appng-api/src/main/java/org/appng/api/RequestUtil.java index fec9875ae..00f5214b4 100644 --- a/appng-api/src/main/java/org/appng/api/RequestUtil.java +++ b/appng-api/src/main/java/org/appng/api/RequestUtil.java @@ -35,45 +35,98 @@ import lombok.extern.slf4j.Slf4j; /** - * Utility-class for retrieving {@link Site}s by name,host or {@link ServletRequest} and also creating a {@link Path} - * -object based on a {@link ServletRequest}. + * Utility-class for mapping requests to {@link Site}s. Also creates a {@link Path} object based on a + * {@link ServletRequest}. + * + *

+ * Mapping logic + *

+ * + *

+ *

* * @author Matthias Müller + * @author Dirk Heuvels */ @Slf4j public class RequestUtil { - private static final String SERVER_LOCAL_NAME = "SERVER_LOCAL_NAME"; - private static final String X_APPNG_SITE = "X-appNG-site"; + static final String SERVER_LOCAL_NAME = "SERVER_LOCAL_NAME"; + static final String X_APPNG_SITE = "X-appNG-site"; + + private enum MatchScope { + MATCH_SITE_NAME, MATCH_SITE_HOSTS, UNDEFINED + } /** - * Retrieves a {@link Site} by its name. + * Class to describe the request's name attribute used for site-mapping. + */ + private static class RequestIdentifier { + public String name; + public MatchScope matchScope; + + RequestIdentifier() { + this.matchScope = MatchScope.UNDEFINED; + } + + RequestIdentifier(String name, MatchScope matchScope) { + this.name = name; + this.matchScope = matchScope; + } + } + + /** + * Retrieves a {@link Site} using the mapping logic described in the class documentation above. * * @param env - * the current {@link Environment} + * the current {@link Environment} * @param servletRequest - * the current {@link ServletRequest} + * the current {@link ServletRequest} * - * @return the {@link Site}, if any + * @return the {@link Site} or null, if no site matched * - * @see #getSiteName(Environment, ServletRequest) */ public static Site getSite(Environment env, ServletRequest servletRequest) { - if (null == servletRequest || null == env) { + if (null == servletRequest || null == env) + return null; + RequestIdentifier reqId = getRequestIdentifier(env, servletRequest); + switch (reqId.matchScope) { + case MATCH_SITE_NAME: + return getSiteByName(env, reqId.name); + case MATCH_SITE_HOSTS: + return getSiteByHost(env, reqId.name); + default: return null; } - String siteName = getSiteName(env, servletRequest); - Optional optionalSite = Optional.ofNullable(getSiteByName(env, siteName)); - return optionalSite.isPresent() ? optionalSite.get() : getSiteByHost(env, siteName); } /** - * Retrieves a {@link Site} by its hostnames. + * Retrieves a {@link Site} by its hostnames (primary and aliases). * * @param env - * the current {@link Environment} + * the current {@link Environment} * @param host - * the hostname to match against the hostnames of the {@link Site} + * the name to be compared against the {@link Site}'s hostnames * * @return the {@link Site} or null if no site matches * @@ -82,8 +135,11 @@ public static Site getSite(Environment env, ServletRequest servletRequest) { * */ public static Site getSiteByHost(Environment env, String host) { + if (null == host || host.isEmpty()) + return null; for (Site site : getSiteMap(env).values()) { - if (host.equals(site.getHost()) || site.getHostAliases().contains(host)) return site; + if (host.equals(site.getHost()) || site.getHostAliases().contains(host)) + return site; } return null; } @@ -92,9 +148,9 @@ public static Site getSiteByHost(Environment env, String host) { * Retrieves a {@link Site} by its name. * * @param env - * the current {@link Environment} + * the current {@link Environment} * @param name - * the name of the {@link Site} + * the name of the {@link Site} * * @return the {@link Site}, if any * @@ -109,9 +165,9 @@ public static Site getSiteByName(Environment env, String name) { * it's state is {@code SiteState#STARTED}. * * @param env - * the current {@link Environment} + * the current {@link Environment} * @param name - * the name of the {@link Site} + * the name of the {@link Site} * * @return the {@link Site}, if any * @@ -166,11 +222,11 @@ public static Set getSiteNames(Environment env) { * Creates and returns a {@link PathInfo}-object based upon the given parameters. * * @param env - * the current {@link Environment} + * the current {@link Environment} * @param site - * the current {@link Site} + * the current {@link Site} * @param servletPath - * the current servlet-path + * the current servlet-path * * @return a {@link PathInfo}-object */ @@ -193,62 +249,60 @@ public static PathInfo getPathInfo(Environment env, Site site, String servletPat blobDirectories, documentDirectories, repoPath, monitoringPath, extension); } - /** @deprecated use {@link #getSiteName(Environment, ServletRequest)} */ + /** @deprecated use {@link #getRequestIdentifier(Environment, ServletRequest)} */ @Deprecated public static String getHostIdentifier(ServletRequest request, Environment env) { return getSiteName(env, request); } + /** @deprecated use {@link #getRequestIdentifier(Environment, ServletRequest)} */ + @Deprecated + public static String getSiteName(Environment env, ServletRequest request) { + RequestIdentifier reqId = getRequestIdentifier(env, request); + return reqId.name; + } + /** - * Retrieves a {@link Site}'s name for the given {@link ServletRequest}, using the given {@link Environment} to - * retrieve the {@link VHostMode}. + * Retrieves a name according to the mapping logic described in the class documentation above together with the + * matching scope to be applied. * * @param env - * an {@link Environment} + * an {@link Environment} * @param request - * the {@link ServletRequest} + * the {@link ServletRequest} * - * @return - *
    - *
  • the IP-address, if {@link VHostMode#IP_BASED} is used (see {@link ServletRequest#getLocalAddr()}) - *
  • the value of the request-attribute {@value #SERVER_LOCAL_NAME}, if present. - *

    - * This header has to be added by the webserver of choice (usually Apache - * httpd), in case a {@link Site} needs to be accessible from a domain that is different from the one - * configured by {@link Site#getDomain()}. - *

    - *
  • the value of the request-header {@value #X_APPNG_SITE}, if present. - *
  • the server name, otherwise (see {@link ServletRequest#getServerName()}) - *
+ * @return {@link RequestUtil.RequestIdentifier} object */ - public static String getSiteName(Environment env, ServletRequest request) { + public static RequestIdentifier getRequestIdentifier(Environment env, ServletRequest request) { Properties platformProperties = env.getAttribute(Scope.PLATFORM, Platform.Environment.PLATFORM_CONFIG); VHostMode vHostMode = VHostMode.valueOf(platformProperties.getString(Platform.Property.VHOST_MODE)); - String siteName; - if (VHostMode.NAME_BASED.equals(vHostMode)) { - siteName = StringUtils.trimToNull((String) request.getAttribute(SERVER_LOCAL_NAME)); - if (null == siteName) { - siteName = StringUtils.trimToNull(((HttpServletRequest) request).getHeader(X_APPNG_SITE)); - if (null == siteName) { - siteName = request.getServerName(); + + RequestIdentifier reqId = new RequestIdentifier(); + if (VHostMode.IP_BASED.equals(vHostMode)) { + reqId = new RequestIdentifier(request.getLocalAddr(), MatchScope.MATCH_SITE_HOSTS); + LOGGER.trace("Using '{}' from 'ServletRequest.getLocalAddr()' for site mapping", reqId.name); + } else { + String attrVal = null; + attrVal = StringUtils.trimToNull((String) request.getAttribute(SERVER_LOCAL_NAME)); + if (null != attrVal) { + reqId = new RequestIdentifier(attrVal, MatchScope.MATCH_SITE_NAME); + LOGGER.trace("Using '{}' from request attribute '{}' for site mapping", attrVal, SERVER_LOCAL_NAME); + } else { + attrVal = StringUtils.trimToNull(((HttpServletRequest) request).getHeader(X_APPNG_SITE)); + if (null != attrVal) { + reqId = new RequestIdentifier(attrVal, MatchScope.MATCH_SITE_NAME); + LOGGER.trace("Using '{}' from request header '{}' for site mapping", attrVal, X_APPNG_SITE); + } else { + reqId = new RequestIdentifier(request.getServerName(), MatchScope.MATCH_SITE_HOSTS); if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Retrieved sitename '{}' from request.getServerName() (Host: {})", siteName, - ((HttpServletRequest) request).getHeader(HttpHeaders.HOST)); + LOGGER.trace( + "Using '{}' from 'request.getServerName()' for site mapping (HOST header was '{}')", + reqId.name, ((HttpServletRequest) request).getHeader(HttpHeaders.HOST)); } - } else if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Retrieved sitename '{}' from request header '{}'", siteName, X_APPNG_SITE); } - } else if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Retrieved sitename '{}' from request attribute '{}'", siteName, SERVER_LOCAL_NAME); - } - - } else { - siteName = request.getLocalAddr(); - if (LOGGER.isTraceEnabled()) { - LOGGER.trace("Retrieved sitename '{}' from request.getLocalAddr()", siteName); } } - return siteName; + return reqId; } private RequestUtil() { diff --git a/appng-api/src/main/java/org/appng/api/model/Site.java b/appng-api/src/main/java/org/appng/api/model/Site.java index b9e9c22d1..27bb47adf 100644 --- a/appng-api/src/main/java/org/appng/api/model/Site.java +++ b/appng-api/src/main/java/org/appng/api/model/Site.java @@ -53,7 +53,7 @@ enum SiteState { * {@link Site}. * * @param name - * the name of the {@link Application} + * the name of the {@link Application} * * @return the {@link Application}, if such a {@link Application} is assigned to this {@link Site}, {@code null} * otherwise @@ -64,7 +64,7 @@ enum SiteState { * Checks whether the {@link Application} with the given name is assigned to this {@link Site}. * * @param name - * the name of the {@link Application} + * the name of the {@link Application} * * @return {@code true} if the {@link Application} with the given name is assigned to this {@link Site}, * {@code false} otherwise @@ -136,11 +136,11 @@ enum SiteState { * does start with a slash, it is expected to be a target relative to the domain of this site. * * @param env - * the actual {@link Environment} + * the actual {@link Environment} * @param target - * the redirect target + * the redirect target * @param statusCode - * -the HTTP status code to send. Use constants from {@link HttpServletRequest} + * -the HTTP status code to send. Use constants from {@link HttpServletRequest} */ void sendRedirect(Environment env, String target, Integer statusCode); @@ -148,7 +148,7 @@ enum SiteState { * Returns the {@link File} defined by the given path, which is located relative to the site's repository-folder. * * @param relativePath - * the relative path of the file + * the relative path of the file * * @return the {@link File}, if such a path exists */ @@ -179,7 +179,7 @@ enum SiteState { * If clustering is enabled, sends an {@link Event} to other appNG nodes * * @param event - * the event to send + * the event to send * * @return {@code true} if the event was sent successfully */ diff --git a/appng-api/src/test/java/org/appng/api/RequestMappingTest.java b/appng-api/src/test/java/org/appng/api/RequestMappingTest.java new file mode 100644 index 000000000..4d516d0cf --- /dev/null +++ b/appng-api/src/test/java/org/appng/api/RequestMappingTest.java @@ -0,0 +1,155 @@ +/* + * Copyright 2011-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.appng.api; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; + +import javax.servlet.ServletContext; +import javax.servlet.http.HttpServletRequest; + +import org.appng.api.model.Properties; +import org.appng.api.model.Site; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; + +public class RequestMappingTest { + + @Mock + private ServletContext ctx; + + @Mock + private Environment env; + + @Mock + Properties platformProperties; + + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + Mockito.when(env.getAttribute(Scope.PLATFORM, Platform.Environment.PLATFORM_CONFIG)).thenReturn(platformProperties); + + Map siteMap = new HashMap<>(); + Site site1 = Mockito.mock(Site.class); + Mockito.when(site1.getName()).thenReturn("site1"); + Mockito.when(site1.getHost()).thenReturn("site1.loc"); + Mockito.when(site1.getHostAliases()).thenReturn(new HashSet(Arrays.asList("alias1-1.loc", "alias1-2.loc", "alias1-2.loc"))); + siteMap.put(site1.getName(), site1); + + Site site2 = Mockito.mock(Site.class); + Mockito.when(site2.getName()).thenReturn("site2"); + Mockito.when(site2.getHost()).thenReturn("127.0.47.11"); + Mockito.when(site2.getHostAliases()).thenReturn(new HashSet(Arrays.asList("127.0.47.12"))); + siteMap.put(site2.getName(), site2); + + Site site3 = Mockito.mock(Site.class); + Mockito.when(site3.getName()).thenReturn("site3"); + Mockito.when(site3.getHost()).thenReturn("site3.loc"); + Mockito.when(site3.getHostAliases()).thenReturn(new HashSet()); + siteMap.put(site3.getName(), site3); + + Mockito.when(env.getAttribute(Scope.PLATFORM, Platform.Environment.SITES)).thenReturn(siteMap); + } + + @Test + public void testSiteMappingIPBasedMatchHost() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getLocalAddr()).thenReturn("127.0.47.11"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.IP_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertEquals(site.getName(), "site2"); + } + + @Test + public void testSiteMappingIPBasedMatchHostAlias() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getLocalAddr()).thenReturn("127.0.47.12"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.IP_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertEquals(site.getName(), "site2"); + } + + @Test + public void testSiteMappingIPBasedMiss() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getLocalAddr()).thenReturn("1.2.3.4"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.IP_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertNull(site); + } + + @Test + public void testSiteMappingNameBasedMatchHost() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getServerName()).thenReturn("site3.loc"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.NAME_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertEquals(site.getName(), "site3"); + } + + @Test + public void testSiteMappingNameBasedMatchHostAlias() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getServerName()).thenReturn("alias1-2.loc"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.NAME_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertEquals(site.getName(), "site1"); + } + + @Test + public void testSiteMappingDirectMatchSERVER_LOCAL_NAME() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getAttribute(RequestUtil.SERVER_LOCAL_NAME)).thenReturn("site1"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.NAME_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertEquals(site.getName(), "site1"); + } + + @Test + public void testSiteMappingDirectMatchX_APPNG_SITE() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getHeader(RequestUtil.X_APPNG_SITE)).thenReturn("site2"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.NAME_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertEquals(site.getName(), "site2"); + } + + @Test + public void testSiteMappingDirectMatchPriority() throws Exception { + HttpServletRequest request = Mockito.mock(HttpServletRequest.class); + Mockito.when(request.getAttribute(RequestUtil.X_APPNG_SITE)).thenReturn("site3"); + Mockito.when(request.getAttribute(RequestUtil.SERVER_LOCAL_NAME)).thenReturn("site1"); + Mockito.when(platformProperties.getString(Platform.Property.VHOST_MODE)).thenReturn(VHostMode.NAME_BASED.toString()); + + Site site = RequestUtil.getSite(env, request); + Assert.assertEquals(site.getName(), "site1"); + } + +} \ No newline at end of file diff --git a/appng-template-parent/pom.xml b/appng-template-parent/pom.xml index a2e5f43ae..2896b568b 100644 --- a/appng-template-parent/pom.xml +++ b/appng-template-parent/pom.xml @@ -54,7 +54,7 @@ yyyyMMdd-HHmm ${maven.build.timestamp} UTF-8 - 1.24.4 + 1.24.6-SNAPSHOT