tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Howard Lewis Ship <hls...@gmail.com>
Subject Re: svn commit: r1068758 - in /tapestry/tapestry5/trunk/tapestry-core/src: main/java/org/apache/tapestry5/ main/java/org/apache/tapestry5/internal/services/ main/java/org/apache/tapestry5/services/ test/java/org/apache/tapestry5/internal/services/
Date Wed, 09 Feb 2011 19:58:30 GMT
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
<kalle.o.korhonen@gmail.com> wrote:
> On Wed, Feb 9, 2011 at 12:10 AM, Igor Drobiazko
> <igor.drobiazko@gmail.com> 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, <kaosko@apache.org> 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: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>



-- 
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: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Mime
View raw message