On Thu, Dec 4, 2014 at 8:03 PM, <[email protected]> wrote:
> Repository: wicket
> Updated Branches:
> refs/heads/master f7fc5d095 -> f4b51cd57
>
>
> WICKET-5780 Add a resource reference for ContextRelativeResource
>
> Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
> Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/f4b51cd5
> Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/f4b51cd5
> Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/f4b51cd5
>
> Branch: refs/heads/master
> Commit: f4b51cd577699b1a7d3f1463ce239386cfa68ce5
> Parents: f7fc5d0
> Author: Andrea Del Bene <[email protected]>
> Authored: Thu Dec 4 17:38:39 2014 +0100
> Committer: Andrea Del Bene <[email protected]>
> Committed: Thu Dec 4 17:58:02 2014 +0100
>
> ----------------------------------------------------------------------
> .../ContextRelativeResourceReference.java | 173 +++++++++++++++++++
> .../resource/PackageResourceReference.java | 22 +--
> .../ContextRelativeResourceReferenceTest.java | 85 +++++++++
> .../wicket/util/resource/ResourceUtils.java | 37 +++-
> 4 files changed, 294 insertions(+), 23 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/f4b51cd5/wicket-core/src/main/java/org/apache/wicket/request/resource/ContextRelativeResourceReference.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/request/resource/ContextRelativeResourceReference.java
> b/wicket-core/src/main/java/org/apache/wicket/request/resource/ContextRelativeResourceReference.java
> new file mode 100644
> index 0000000..4d7b81f
> --- /dev/null
> +++
> b/wicket-core/src/main/java/org/apache/wicket/request/resource/ContextRelativeResourceReference.java
> @@ -0,0 +1,173 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You 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.wicket.request.resource;
> +
> +import org.apache.wicket.Application;
> +import org.apache.wicket.util.lang.Args;
> +import org.apache.wicket.util.resource.ResourceUtils;
> +
> +/**
> + * This is a ResourceReference to handle context-relative resources such
> as js, css and
> + * picture files placed in a folder on the context root (ex:
> '/css/coolTheme.css').
> + * The class has a flag (see {@link #isMinifyIt()}) to decide if
> referenced resource can be
> + * minified (ex: '/css/coolTheme.min.css') or not.
> + *
> + * @author Andrea Del Bene
> + */
> +public class ContextRelativeResourceReference extends ResourceReference
> +{
> +
> + /** The Constant serialVersionUID. */
> + private static final long serialVersionUID = 1L;
> +
> + /** Says if the resource name can be minified or not. */
> + private final boolean minifyIt;
> +
> + /** The minfied postfix. */
> + private final String minPostfix;
> +
> + /** The context relative resource. */
> + private final ContextRelativeResource contextRelativeResource;
> +
> + /**
> + * Instantiates a new context relative resource reference for the
> given name. The resource
> + * will be minified in DEPLOYMENT mode and "min" will be used as
> postfix.
> + *
> + * @param name
> + * the resource name
> + */
> + public ContextRelativeResourceReference(final String name)
> + {
> + this(name, ResourceUtils.MIN_POSTFIX_DEFAULT, true);
> + }
> +
> +
> + /**
> + * Instantiates a new context relative resource reference for the
> given name.
> + * Parameter {@code minifyIt} says if the resource can be minified
> (true) or not (false).
> + *
> + * @param name
> + * the resource name
> + * @param minifyIt
> + * says if the resource name can be
> minified or not
> + */
> + public ContextRelativeResourceReference(final String name, final
> boolean minifyIt)
> + {
> + this(name, ResourceUtils.MIN_POSTFIX_DEFAULT, minifyIt);
> + }
> +
> + /**
> + * Instantiates a new context relative resource reference for the
> given name. We can
> + * specify which postfix we want to use for minification with
> parameter @code minPostfix}
> + *
> + * @param name
> + * the resource name
> + * @param minPostfix
> + * the minfied postfix
> + */
> + public ContextRelativeResourceReference(final String name, final
> String minPostfix)
> + {
> + this(name, minPostfix, true);
> + }
> +
> + /**
> + * Instantiates a new context relative resource reference for the
> given name. We can
> + * specify which postfix we want to use for minification with
> parameter @code minPostfix}
> + * while parameter {@code minifyIt} says if the resource can be
> minified (true) or not (false).
> + * @param name
> + * the resource name
> + * @param minPostfix
> + * the minfied postfix
> + * @param minifyIt
> + * says if the resource name can be
> minified or not
> + */
> + public ContextRelativeResourceReference(final String name, final
> String minPostfix, final boolean minifyIt)
> + {
> + super(name);
> +
> + Args.notNull(minPostfix, "minPostfix");
> +
> + this.minPostfix = minPostfix;
> + this.minifyIt = minifyIt;
>
> + this.contextRelativeResource =
> buildContextRelativeResource(name, minPostfix);
>
What is the reason the resource has to be pre-built. A new instance could
be instantiated on each request in #getReource() too.
> + }
> +
> + /**
> + * Build the context-relative resource for this resource reference.
> + *
> + * @param name
> + * the resource name
> + * @param minPostfix
> + * the postfix to use to minify the
> resource name (typically "min")
> + * @return the context-relative resource
> + */
> + protected ContextRelativeResource
> buildContextRelativeResource(final String name, final String minPostfix)
>
Overrideable method called by the constructor. May lead to subtle bugs ...
> + {
> + String minifiedName = name;
> +
> + if (canBeMinified())
> + {
> + minifiedName = ResourceUtils.getMinifiedName(name,
> minPostfix);
> + }
> +
> + return new ContextRelativeResource(minifiedName);
> + }
> +
> + /**
> + * Says if the referenced resource can be minified. It returns
> {@code true} if
> + * both flag {@link #minifyIt} and application's resource settings
> method
> + * {@link
> org.apache.wicket.settings.ResourceSettings#getUseMinifiedResources()}}
> + * are true.
> + *
> + * @return {@code true} if resource can be minified, {@code false}
> otherwise
> + */
> + protected boolean canBeMinified()
> + {
> + return minifyIt && Application.exists()
> + &&
> Application.get().getResourceSettings().getUseMinifiedResources();
> + }
> +
> + /* (non-Javadoc)
> + * @see
> org.apache.wicket.request.resource.ResourceReference#getResource()
> + */
>
This kind of javadoc is just noise. I'm trying to remove them from the rest
of the code anyway.
> + @Override
> + public final ContextRelativeResource getResource()
> + {
> + return contextRelativeResource;
> + }
> +
> + /**
> + * Returns the flag that says if the resource can be minified
> (true) or not (false).
> + *
> + * @return true, if resource can be minified
> + */
> + public final boolean isMinifyIt()
>
Why 'final' ?
This just reduces the flexibility.
And people complain about the overusa of 'final' in Wicket every second day
...
> + {
> + return minifyIt;
> + }
> +
> +
> + /**
> + * Gets the minified postfix we use for this resource.
> + *
> + * @return the minified postfix
> + */
> + public final String getMinPostfix()
>
final
> + {
> + return minPostfix;
> + }
> +}
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/f4b51cd5/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
> b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
> index 1c874ad..749769e 100644
> ---
> a/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
> +++
> b/wicket-core/src/main/java/org/apache/wicket/request/resource/PackageResourceReference.java
> @@ -21,10 +21,11 @@ import java.util.concurrent.ConcurrentMap;
>
> import org.apache.wicket.Application;
> import org.apache.wicket.Session;
> +import
> org.apache.wicket.core.util.resource.locator.IResourceStreamLocator;
> import org.apache.wicket.util.lang.Generics;
> import org.apache.wicket.util.lang.Packages;
> import org.apache.wicket.util.resource.IResourceStream;
> -import
> org.apache.wicket.core.util.resource.locator.IResourceStreamLocator;
> +import org.apache.wicket.util.resource.ResourceUtils;
> import org.slf4j.Logger;
> import org.slf4j.LoggerFactory;
>
> @@ -216,24 +217,7 @@ public class PackageResourceReference extends
> ResourceReference
> protected String getMinifiedName()
> {
> String name = super.getName();
> - String minifiedName;
> - int idxOfExtension = name.lastIndexOf('.');
> - if (idxOfExtension > -1)
> - {
> - String extension = name.substring(idxOfExtension);
> - final String baseName = name.substring(0,
> name.length() - extension.length() + 1);
> - if (!".min".equals(extension) &&
> !baseName.endsWith(".min."))
> - {
> - minifiedName = baseName + "min" +
> extension;
> - } else
> - {
> - minifiedName = name;
> - }
> - } else
> - {
> - minifiedName = name + ".min";
> - }
> - return minifiedName;
> + return ResourceUtils.getMinifiedName(name,
> ResourceUtils.MIN_POSTFIX_DEFAULT);
> }
>
> /**
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/f4b51cd5/wicket-core/src/test/java/org/apache/wicket/request/resource/ContextRelativeResourceReferenceTest.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-core/src/test/java/org/apache/wicket/request/resource/ContextRelativeResourceReferenceTest.java
> b/wicket-core/src/test/java/org/apache/wicket/request/resource/ContextRelativeResourceReferenceTest.java
> new file mode 100644
> index 0000000..80c31bd
> --- /dev/null
> +++
> b/wicket-core/src/test/java/org/apache/wicket/request/resource/ContextRelativeResourceReferenceTest.java
> @@ -0,0 +1,85 @@
> +/*
> + * Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You 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.wicket.request.resource;
> +
> +import org.apache.wicket.mock.MockApplication;
> +import org.apache.wicket.util.tester.WicketTester;
> +import org.junit.Assert;
> +import org.junit.BeforeClass;
> +import org.junit.Test;
> +
> +
> +public class ContextRelativeResourceReferenceTest
> +{
> + private static WicketTester tester;
>
Why static ?
It begs for memory/thread local leaks.
Why not extending from WicketTestCase ?
> +
> + static final String RESOURCE_NAME = "/foo/baar/myLibrary";
> + static final String ALREADY_MINIFIED = RESOURCE_NAME + ".min.js";
> + static final String TO_BE_MINIFIED = RESOURCE_NAME + ".js";
> + static final String CUSTOM_SUFFIX = "compress";
> +
> + @BeforeClass
> + static public void setUp()
> + {
> + MockApplication application = new MockApplication()
> + {
> + @Override
> + protected void init()
> + {
> + super.init();
> +
> getResourceSettings().setUseMinifiedResources(true);
> + }
> + };
> +
> + tester = new WicketTester(application);
> + }
> +
> +
> + @Test
> + public void testMinifyResource() throws Exception
> + {
> + ContextRelativeResourceReference resourceReference = new
> ContextRelativeResourceReference(TO_BE_MINIFIED);
> + Assert.assertTrue(testResourceKey(resourceReference,
> ALREADY_MINIFIED));
>
no tester.destroy() => thread local leak
> + }
> +
> + @Test
> + public void testDontMinifyResource() throws Exception
> + {
> + ContextRelativeResourceReference resourceReference = new
> ContextRelativeResourceReference(ALREADY_MINIFIED, false);
> + Assert.assertTrue(testResourceKey(resourceReference,
> ALREADY_MINIFIED));
> +
> + resourceReference = new
> ContextRelativeResourceReference(TO_BE_MINIFIED, false);
> + Assert.assertTrue(testResourceKey(resourceReference,
> TO_BE_MINIFIED));
>
no tester.destroy() => thread local leak
> +
> + }
> +
> + @Test
> + public void testCustomSuffix() throws Exception
> + {
> + ContextRelativeResourceReference resourceReference = new
> ContextRelativeResourceReference(TO_BE_MINIFIED, CUSTOM_SUFFIX);
> + Assert.assertTrue(testResourceKey(resourceReference,
> RESOURCE_NAME + "." + CUSTOM_SUFFIX + ".js"));
>
no tester.destroy() => thread local leak
> + }
> +
> + private boolean testResourceKey(ContextRelativeResourceReference
> resourceReference, String expectedName)
> + {
> + ContextRelativeResource resource =
> resourceReference.getResource();
> + String resourceKey = resource.getCacheKey().toString();
> +
> + return resourceKey.endsWith(expectedName);
> + }
> +
> +}
>
>
> http://git-wip-us.apache.org/repos/asf/wicket/blob/f4b51cd5/wicket-util/src/main/java/org/apache/wicket/util/resource/ResourceUtils.java
> ----------------------------------------------------------------------
> diff --git
> a/wicket-util/src/main/java/org/apache/wicket/util/resource/ResourceUtils.java
> b/wicket-util/src/main/java/org/apache/wicket/util/resource/ResourceUtils.java
> index a51f359..5fd28dc 100644
> ---
> a/wicket-util/src/main/java/org/apache/wicket/util/resource/ResourceUtils.java
> +++
> b/wicket-util/src/main/java/org/apache/wicket/util/resource/ResourceUtils.java
> @@ -31,6 +31,8 @@ import org.apache.wicket.util.string.Strings;
> */
> public class ResourceUtils
> {
> + public static final String MIN_POSTFIX_DEFAULT = "min";
>
missing javadoc
> +
> private static final Pattern LOCALE_PATTERN =
> Pattern.compile("_([a-z]{2})(_([A-Z]{2})(_([^_]+))?)?$");
>
> private final static Set<String> isoCountries = new HashSet<>(
> @@ -38,14 +40,41 @@ public class ResourceUtils
>
> private final static Set<String> isoLanguages = new HashSet<>(
> Arrays.asList(Locale.getISOLanguages()));
> -
> +
> /**
> - * Construct.
> + * Return the minified version for a given resource name.
> + * For example '/css/coolTheme.css' becomes
> '/css/coolTheme.min.css'
> + *
> + * @param name
> + * The original resource name
> + * @param minPostfix
> + * The postfix to use for minified name
> + * @return The minified resource name
> */
>
> - private ResourceUtils()
>
Why did you remove the private constructor ?
Is this class supposed to be instantiated now ?
> + public static String getMinifiedName(String name, String
> minPostfix)
> {
> + String minifiedName;
> + int idxOfExtension = name.lastIndexOf('.');
> + final String dottedPostfix = "." + minPostfix;
> +
> + if (idxOfExtension > -1)
> + {
> + String extension = name.substring(idxOfExtension);
> + final String baseName = name.substring(0,
> name.length() - extension.length() + 1);
> + if (!dottedPostfix.equals(extension) &&
> !baseName.endsWith(dottedPostfix + "."))
> + {
> + minifiedName = baseName + minPostfix +
> extension;
> + } else
> + {
> + minifiedName = name;
> + }
> + } else
> + {
> + minifiedName = name + dottedPostfix;
+ }
> + return minifiedName;
> }
> -
> +
> /**
> * Extract the locale from the filename
> *
>
>
