Ok, will do. On Wed, Feb 9, 2011 at 11:58 AM, Howard Lewis Ship <[email protected]> wrote: > If you are committing to trunk, you are @since 5.3.0. I suppose > @since 5.3 would be sufficient, I kind of like the extra precision. > > The 5.2.5 was becuase I committed a back fix to 5.2.5 and then > cherrypicked it onto trunk. > > I may be looking at a couple of additional small changes for 5.2.5 and > then seeing about a release. > > On Wed, Feb 9, 2011 at 11:37 AM, Kalle Korhonen > <[email protected]> wrote: >> On Wed, Feb 9, 2011 at 12:10 AM, Igor Drobiazko >> <[email protected]> wrote: >>> 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 >> >> Thanks Igor, meant to ask about that. >> >>> Regarding the change. Wouldn't it be better to set defaults for new symbols >>> to hostport = 80 and hostport-secure=443? >> >> That's certainly up for discussion. I didn't want to change the >> current behavior without any consensus. It'd almost make sense to use >> those as the defaults, but it could cause some unexpected surprises in >> development environments (interestingly BaseUrlSourceImpl doesn't >> honor tapestry.secure-enabled, another issue there?). Perhaps you >> could do something more complex, such as contribute 80 and 443 when >> production mode is on, but not at that time when contributing factory >> defaults. Ties to the larger discussion about better supporting dev, >> test, production profiles, but probably best to leave that for another >> issue, don't you think? >> >> One more thing - which value should I have used for @since? 5.3.0? (I >> used 5.2.5 for now following Howard's lead on another new symbol, but >> didn't check if his was actually merged into 5.2 maintenance branch). >> >> Kalle >> >> >>> 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 >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >> > > > > -- > Howard M. Lewis Ship > > Creator of Apache Tapestry > > The source for Tapestry training, mentoring and support. Contact me to > learn how I can get you up and productive in Tapestry fast! > > (971) 678-5210 > http://howardlewisship.com > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
--------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
