From 53be0f8db7688d0cd6c5a743897b1cc7be293a89 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 30 Jun 2014 14:19:03 +0100 Subject: [PATCH] Fix Gradle plugin logic for removing provided artifacts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new ResolvedArtifact-based minus implementation was checking the wrong Set when deciding whether or not a ResolvedArtifact should be included in the result. This was leading to provided dependencies, that should have only been packaging in WEB-INF/lib-provided also being packaged in WEB-INF/lib. The WarPackaging tests have been updated. In addition to checking that WEB-INF/lib-provided does not contain anything that’s unexpected, they also verify the contents of WEB-INF/lib Fixes #1187 --- .../boot/gradle/WarPackagingTests.java | 95 +++++++++++++------ .../gradle/repackage/ProjectLibraries.java | 2 +- 2 files changed, 66 insertions(+), 31 deletions(-) diff --git a/spring-boot-integration-tests/src/test/java/org/springframework/boot/gradle/WarPackagingTests.java b/spring-boot-integration-tests/src/test/java/org/springframework/boot/gradle/WarPackagingTests.java index c29c1cd33c..86b29f7b91 100644 --- a/spring-boot-integration-tests/src/test/java/org/springframework/boot/gradle/WarPackagingTests.java +++ b/spring-boot-integration-tests/src/test/java/org/springframework/boot/gradle/WarPackagingTests.java @@ -37,40 +37,27 @@ import static org.junit.Assert.assertTrue; /** * Tests for war packaging with Gradle to ensure that only the Servlet container and its * dependencies are packaged in WEB-INF/lib-provided - * + * * @author Andy Wilkinson */ public class WarPackagingTests { private static final String WEB_INF_LIB_PROVIDED_PREFIX = "WEB-INF/lib-provided/"; + private static final String WEB_INF_LIB_PREFIX = "WEB-INF/lib/"; + private static final Set TOMCAT_EXPECTED_IN_WEB_INF_LIB_PROVIDED = new HashSet( - Arrays.asList(WEB_INF_LIB_PROVIDED_PREFIX + "spring-boot-starter-tomcat-", - WEB_INF_LIB_PROVIDED_PREFIX + "tomcat-embed-core-", - WEB_INF_LIB_PROVIDED_PREFIX + "tomcat-embed-el-", - WEB_INF_LIB_PROVIDED_PREFIX + "tomcat-embed-logging-juli-")); + Arrays.asList("spring-boot-starter-tomcat-", "tomcat-embed-core-", + "tomcat-embed-el-", "tomcat-embed-logging-juli-")); private static final Set JETTY_EXPECTED_IN_WEB_INF_LIB_PROVIDED = new HashSet( - Arrays.asList(WEB_INF_LIB_PROVIDED_PREFIX + "spring-boot-starter-jetty-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-util-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-xml-", - WEB_INF_LIB_PROVIDED_PREFIX + "javax.servlet-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-continuation-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-io-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-http-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-server-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-security-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-servlet-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-webapp-", - WEB_INF_LIB_PROVIDED_PREFIX + "javax.servlet.jsp-", - WEB_INF_LIB_PROVIDED_PREFIX + "org.apache.jasper.glassfish-", - WEB_INF_LIB_PROVIDED_PREFIX + "javax.servlet.jsp.jstl-", - WEB_INF_LIB_PROVIDED_PREFIX - + "org.apache.taglibs.standard.glassfish-", - WEB_INF_LIB_PROVIDED_PREFIX + "javax.el-", - WEB_INF_LIB_PROVIDED_PREFIX + "com.sun.el-", - WEB_INF_LIB_PROVIDED_PREFIX + "org.eclipse.jdt.core-", - WEB_INF_LIB_PROVIDED_PREFIX + "jetty-jsp-")); + Arrays.asList("spring-boot-starter-jetty-", "jetty-util-", "jetty-xml-", + "javax.servlet-", "jetty-continuation-", "jetty-io-", "jetty-http-", + "jetty-server-", "jetty-security-", "jetty-servlet-", + "jetty-webapp-", "javax.servlet.jsp-", + "org.apache.jasper.glassfish-", "javax.servlet.jsp.jstl-", + "org.apache.taglibs.standard.glassfish-", "javax.el-", "com.sun.el-", + "org.eclipse.jdt.core-", "jetty-jsp-")); private static final String BOOT_VERSION = ManagedDependencies.get() .find("spring-boot").getVersion(); @@ -84,24 +71,32 @@ public class WarPackagingTests { @Test public void onlyTomcatIsPackackedInWebInfLibProvided() throws IOException { - checkWebInfLibProvidedEntriesForServletContainer("tomcat", + checkWebInfEntriesForServletContainer("tomcat", TOMCAT_EXPECTED_IN_WEB_INF_LIB_PROVIDED); } @Test public void onlyJettyIsPackackedInWebInfLibProvided() throws IOException { - checkWebInfLibProvidedEntriesForServletContainer("jetty", + checkWebInfEntriesForServletContainer("jetty", JETTY_EXPECTED_IN_WEB_INF_LIB_PROVIDED); } - private void checkWebInfLibProvidedEntriesForServletContainer( - String servletContainer, Set expectedEntries) throws IOException { + private void checkWebInfEntriesForServletContainer(String servletContainer, + Set expectedLibProvidedEntries) throws IOException { project.newBuild() .forTasks("clean", "build") .withArguments("-PbootVersion=" + BOOT_VERSION, "-PservletContainer=" + servletContainer).run(); JarFile war = new JarFile("target/war-packaging/build/libs/war-packaging.war"); + + checkWebInfLibProvidedEntries(war, expectedLibProvidedEntries); + + checkWebInfLibEntries(war, expectedLibProvidedEntries); + } + + private void checkWebInfLibProvidedEntries(JarFile war, Set expectedEntries) + throws IOException { Set entries = getWebInfLibProvidedEntries(war); assertEquals( @@ -118,6 +113,21 @@ public class WarPackagingTests { + unexpectedLibProvidedEntries, unexpectedLibProvidedEntries.isEmpty()); } + private void checkWebInfLibEntries(JarFile war, Set entriesOnlyInLibProvided) + throws IOException { + Set entries = getWebInfLibEntries(war); + + List unexpectedLibEntries = new ArrayList(); + for (String entry : entries) { + if (!isExpectedInWebInfLib(entry, entriesOnlyInLibProvided)) { + unexpectedLibEntries.add(entry); + } + } + + assertTrue("Found unexpected entries in WEB-INF/lib: " + unexpectedLibEntries, + unexpectedLibEntries.isEmpty()); + } + private Set getWebInfLibProvidedEntries(JarFile war) throws IOException { Set webInfLibProvidedEntries = new HashSet(); Enumeration entries = war.entries(); @@ -130,17 +140,42 @@ public class WarPackagingTests { return webInfLibProvidedEntries; } + private Set getWebInfLibEntries(JarFile war) throws IOException { + Set webInfLibEntries = new HashSet(); + Enumeration entries = war.entries(); + while (entries.hasMoreElements()) { + String name = entries.nextElement().getName(); + if (isWebInfLibEntry(name)) { + webInfLibEntries.add(name); + } + } + return webInfLibEntries; + } + private boolean isWebInfLibProvidedEntry(String name) { return name.startsWith(WEB_INF_LIB_PROVIDED_PREFIX) && !name.equals(WEB_INF_LIB_PROVIDED_PREFIX); } + private boolean isWebInfLibEntry(String name) { + return name.startsWith(WEB_INF_LIB_PREFIX) && !name.equals(WEB_INF_LIB_PREFIX); + } + private boolean isExpectedInWebInfLibProvided(String name, Set expectedEntries) { for (String expected : expectedEntries) { - if (name.startsWith(expected)) { + if (name.startsWith(WEB_INF_LIB_PROVIDED_PREFIX + expected)) { return true; } } return false; } + + private boolean isExpectedInWebInfLib(String name, Set unexpectedEntries) { + for (String unexpected : unexpectedEntries) { + if (name.startsWith(WEB_INF_LIB_PREFIX + unexpected)) { + return false; + } + } + return true; + } } diff --git a/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/repackage/ProjectLibraries.java b/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/repackage/ProjectLibraries.java index 0394ef61a3..f1496bb476 100644 --- a/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/repackage/ProjectLibraries.java +++ b/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/repackage/ProjectLibraries.java @@ -112,7 +112,7 @@ class ProjectLibraries implements Libraries { } Set result = new LinkedHashSet(); for (ResolvedArtifact artifact : source) { - if (!toRemove.contains(artifact.getFile())) { + if (!filesToRemove.contains(artifact.getFile())) { result.add(artifact); } }