On 04/12/14 21:48, Martin Grigorov wrote:
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.
Just for performance reasons. Do you think it's neglectable?
+ }
+
+ /**
+ * 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 ...
Ok, better make it final....
+ {
+ 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.
Ok
+ @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
...
Ok.
+ {
+ 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 ?
Well, actually I don't need WicketTester for my tests, I just need to
have a valid Application in the ThreadContext. I will remove static
variable and if I don't find better solutions I will use the classic
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
ok
+
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 ?
No no, it's not intended to be instantiated. I just realized now that
this is a practice to prevent "static" classes from being instantiated.
I will revert this change.
+ 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
*
