Descriptor cleanup

+ Sanity check Resource being provided
+ Descriptor toString changed to include full URI to descriptor
  so that it can be used consistently in Logging and Exception messages.
+ New Descriptor.toURI() for use in Servlet Source and other logging messages
+ Servlet Source value is now the URI of the Descriptor

jetty-ee10/jetty-ee10-annotations/src/test/java/org/eclipse/jetty/ee10/annotations/TestAnnotationDecorator.java

@@ -23,30 +23,27 @@
 import org.eclipse.jetty.ee10.webapp.WebAppContext;
 import org.eclipse.jetty.ee10.webapp.WebDescriptor;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
-import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
 import org.eclipse.jetty.util.DecoratedObjectFactory;
 import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.util.resource.ResourceFactory;
 import org.eclipse.jetty.xml.XmlParser;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-@ExtendWith(WorkDirExtension.class)
 public class TestAnnotationDecorator
 {
     public WorkDir workDir;
 
     public class TestWebDescriptor extends WebDescriptor
     {
-        public TestWebDescriptor(Resource resource, MetaData.Complete metadata)
+        public TestWebDescriptor(Resource xml, MetaData.Complete metadata)
         {
-            super(resource);
+            super(xml);
             _metaDataComplete = metadata;
         }
 
@@ -86,9 +83,10 @@ public int getMinorVersion()
     @Test
     public void testAnnotationDecorator() throws Exception
     {
-        Path dummyXml = workDir.getEmptyPathDir().resolve("dummy.xml");
-        Files.createFile(dummyXml);
-        Resource dummyXmlResource = ResourceFactory.root().newResource(dummyXml);
+        Path docroot = workDir.getEmptyPathDir();
+        Path dummyDescriptor = docroot.resolve("dummy.xml");
+        Files.createFile(dummyDescriptor);
+        Resource dummyResource = ResourceFactory.root().newResource(dummyDescriptor);
 
         assertThrows(NullPointerException.class, () ->
         {
@@ -108,7 +106,7 @@ public void testAnnotationDecorator() throws Exception
         context.removeAttribute(LifeCycleCallbackCollection.LIFECYCLE_CALLBACK_COLLECTION);
 
         //test with BaseHolder metadata, should not introspect with metdata-complete==true
-        context.getMetaData().setWebDescriptor(new TestWebDescriptor(dummyXmlResource, MetaData.Complete.True));
+        context.getMetaData().setWebDescriptor(new TestWebDescriptor(dummyResource, MetaData.Complete.True));
         assertTrue(context.getMetaData().isMetaDataComplete());
         ServletHolder holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, ""));
         holder.setHeldClass(ServletE.class);
@@ -124,7 +122,7 @@ public void testAnnotationDecorator() throws Exception
         context.removeAttribute(LifeCycleCallbackCollection.LIFECYCLE_CALLBACK_COLLECTION);
 
         //test with BaseHolder metadata, should introspect with metadata-complete==false
-        context.getMetaData().setWebDescriptor(new TestWebDescriptor(dummyXmlResource, MetaData.Complete.False));
+        context.getMetaData().setWebDescriptor(new TestWebDescriptor(dummyResource, MetaData.Complete.False));
         DecoratedObjectFactory.associateInfo(holder);
         decorator = new AnnotationDecorator(context);
         decorator.decorate(servlet);

jetty-ee10/jetty-ee10-annotations/src/test/java/org/eclipse/jetty/ee10/annotations/TestAnnotationIntrospector.java

@@ -13,7 +13,7 @@
 
 package org.eclipse.jetty.ee10.annotations;
 
-import java.io.File;
+import java.nio.file.Path;
 
 import org.eclipse.jetty.ee10.servlet.ServletHolder;
 import org.eclipse.jetty.ee10.servlet.Source;
@@ -23,7 +23,6 @@
 import org.eclipse.jetty.logging.StacklessLogging;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.eclipse.jetty.util.resource.Resource;
-import org.eclipse.jetty.util.resource.ResourceFactory;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -63,31 +62,33 @@ public void testIsIntrospectable() throws Exception
             assertTrue(introspector.isIntrospectable(new ServletE(), holder));
 
             //a DESCRIPTOR sourced servlet can be introspected if web.xml metdata-complete==false
-            File file = MavenTestingUtils.getTestResourceFile("web31false.xml");
-            Resource resource = ResourceFactory.root().newResource(file.toPath());
-            wac.getMetaData().setWebDescriptor(new WebDescriptor(resource));
-            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, resource.toString()));
+            Path xml = MavenTestingUtils.getTestResourcePathFile("web31false.xml");
+            Resource xmlResource = wac.getResourceFactory().newResource(xml);
+            wac.getMetaData().setWebDescriptor(new WebDescriptor(xmlResource));
+            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, xml.toUri().toASCIIString()));
             assertTrue(introspector.isIntrospectable(new ServletE(), holder));
 
             //a DESCRIPTOR sourced servlet can be introspected if web-fragment.xml medata-complete==false && web.xml metadata-complete==false
-            file = MavenTestingUtils.getTestResourceFile("web-fragment4false.xml");
-            resource = ResourceFactory.root().newResource(file.toPath());
-            wac.getMetaData().addFragmentDescriptor(ResourceFactory.root().newResource(file.getParentFile().toPath()), new FragmentDescriptor(resource));
-            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, resource.toString()));
+            xml = MavenTestingUtils.getTestResourcePathFile("web-fragment4false.xml");
+            xmlResource = wac.getResourceFactory().newResource(xml);
+            Resource parent = wac.getResourceFactory().newResource(xml.getParent());
+            wac.getMetaData().addFragmentDescriptor(parent, new FragmentDescriptor(xmlResource));
+            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, xml.toUri().toASCIIString()));
             assertTrue(introspector.isIntrospectable(new ServletE(), holder));
 
             //a DESCRIPTOR sourced servlet cannot be introspected if web-fragment.xml medata-complete==true (&& web.xml metadata-complete==false)
-            file = MavenTestingUtils.getTestResourceFile("web-fragment4true.xml");
-            resource = ResourceFactory.root().newResource(file.toPath());
-            wac.getMetaData().addFragmentDescriptor(ResourceFactory.root().newResource(file.getParentFile().toPath()), new FragmentDescriptor(resource));
-            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, resource.toString()));
+            xml = MavenTestingUtils.getTestResourcePathFile("web-fragment4true.xml");
+            xmlResource = wac.getResourceFactory().newResource(xml);
+            parent = wac.getResourceFactory().newResource(xml.getParent());
+            wac.getMetaData().addFragmentDescriptor(parent, new FragmentDescriptor(xmlResource));
+            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, xml.toUri().toASCIIString()));
             assertFalse(introspector.isIntrospectable(new ServletE(), holder));
 
             //a DESCRIPTOR sourced servlet cannot be introspected if web.xml medata-complete==true
-            file = MavenTestingUtils.getTestResourceFile("web31true.xml");
-            resource = ResourceFactory.root().newResource(file.toPath());
-            wac.getMetaData().setWebDescriptor(new WebDescriptor(resource));
-            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, resource.toString()));
+            xml = MavenTestingUtils.getTestResourcePathFile("web31true.xml");
+            xmlResource = wac.getResourceFactory().newResource(xml);
+            wac.getMetaData().setWebDescriptor(new WebDescriptor(xmlResource));
+            holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR, xml.toUri().toASCIIString()));
             assertFalse(introspector.isIntrospectable(new ServletE(), holder));
         }
     }

jetty-ee10/jetty-ee10-annotations/src/test/java/org/eclipse/jetty/ee10/annotations/TestRunAsAnnotation.java

@@ -20,26 +20,18 @@
 import org.eclipse.jetty.ee10.webapp.WebAppContext;
 import org.eclipse.jetty.ee10.webapp.WebDescriptor;
 import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
-import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
-import org.eclipse.jetty.util.resource.Resource;
-import org.eclipse.jetty.util.resource.ResourceFactory;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 
-@ExtendWith(WorkDirExtension.class)
 public class TestRunAsAnnotation
 {
     public WorkDir workDir;
 
     @Test
     public void testRunAsAnnotation() throws Exception
     {
-        Path dummyXml = workDir.getEmptyPathDir().resolve("dummy.xml");
-        Files.createFile(dummyXml);
-        Resource dummyXmlResource = ResourceFactory.root().newResource(dummyXml);
-
         WebAppContext wac = new WebAppContext();
 
         //pre-add a servlet but not by descriptor
@@ -55,14 +47,16 @@ public void testRunAsAnnotation() throws Exception
         holder2.setHeldClass(ServletC.class);
         holder2.setInitOrder(1);
         wac.getServletHandler().addServletWithMapping(holder2, "/foo2/*");
-        wac.getMetaData().setOrigin(holder2.getName() + ".servlet.run-as", new WebDescriptor(dummyXmlResource));
+        Path fakeXml = workDir.getEmptyPathDir().resolve("fake.xml");
+        Files.createFile(fakeXml);
+        wac.getMetaData().setOrigin(holder2.getName() + ".servlet.run-as", new WebDescriptor(wac.getResourceFactory().newResource(fakeXml)));
 
         AnnotationIntrospector parser = new AnnotationIntrospector(wac);
         RunAsAnnotationHandler handler = new RunAsAnnotationHandler(wac);
         parser.registerHandler(handler);
         parser.introspect(new ServletC(), null);
 
         assertEquals("admin", holder.getRunAsRole());
-        assertEquals(null, holder2.getRunAsRole());
+        assertNull(holder2.getRunAsRole());
     }
 }

jetty-ee10/jetty-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/PlusDescriptorProcessor.java

@@ -142,7 +142,7 @@ public void visitEnvEntry(WebAppContext context, Descriptor descriptor, XmlParse
             case WebFragment:
             {
                 //ServletSpec p.75. No declaration in web.xml, but in multiple web-fragments. Error.
-                throw new IllegalStateException("Conflicting env-entry " + name + " in " + descriptor.getResource());
+                throw new IllegalStateException("Conflicting env-entry " + name + " in " + descriptor);
             }
             default:
                 break;
@@ -293,7 +293,7 @@ public void visitResourceRef(WebAppContext context, Descriptor descriptor, XmlPa
 
                     //ServletSpec p.75. No declaration of resource-ref in web xml, but different in multiple web-fragments. Error.
                     if (!type.equals(otherType) || !auth.equals(otherAuth) || !shared.equals(otherShared))
-                        throw new IllegalStateException("Conflicting resource-ref " + jndiName + " in " + descriptor.getResource());
+                        throw new IllegalStateException("Conflicting resource-ref " + jndiName + " in " + descriptor);
                     //same in multiple web-fragments, merge the injections
                     addInjections(context, descriptor, node, jndiName, TypeUtil.fromName(type));
                 }
@@ -401,7 +401,7 @@ public void visitResourceEnvRef(WebAppContext context, Descriptor descriptor, Xm
 
                     //ServletSpec p.75. No declaration of resource-ref in web xml, but different in multiple web-fragments. Error.
                     if (!type.equals(otherType))
-                        throw new IllegalStateException("Conflicting resource-env-ref " + jndiName + " in " + descriptor.getResource());
+                        throw new IllegalStateException("Conflicting resource-env-ref " + jndiName + " in " + descriptor);
 
                     //same in multiple web-fragments, merge the injections
                     addInjections(context, descriptor, node, jndiName, TypeUtil.fromName(type));
@@ -503,7 +503,7 @@ public void visitMessageDestinationRef(WebAppContext context, Descriptor descrip
                     type = (type == null ? "" : type);
                     usage = (usage == null ? "" : usage);
                     if (!type.equals(otherType) || !usage.equalsIgnoreCase(otherUsage))
-                        throw new IllegalStateException("Conflicting message-destination-ref " + jndiName + " in " + descriptor.getResource());
+                        throw new IllegalStateException("Conflicting message-destination-ref " + jndiName + " in " + descriptor);
 
                     //same in multiple web-fragments, merge the injections
                     addInjections(context, descriptor, node, jndiName, TypeUtil.fromName(type));

jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/Descriptor.java

@@ -15,8 +15,6 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.nio.file.Files;
-import java.nio.file.StandardOpenOption;
 import java.util.Objects;
 
 import org.eclipse.jetty.util.resource.Resource;
@@ -33,13 +31,11 @@ public abstract class Descriptor
     protected XmlParser.Node _root;
     protected String _dtd;
 
-    public Descriptor(Resource xml)
+    public Descriptor(Resource resource)
     {
-        _xml = Objects.requireNonNull(xml);
-        if (!_xml.exists())
-            throw new IllegalArgumentException("Descriptor does not exist: " + xml);
+        _xml = Objects.requireNonNull(resource, "Resource must not be null");
         if (_xml.isDirectory())
-            throw new IllegalArgumentException("Descriptor is not a file: " + xml);
+            throw new IllegalArgumentException("Descriptor cannot be a directory");
     }
 
     public void parse(XmlParser parser)
@@ -48,7 +44,7 @@ public void parse(XmlParser parser)
         if (_root == null)
         {
             Objects.requireNonNull(parser);
-            try (InputStream is = Files.newInputStream(_xml.getPath(), StandardOpenOption.READ))
+            try (InputStream is = _xml.newInputStream())
             {
                 _root = parser.parse(is);
                 _dtd = parser.getDTD();
@@ -61,9 +57,9 @@ public void parse(XmlParser parser)
         }
     }
 
-    public boolean isParsed()
+    public String getURI()
     {
-        return _root != null;
+        return _xml.getURI().toASCIIString();
     }
 
     public Resource getResource()
@@ -79,6 +75,6 @@ public XmlParser.Node getRoot()
     @Override
     public String toString()
     {
-        return this.getClass().getSimpleName() + "(" + _xml + ")";
+        return this.getClass().getSimpleName() + "(" + getURI() + ")";
     }
 }

jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/FragmentConfiguration.java

@@ -24,6 +24,9 @@
  */
 public class FragmentConfiguration extends AbstractConfiguration
 {
+    // Holds a Map<Resource, Resource> .
+    //  key: Resource to the Jar
+    //  value: Resource to the web fragment xml
     public static final String FRAGMENT_RESOURCES = "org.eclipse.jetty.webFragments";
 
     public FragmentConfiguration()

jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/FragmentDescriptor.java

@@ -36,15 +36,13 @@ public enum OtherType
         None, Before, After
     }
 
-    ;
     protected OtherType _otherType = OtherType.None;
 
     protected List<String> _befores = new ArrayList<String>();
     protected List<String> _afters = new ArrayList<String>();
     protected String _name;
 
     public FragmentDescriptor(Resource xml)
-        throws Exception
     {
         super(xml);
     }
@@ -109,7 +107,7 @@ public void processBefores(XmlParser.Node ordering)
             if (node.getTag().equalsIgnoreCase("others"))
             {
                 if (_otherType != OtherType.None)
-                    throw new IllegalStateException("Duplicate <other> clause detected in " + _xml.getURI());
+                    throw new IllegalStateException("Duplicate <other> clause detected in " + _xml);
 
                 _otherType = OtherType.Before;
             }
@@ -136,7 +134,7 @@ public void processAfters(XmlParser.Node ordering)
             if (node.getTag().equalsIgnoreCase("others"))
             {
                 if (_otherType != OtherType.None)
-                    throw new IllegalStateException("Duplicate <other> clause detected in " + _xml.getURI());
+                    throw new IllegalStateException("Duplicate <other> clause detected in " + _xml);
 
                 _otherType = OtherType.After;
             }

jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaData.java

@@ -314,7 +314,7 @@ public void addFragmentDescriptor(Resource jarResource, FragmentDescriptor descr
             Descriptor existing = _webFragmentNameMap.get(descriptor.getName());
             if (existing != null && !isAllowDuplicateFragmentNames())
             {
-                throw new IllegalStateException("Duplicate fragment name: " + descriptor.getName() + " for " + existing.getResource() + " and " + descriptor.getResource());
+                throw new IllegalStateException("Duplicate fragment name: " + descriptor.getName() + " for " + existing.getURI() + " and " + descriptor.getURI());
             }
             else
                 _webFragmentNameMap.put(descriptor.getName(), descriptor);
@@ -597,7 +597,7 @@ public FragmentDescriptor getFragmentDescriptor(String name)
      */
     public FragmentDescriptor getFragmentDescriptor(Resource descriptorResource)
     {
-        return _webFragmentRoots.stream().filter(d -> d.getResource().equals(descriptorResource)).findFirst().orElse(null);
+        return _webFragmentRoots.stream().filter(d -> d.getRoot().equals(descriptorResource)).findFirst().orElse(null);
     }
 
     /**

jetty-ee10/jetty-ee10-webapp/src/main/java/org/eclipse/jetty/ee10/webapp/MetaInfConfiguration.java

@@ -376,10 +376,8 @@ public void scanJars(final WebAppContext context, Collection<Resource> jars, boo
      * @param context the context for the scan
      * @param target the target resource to scan for
      * @param cache the resource cache
-     * @throws Exception if unable to scan for resources
      */
     public void scanForResources(WebAppContext context, Resource target, ConcurrentHashMap<Resource, Resource> cache)
-        throws Exception
     {
         Resource resourcesDir = null;
         if (cache != null && cache.containsKey(target))
@@ -450,10 +448,8 @@ private static boolean isEmptyResource(Resource resourcesDir)
      * @param context the context for the scan
      * @param jar the jar resource to scan for fragements in
      * @param cache the resource cache
-     * @throws Exception if unable to scan for fragments
      */
     public void scanForFragment(WebAppContext context, Resource jar, ConcurrentHashMap<Resource, Resource> cache)
-        throws Exception
     {
         Resource webFrag = null;
         if (cache != null && cache.containsKey(jar))
@@ -485,7 +481,7 @@ else if (LOG.isDebugEnabled())
 
             if (cache != null)
             {
-                //web-fragment.xml doesn't exist: put token in cache to signal we've seen the jar               
+                //web-fragment.xml doesn't exist: put token in cache to signal we've seen the jar
                 Resource old = cache.putIfAbsent(jar, webFrag);
                 if (old != null)
                     webFrag = old;
@@ -569,7 +565,6 @@ public void scanForTlds(WebAppContext context, Resource jar, ConcurrentHashMap<R
                 return;
         }
 
-        // TODO do we want to keep a collection of URLs or should that be changed to a collection of URIs?
         Collection<URL> metaInfTlds = (Collection<URL>)context.getAttribute(METAINF_TLDS);
         if (metaInfTlds == null)
         {
@@ -692,7 +687,7 @@ protected List<Resource> findWebInfLibJars(WebAppContext context)
         throws Exception
     {
         Resource webInf = context.getWebInf();
-        if (webInf == null)
+        if (webInf == null || !webInf.exists() || webInf.isDirectory())
             return null;
 
         Resource webInfLib = webInf.resolve("/lib");