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/05/28 17:29:26 UTC
[sling-org-apache-sling-scripting-sightly] branch issue/SLING-6779
created (now 44c6151)
This is an automated email from the ASF dual-hosted git repository.
radu pushed a change to branch issue/SLING-6779
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly.git.
at 44c6151 SLING-6779 - The HTL compiler and Maven Plugin should warn when using potentially invalid options
This branch includes the following new commits:
new 44c6151 SLING-6779 - The HTL compiler and Maven Plugin should warn when using potentially invalid options
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[sling-org-apache-sling-scripting-sightly] 01/01: SLING-6779 - The
HTL compiler and Maven Plugin should warn when using potentially invalid
options
Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
radu pushed a commit to branch issue/SLING-6779
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly.git
commit 44c615111254e1b0173cd19b69b48a3e586fcd9b
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Tue May 28 19:25:06 2019 +0200
SLING-6779 - The HTL compiler and Maven Plugin should warn when using potentially invalid options
* added all known expression and plugin options to the compiler
* added the possibility to configure the compiler to ignore certain additional options
* enhanced the HTL Script Engine to allow it to configure the compiler for additional options
* enhanced the HTL Maven Plugin to rely on a new configuration option to pass down additional
options to the compiler
---
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);
}