Hi Kalle, congrats to your first commit. You should import Tapestry code formatting into your IDE. Please see here:
http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/support/?pathrev=1068758 Regarding the change. Wouldn't it be better to set defaults for new symbols to hostport = 80 and hostport-secure=443? On Wed, Feb 9, 2011 at 7:25 AM, <[email protected]> wrote: > Author: kaosko > Date: Wed Feb 9 06:25:01 2011 > New Revision: 1068758 > > URL: http://svn.apache.org/viewvc?rev=1068758&view=rev > Log: > Complete - issue TAP5-1414: Add HOSTNAME symbol to SymbolConstants, use in > BaseUrlSource > https://issues.apache.org/jira/browse/TAP5-1414 > - added HOSTNAME, HOSTPORT and HOSTPORT_SECURE symbols to SymbolConstants > - contributed defaults for the new symbols (defaults won't change existing > behavior) > - added javadoc > - modified BaseURLSourceImpl to use the new symbols > - added new test class for testing BaseURLSourceImpl > > Added: > > > tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java > Modified: > > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java > > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/BaseURLSource.java > > > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > > Modified: > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > URL: > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java?rev=1068758&r1=1068757&r2=1068758&view=diff > > ============================================================================== > --- > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > (original) > +++ > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/SymbolConstants.java > Wed Feb 9 06:25:01 2011 > @@ -334,4 +334,29 @@ public class SymbolConstants > * @since 5.2.5 > */ > public static final String COMPONENT_RENDER_TRACING_ENABLED = > "tapestry.component-render-tracing-enabled"; > + > + /** > + * The hostname that application should use when constructing an > absolute URL. The default is "", i.e. an empty string, > + * in which case system will use request.getServerName(). Not the same > as environment variable HOSTNAME, but you can also > + * contribute "$HOSTNAME" as the value to make it the same as the > environment variable HOSTNAME. > + * > + * @since 5.2.5 > + */ > + public static final String HOSTNAME = "tapestry.hostname"; > + > + /** > + * The hostport that application should use when constructing an > absolute URL. The default is "0", i.e. use the port value from > + * the request. > + * > + * @since 5.2.5 > + */ > + public static final String HOSTPORT = "tapestry.hostport"; > + > + /** > + * The secure (https) hostport that application should use when > constructing an absolute URL. The default is "0", i.e. use > + * the value from the request. > + * > + * @since 5.2.5 > + */ > + public static final String HOSTPORT_SECURE = > "tapestry.hostport-secure"; > } > > Modified: > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java > URL: > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java?rev=1068758&r1=1068757&r2=1068758&view=diff > > ============================================================================== > --- > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java > (original) > +++ > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BaseURLSourceImpl.java > Wed Feb 9 06:25:01 2011 > @@ -14,26 +14,44 @@ > > package org.apache.tapestry5.internal.services; > > +import org.apache.tapestry5.SymbolConstants; > +import org.apache.tapestry5.ioc.annotations.Inject; > +import org.apache.tapestry5.ioc.annotations.Symbol; > import org.apache.tapestry5.services.BaseURLSource; > import org.apache.tapestry5.services.Request; > > public class BaseURLSourceImpl implements BaseURLSource > { > private final Request request; > + > + private String hostname; > + private int hostPort; > + private int secureHostPort; > > - public BaseURLSourceImpl(Request request) > + public BaseURLSourceImpl(Request request, @Inject > @Symbol(SymbolConstants.HOSTNAME) String hostname, > + @Symbol(SymbolConstants.HOSTPORT) int hostPort, > @Symbol(SymbolConstants.HOSTPORT_SECURE) int secureHostPort) > { > this.request = request; > + this.hostname = hostname; > + this.hostPort = hostPort; > + this.secureHostPort = secureHostPort; > } > > public String getBaseURL(boolean secure) > { > - int port = request.getServerPort(); > + int port = secure ? secureHostPort : hostPort; > + String portSuffix = ""; > > - int schemeDefaultPort = request.isSecure() ? 443 : 80; > - > - String portSuffix = port == schemeDefaultPort ? "" : ":" + port; > - > - return String.format("%s://%s%s", secure ? "https" : "http", > request.getServerName(), portSuffix); > + if (port <= 0) { > + port = request.getServerPort(); > + int schemeDefaultPort = request.isSecure() ? 443 : 80; > + portSuffix = port == schemeDefaultPort ? "" : ":" + port; > + } > + else if (secure && port != 443) portSuffix = ":" + port; > + else if (port != 80) portSuffix = ":" + port; > + > + String hostname = "".equals(this.hostname) ? > request.getServerName() : this.hostname.startsWith("$") ? > System.getenv(this.hostname.substring(1)) : this.hostname; > + > + return String.format("%s://%s%s", secure ? "https" : "http", > hostname, portSuffix); > } > } > > Modified: > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/BaseURLSource.java > URL: > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/BaseURLSource.java?rev=1068758&r1=1068757&r2=1068758&view=diff > > ============================================================================== > --- > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/BaseURLSource.java > (original) > +++ > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/BaseURLSource.java > Wed Feb 9 06:25:01 2011 > @@ -24,7 +24,17 @@ import org.apache.tapestry5.ioc.services > * necessary to do a bit more, since <code>getServerName()</code> will > often be the name of the internal server (not > * visible to the client web browser), and a hard-coded name of a server > that <em>is</em> visible to the web browser > * is needed. Further, in testing, non-default ports are often used. In > those cases, an overriding contribution to the > - * {@link ServiceOverride} service will allow a custom implementation to > supercede the default version. > + * {@link ServiceOverride} service will allow a custom implementation to > supercede the default version. You may also > + * contribute application specific values for the following {@link > org.apache.tapestry5.SymbolConstants}: > + * {@link org.apache.tapestry5.SymbolConstants#HOSTNAME}, {@link > org.apache.tapestry5.SymbolConstants#HOSTPORT} and > + * {@link org.apache.tapestry5.SymbolConstants#HOSTPORT_SECURE} to alter > the behavior of the default BaseURLSource > + * implementation. The default values for the SymbolConstants require > {@link org.apache.tapestry5.services.Request} > + * context to be available. If you contribute specific values for the > specified SymbolConstants, it's safe to use > + * the default implementation of this service outside of request context, > for example in a batch job. For > + * {@link org.apache.tapestry5.SymbolConstants#HOSTNAME}, a value starting > with a dollar sign ($) will be resolved > + * using {@link java.langSystem#getEnv()} - contributing "$HOSTNAME" for > {@link org.apache.tapestry5.SymbolConstants#HOSTNAME} > + * is the most sensible choice for a dynamic value that doesn't use > + * {@link org.apache.tapestry5.services.Request#getServerName()}. > */ > public interface BaseURLSource > { > > Modified: > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > URL: > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java?rev=1068758&r1=1068757&r2=1068758&view=diff > > ============================================================================== > --- > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > (original) > +++ > tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java > Wed Feb 9 06:25:01 2011 > @@ -2450,6 +2450,11 @@ public final class TapestryModule > configuration.add(InternalSymbols.PRE_SELECTED_FORM_NAMES, > "reset,submit,select,id,method,action,onsubmit"); > > configuration.add(SymbolConstants.COMPONENT_RENDER_TRACING_ENABLED, > "false"); > + > + // The default values denote "use values from request" > + configuration.add(SymbolConstants.HOSTNAME, ""); > + configuration.add(SymbolConstants.HOSTPORT, "0"); > + configuration.add(SymbolConstants.HOSTPORT_SECURE, "0"); > } > > /** > > Added: > tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java > URL: > http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java?rev=1068758&view=auto > > ============================================================================== > --- > tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java > (added) > +++ > tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/BaseURLSourceImplTest.java > Wed Feb 9 06:25:01 2011 > @@ -0,0 +1,74 @@ > +// Copyright 2008, 2010, 2011 The Apache Software Foundation > +// > +// 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.apache.tapestry5.internal.services; > + > +import org.apache.tapestry5.internal.test.InternalBaseTestCase; > +import org.apache.tapestry5.services.BaseURLSource; > +import org.apache.tapestry5.services.Request; > +import org.testng.annotations.BeforeMethod; > +import org.testng.annotations.Test; > + > + > +public class BaseURLSourceImplTest extends InternalBaseTestCase { > + private Request request; > + > + @BeforeMethod > + public void setUp() { > + request = mockRequest(); > + } > + > + @Test > + public void getBaseURLFromRequest() { > + expect(request.getServerName()).andReturn("localhost").once(); > + expect(request.getServerPort()).andReturn(80).once(); > + expect(request.isSecure()).andReturn(false).once(); > + replay(); > + BaseURLSource baseURLSource = new BaseURLSourceImpl(request, > "localhost", 0, 0); > + assertEquals(baseURLSource.getBaseURL(false), "http://localhost > "); > + } > + > + @Test > + public void getBaseURLWithContributedHostname() { > + expect(request.getServerPort()).andReturn(80).once(); > + expect(request.isSecure()).andReturn(false).once(); > + replay(); > + BaseURLSource baseURLSource = new BaseURLSourceImpl(request, " > my.server.com", 0, 0); > + assertEquals(baseURLSource.getBaseURL(false), " > http://my.server.com";); > + } > + > + @Test > + public void getBaseURLWithEnvHostname() { > + expect(request.getServerPort()).andReturn(80).once(); > + expect(request.isSecure()).andReturn(false).once(); > + replay(); > + BaseURLSource baseURLSource = new BaseURLSourceImpl(request, > "$HOSTNAME", 0, 0); > + assertEquals(baseURLSource.getBaseURL(false), "http://"; + > System.getenv("HOSTNAME")); > + } > + > + @Test > + public void getBaseURLWithContributedValuesDontUseRequest() { > + replay(); > + BaseURLSource baseURLSource = new BaseURLSourceImpl(request, > "localhost", 80, 443); > + assertEquals(baseURLSource.getBaseURL(false), "http://localhost > "); > + } > + > + @Test > + public void getBaseURLWithContributedNonStandardSecurePort() { > + replay(); > + BaseURLSource baseURLSource = new BaseURLSourceImpl(request, > "localhost", 80, 8443); > + assertEquals(baseURLSource.getBaseURL(true), " > https://localhost:8443";); > + } > + > +} > > > -- Best regards, Igor Drobiazko http://tapestry5.de
