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]
