You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2019/06/05 13:33:23 UTC
[sling-org-apache-sling-scripting-sightly] branch master updated:
SLING-6779 - The HTL compiler and Maven Plugin should warn when using
potentially invalid options
This is an automated email from the ASF dual-hosted git repository.
radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly.git
The following commit(s) were added to refs/heads/master by this push:
new cb72815 SLING-6779 - The HTL compiler and Maven Plugin should warn when using potentially invalid options
cb72815 is described below
commit cb72815369a9ad0757875d81c85b59c413c23f08
Author: Radu Cotescu <17...@users.noreply.github.com>
AuthorDate: Wed Jun 5 15:33:18 2019 +0200
SLING-6779 - The HTL compiler and Maven Plugin should warn when using potentially invalid options
* updated HTL compiler
* enhanced the HTL script engine to allow it to configure the compiler for additional options
---
pom.xml | 22 +++++----
.../impl/engine/SightlyEngineConfiguration.java | 15 ++++++
.../engine/compiled/SlingHTLMasterCompiler.java | 1 +
.../engine/extension/FormatFilterExtension.java | 4 --
.../impl/engine/SightlyCompiledScriptTest.java | 10 ++--
.../compiled/SlingHTLMasterCompilerTest.java | 56 +++++++++-------------
6 files changed, 59 insertions(+), 49 deletions(-)
diff --git a/pom.xml b/pom.xml
index 0966295..6278094 100644
--- a/pom.xml
+++ b/pom.xml
@@ -37,7 +37,7 @@
The versioning scheme defined here corresponds to SLING-7406 (<module_version>-<htl_specification_version>). Take care when
releasing to only increase the first part, unless the module provides support for a newer version of the HTL specification.
-->
- <version>1.1.3-1.4.0-SNAPSHOT</version>
+ <version>1.2.0-1.4.0-SNAPSHOT</version>
<packaging>bundle</packaging>
<name>Apache Sling Scripting HTL Engine</name>
@@ -103,9 +103,9 @@
</configuration>
</plugin>
<plugin>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>findbugs-maven-plugin</artifactId>
- <version>3.0.5</version>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-maven-plugin</artifactId>
+ <version>3.1.11</version>
<configuration>
<effort>Max</effort>
<xmlOutput>true</xmlOutput>
@@ -187,7 +187,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.scripting.sightly.compiler</artifactId>
- <version>1.1.3-1.4.0-SNAPSHOT</version>
+ <version>1.2.0-1.4.0-SNAPSHOT</version>
<scope>provided</scope>
</dependency>
<dependency>
@@ -334,13 +334,19 @@
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
- <version>1.9.5</version>
+ <version>2.25.1</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-api-mockito2</artifactId>
+ <version>2.0.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.powermock</groupId>
- <artifactId>powermock-reflect</artifactId>
- <version>1.6.5</version>
+ <artifactId>powermock-module-junit4</artifactId>
+ <version>2.0.2</version>
<scope>test</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyEngineConfiguration.java b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyEngineConfiguration.java
index d1220cc..8142cc4 100644
--- a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyEngineConfiguration.java
+++ b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/SightlyEngineConfiguration.java
@@ -21,6 +21,9 @@ package org.apache.sling.scripting.sightly.impl.engine;
import java.io.IOException;
import java.io.InputStream;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
import java.util.jar.Attributes;
import java.util.jar.Manifest;
@@ -48,11 +51,18 @@ public class SightlyEngineConfiguration {
)
boolean keepGenerated() default true;
+ @AttributeDefinition(
+ name = "Known Expression Options",
+ description = "A list of extra expression options that should be ignored by the HTL compiler when reporting unknown options."
+ )
+ String[] allowedExpressionOptions();
+
}
private String engineVersion = "0";
private boolean keepGenerated;
private String bundleSymbolicName = "org.apache.sling.scripting.sightly";
+ private Set<String> allowedExpressionOptions;
public String getEngineVersion() {
return engineVersion;
@@ -70,6 +80,10 @@ public class SightlyEngineConfiguration {
return keepGenerated;
}
+ public Set<String> getAllowedExpressionOptions() {
+ return allowedExpressionOptions;
+ }
+
@Activate
protected void activate(Configuration configuration) {
InputStream ins = null;
@@ -97,5 +111,6 @@ public class SightlyEngineConfiguration {
}
}
keepGenerated = configuration.keepGenerated();
+ allowedExpressionOptions = new HashSet<>(Arrays.asList(configuration.allowedExpressionOptions()));
}
}
diff --git a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompiler.java b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompiler.java
index b004579..bdd1f7d 100644
--- a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompiler.java
+++ b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompiler.java
@@ -158,6 +158,7 @@ public class SlingHTLMasterCompiler {
}
}
}
+ sightlyCompiler = SightlyCompiler.withKnownExpressionOptions(sightlyEngineConfiguration.getAllowedExpressionOptions());
}
/**
diff --git a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/FormatFilterExtension.java b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/FormatFilterExtension.java
index bd7d2b8..ce3245f 100644
--- a/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/FormatFilterExtension.java
+++ b/src/main/java/org/apache/sling/scripting/sightly/impl/engine/extension/FormatFilterExtension.java
@@ -50,7 +50,6 @@ public class FormatFilterExtension implements RuntimeExtension {
private static final String FORMAT_OPTION = "format";
private static final String TYPE_OPTION = "type";
private static final String LOCALE_OPTION = "locale";
- private static final String FORMAT_LOCALE_OPTION = "formatLocale";
private static final String TIMEZONE_OPTION = "timezone";
private static final String DATE_FORMAT_TYPE = "date";
@@ -90,9 +89,6 @@ public class FormatFilterExtension implements RuntimeExtension {
if (options.containsKey(LOCALE_OPTION)) {
localeOption = runtimeObjectModel.toString(options.get(LOCALE_OPTION));
}
- if (localeOption == null && options.containsKey(FORMAT_LOCALE_OPTION)) {
- localeOption = runtimeObjectModel.toString(options.get(FORMAT_LOCALE_OPTION));
- }
if (StringUtils.isNotBlank(localeOption)) {
return LocaleUtils.toLocale(localeOption);
}
diff --git a/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyCompiledScriptTest.java b/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyCompiledScriptTest.java
index 9fc3b03..2ae50d8 100644
--- a/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyCompiledScriptTest.java
+++ b/src/test/java/org/apache/sling/scripting/sightly/impl/engine/SightlyCompiledScriptTest.java
@@ -21,9 +21,9 @@ import java.util.HashMap;
import java.util.Hashtable;
import java.util.List;
import java.util.Map;
+
import javax.script.Bindings;
import javax.script.ScriptContext;
-import javax.script.ScriptEngine;
import javax.script.ScriptException;
import javax.script.SimpleBindings;
@@ -37,11 +37,15 @@ import org.apache.sling.testing.mock.sling.servlet.MockSlingHttpServletRequest;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
-import org.mockito.internal.util.reflection.Whitebox;
import org.osgi.framework.BundleContext;
+import org.powermock.reflect.Whitebox;
import static org.junit.Assert.assertEquals;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
public class SightlyCompiledScriptTest {
diff --git a/src/test/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompilerTest.java b/src/test/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompilerTest.java
index ed85dca..719a44d 100644
--- a/src/test/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompilerTest.java
+++ b/src/test/java/org/apache/sling/scripting/sightly/impl/engine/compiled/SlingHTLMasterCompilerTest.java
@@ -32,10 +32,10 @@ import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.commons.classloader.ClassLoaderWriter;
import org.apache.sling.commons.compiler.CompilationResult;
import org.apache.sling.commons.compiler.CompilationUnit;
-import org.apache.sling.commons.compiler.CompilerMessage;
import org.apache.sling.commons.compiler.JavaCompiler;
import org.apache.sling.commons.compiler.Options;
import org.apache.sling.scripting.api.resource.ScriptingResourceResolverProvider;
+import org.apache.sling.scripting.sightly.compiler.SightlyCompiler;
import org.apache.sling.scripting.sightly.impl.compiler.MockPojo;
import org.apache.sling.scripting.sightly.impl.engine.ResourceBackedPojoChangeMonitor;
import org.apache.sling.scripting.sightly.impl.engine.SightlyEngineConfiguration;
@@ -45,15 +45,13 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
import org.powermock.reflect.Whitebox;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
-import static org.mockito.Matchers.anyString;
+import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -85,15 +83,18 @@ public class SlingHTLMasterCompilerTest {
compiler = spy(new SlingHTLMasterCompiler());
javaCompiler = mock(JavaCompiler.class);
+ SightlyCompiler sightlyCompiler = new SightlyCompiler();
+ classLoaderWriter = Mockito.mock(ClassLoaderWriter.class);
+ ClassLoader classLoader = Mockito.mock(ClassLoader.class);
+ when(classLoaderWriter.getClassLoader()).thenReturn(classLoader);
resourceBackedPojoChangeMonitor = spy(new ResourceBackedPojoChangeMonitor());
Whitebox.setInternalState(compiler, "sightlyEngineConfiguration", sightlyEngineConfiguration);
Whitebox.setInternalState(compiler, "resourceBackedPojoChangeMonitor", resourceBackedPojoChangeMonitor);
Whitebox.setInternalState(compiler, "javaCompiler", javaCompiler);
- classLoaderWriter = Mockito.mock(ClassLoaderWriter.class);
- ClassLoader classLoader = Mockito.mock(ClassLoader.class);
- when(classLoaderWriter.getClassLoader()).thenReturn(classLoader);
+ Whitebox.setInternalState(compiler, "sightlyCompiler", sightlyCompiler);
+ Whitebox.setInternalState(compiler, "classLoaderWriter", classLoaderWriter);
}
@After
@@ -106,21 +107,17 @@ public class SlingHTLMasterCompilerTest {
@Test
public void testActivateNoPreviousInfo() {
- SlingHTLMasterCompiler slingHTLMasterCompiler = new SlingHTLMasterCompiler();
ClassLoaderWriter classLoaderWriter = mock(ClassLoaderWriter.class);
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
when(classLoaderWriter.getOutputStream(SlingHTLMasterCompiler.SIGHTLY_CONFIG_FILE)).thenReturn(outputStream);
- Whitebox.setInternalState(slingHTLMasterCompiler, "classLoaderWriter", classLoaderWriter);
- Whitebox.setInternalState(slingHTLMasterCompiler, "sightlyEngineConfiguration", sightlyEngineConfiguration);
- Whitebox.setInternalState(slingHTLMasterCompiler, "scriptingResourceResolverProvider", scriptingResourceResolverProvider);
- slingHTLMasterCompiler.activate();
+ Whitebox.setInternalState(compiler, "classLoaderWriter", classLoaderWriter);
+ compiler.activate();
verify(classLoaderWriter).delete(sightlyEngineConfiguration.getScratchFolder());
assertEquals("1.0.17-SNAPSHOT", outputStream.toString());
}
@Test
public void testActivateOverPreviousVersion() {
- SlingHTLMasterCompiler slingHTLMasterCompiler = new SlingHTLMasterCompiler();
ClassLoaderWriter classLoaderWriter = mock(ClassLoaderWriter.class);
try {
when(classLoaderWriter.getInputStream(SlingHTLMasterCompiler.SIGHTLY_CONFIG_FILE))
@@ -131,17 +128,14 @@ public class SlingHTLMasterCompilerTest {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
when(classLoaderWriter.getOutputStream(SlingHTLMasterCompiler.SIGHTLY_CONFIG_FILE)).thenReturn(outputStream);
when(classLoaderWriter.delete(sightlyEngineConfiguration.getScratchFolder())).thenReturn(true);
- Whitebox.setInternalState(slingHTLMasterCompiler, "classLoaderWriter", classLoaderWriter);
- Whitebox.setInternalState(slingHTLMasterCompiler, "sightlyEngineConfiguration", sightlyEngineConfiguration);
- Whitebox.setInternalState(slingHTLMasterCompiler, "scriptingResourceResolverProvider", scriptingResourceResolverProvider);
- slingHTLMasterCompiler.activate();
+ Whitebox.setInternalState(compiler, "classLoaderWriter", classLoaderWriter);
+ compiler.activate();
verify(classLoaderWriter).delete(sightlyEngineConfiguration.getScratchFolder());
assertEquals("1.0.17-SNAPSHOT", outputStream.toString());
}
@Test
public void testActivateOverSameVersion() {
- SlingHTLMasterCompiler slingHTLMasterCompiler = new SlingHTLMasterCompiler();
ClassLoaderWriter classLoaderWriter = mock(ClassLoaderWriter.class);
try {
when(classLoaderWriter.getInputStream(SlingHTLMasterCompiler.SIGHTLY_CONFIG_FILE))
@@ -152,10 +146,8 @@ public class SlingHTLMasterCompilerTest {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
ByteArrayOutputStream spyOutputStream = spy(outputStream);
when(classLoaderWriter.getOutputStream(SlingHTLMasterCompiler.SIGHTLY_CONFIG_FILE)).thenReturn(spyOutputStream);
- Whitebox.setInternalState(slingHTLMasterCompiler, "classLoaderWriter", classLoaderWriter);
- Whitebox.setInternalState(slingHTLMasterCompiler, "sightlyEngineConfiguration", sightlyEngineConfiguration);
- Whitebox.setInternalState(slingHTLMasterCompiler, "scriptingResourceResolverProvider", scriptingResourceResolverProvider);
- slingHTLMasterCompiler.activate();
+ Whitebox.setInternalState(compiler, "classLoaderWriter", classLoaderWriter);
+ compiler.activate();
verify(classLoaderWriter, never()).delete(sightlyEngineConfiguration.getScratchFolder());
try {
verify(spyOutputStream, never()).write(any(byte[].class));
@@ -208,7 +200,7 @@ public class SlingHTLMasterCompilerTest {
* assuming the compiled class has a last modified date greater than the source, then the compiler should not recompile the Use
* object
*/
- verify(compiler).getResourceBackedUseObject(any(RenderContext.class), anyString());
+ verify(compiler).getResourceBackedUseObject(any(RenderContext.class), any(String.class));
}
@Test
@@ -250,7 +242,7 @@ public class SlingHTLMasterCompilerTest {
* assuming the compiled class has a last modified date greater than the source, then the compiler should not recompile the Use
* object
*/
- verify(javaCompiler, times(1)).compile(any(CompilationUnit[].class), any(Options.class));
+ verify(javaCompiler, times(1)).compile(any(CompilationUnit[].class), isNull());
}
@Test
@@ -277,19 +269,15 @@ public class SlingHTLMasterCompilerTest {
}
private void getInstancePojoTest(String className) throws Exception {
- RenderContextImpl renderContext = Mockito.mock(RenderContextImpl.class);
- CompilationResult compilationResult = Mockito.mock(CompilationResult.class);
- when(compilationResult.getErrors()).thenReturn(new ArrayList<CompilerMessage>());
- when(javaCompiler.compile(Mockito.any(CompilationUnit[].class), Mockito.any(Options.class))).thenReturn(compilationResult);
- when(classLoaderWriter.getClassLoader().loadClass(className)).thenAnswer(new Answer<Object>() {
- @Override
- public Object answer(InvocationOnMock invocation) {
- return MockPojo.class;
- }
- });
+ RenderContextImpl renderContext = mock(RenderContextImpl.class);
+ CompilationResult compilationResult = mock(CompilationResult.class);
+ when(compilationResult.getErrors()).thenReturn(new ArrayList<>());
+ when(javaCompiler.compile(any(CompilationUnit[].class), isNull())).thenReturn(compilationResult);
+ when(classLoaderWriter.getClassLoader().loadClass(className)).thenAnswer(invocationOnMock -> MockPojo.class);
Whitebox.setInternalState(compiler, "classLoaderWriter", classLoaderWriter);
Whitebox.setInternalState(compiler, "javaCompiler", javaCompiler);
Whitebox.setInternalState(compiler, "scriptingResourceResolverProvider", scriptingResourceResolverProvider);
+ Whitebox.setInternalState(compiler, "sightlyCompiler", new SightlyCompiler());
Object obj = compiler.getResourceBackedUseObject(renderContext, className);
assertTrue("Expected to obtain a " + MockPojo.class.getName() + " object.", obj instanceof MockPojo);
}