tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Igor Drobiazko <igor.drobia...@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 08:10:44 GMT
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, <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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message