Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jetty 12 : Descriptor cleanup #8611

Merged
merged 7 commits into from
Sep 28, 2022
Merged

Jetty 12 : Descriptor cleanup #8611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aa77c88
Descriptor cleanup
joakime Sep 22, 2022
663aab4
More cleanup around `Source` location/name
joakime Sep 22, 2022
6f30001
Fix compilation
joakime Sep 22, 2022
72ade0e
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/jetty-12-…
joakime Sep 23, 2022
cf0225b
Fixing TestAnnotationConfiguration
joakime Sep 23, 2022
fcb1d8c
Changes from review
joakime Sep 27, 2022
ac409aa
More changes from review
joakime Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ public void createServletContainerInitializerAnnotationHandlers(WebAppContext co
if (annotation != null)
classes = annotation.value();

DiscoveredServletContainerInitializerHolder holder = new DiscoveredServletContainerInitializerHolder(new Source(Origin.ANNOTATION, sci.getClass().getName()), sci);
DiscoveredServletContainerInitializerHolder holder = new DiscoveredServletContainerInitializerHolder(new Source(Origin.ANNOTATION, sci.getClass()), sci);
_sciHolders.add(holder);

if (classes.length > 0)
Expand Down
5 changes: 3 additions & 2 deletions ...-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/AnnotationIntrospector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.eclipse.jetty.ee10.servlet.Source.Origin;
import org.eclipse.jetty.ee10.webapp.WebAppContext;
import org.eclipse.jetty.ee10.webapp.WebDescriptor;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.thread.AutoLock;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -165,10 +166,10 @@ public boolean isIntrospectable(Object o, Object metaInfo)
if (_context.getMetaData().isMetaDataComplete())
return false;

String descriptorLocation = holder.getSource().getResource();
Resource descriptorLocation = holder.getSource().getResource();
if (descriptorLocation == null)
return true; //no descriptor, can't be metadata-complete
return !WebDescriptor.isMetaDataComplete(_context.getMetaData().getFragmentDescriptor(_context.getResourceFactory().newResource(descriptorLocation)));
return !WebDescriptor.isMetaDataComplete(_context.getMetaData().getFragmentDescriptor(descriptorLocation));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...e10-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/WebFilterAnnotation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void apply()
if (holder == null)
{
//Filter with this name does not already exist, so add it
holder = _context.getServletHandler().newFilterHolder(new Source(Source.Origin.ANNOTATION, clazz.getName()));
holder = _context.getServletHandler().newFilterHolder(new Source(Source.Origin.ANNOTATION, clazz));
holder.setName(name);

holder.setHeldClass(clazz);
Expand Down
2 changes: 1 addition & 1 deletion ...0-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/WebListenerAnnotation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void apply()
MetaData metaData = _context.getMetaData();
if (metaData.getOrigin(clazz.getName() + ".listener") == Origin.NotSet)
{
ListenerHolder h = _context.getServletHandler().newListenerHolder(new Source(Source.Origin.ANNOTATION, clazz.getName()));
ListenerHolder h = _context.getServletHandler().newListenerHolder(new Source(Source.Origin.ANNOTATION, clazz));
h.setHeldClass(clazz);
_context.getServletHandler().addListener(h);
metaData.setOrigin(clazz.getName() + ".listener", clazz.getAnnotation(WebListener.class), clazz);
Expand Down
4 changes: 2 additions & 2 deletions ...10-annotations/src/main/java/org/eclipse/jetty/ee10/annotations/WebServletAnnotation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public void apply()
{
//No servlet of this name has already been defined, either by a descriptor
//or another annotation (which would be impossible).
Source source = new Source(Source.Origin.ANNOTATION, clazz.getName());
Source source = new Source(Source.Origin.ANNOTATION, clazz);

holder = _context.getServletHandler().newServletHolder(source);
holder.setHeldClass(clazz);
Expand Down Expand Up @@ -183,7 +183,7 @@ public void apply()
//about processing these url mappings
if (existingMappings.isEmpty() || !containsNonDefaultMappings(existingMappings))
{
mapping = new ServletMapping(new Source(Source.Origin.ANNOTATION, clazz.getName()));
mapping = new ServletMapping(new Source(Source.Origin.ANNOTATION, clazz));
mapping.setServletName(servletName);
mapping.setPathSpecs(LazyList.toStringArray(urlPatternList));
_context.getMetaData().setOrigin(servletName + ".servlet.mapping." + Long.toHexString(mapping.hashCode()), annotation, clazz);
Expand Down
124 changes: 57 additions & 67 deletions ...tations/src/test/java/org/eclipse/jetty/ee10/annotations/TestAnnotationConfiguration.java
View file
Open in desktop

Large diffs are not rendered by default.

20 changes: 9 additions & 11 deletions ...annotations/src/test/java/org/eclipse/jetty/ee10/annotations/TestAnnotationDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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, () ->
{
Expand All @@ -108,9 +106,9 @@ 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, ""));
ServletHolder holder = new ServletHolder(new Source(Source.Origin.DESCRIPTOR));
holder.setHeldClass(ServletE.class);
context.getServletHandler().addServlet(holder);
DecoratedObjectFactory.associateInfo(holder);
Expand All @@ -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);
Expand Down
39 changes: 20 additions & 19 deletions ...otations/src/test/java/org/eclipse/jetty/ee10/annotations/TestAnnotationIntrospector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -58,36 +57,38 @@ public void testIsIntrospectable() throws Exception
assertTrue(introspector.isIntrospectable(new ServletE(), holder));

//an ANNOTATION sourced servlet can be introspected
holder = new ServletHolder(new Source(Source.Origin.ANNOTATION, ServletE.class.getName()));
holder = new ServletHolder(new Source(Source.Origin.ANNOTATION, ServletE.class));
holder.setHeldClass(ServletE.class);
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, xmlResource));
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, xmlResource));
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, xmlResource));
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, xmlResource));
assertFalse(introspector.isIntrospectable(new ServletE(), holder));
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...a/org/eclipse/jetty/ee10/annotations/TestDiscoveredServletContainerInitializerHolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void test() throws Exception
SampleServletContainerInitializer sci = new SampleServletContainerInitializer();

AnnotationConfiguration.DiscoveredServletContainerInitializerHolder holder =
new AnnotationConfiguration.DiscoveredServletContainerInitializerHolder(new Source(Source.Origin.ANNOTATION, sci.getClass().getName()),
new AnnotationConfiguration.DiscoveredServletContainerInitializerHolder(new Source(Source.Origin.ANNOTATION, sci.getClass()),
sci);

//add the @HandlesTypes to the holder
Expand Down
16 changes: 5 additions & 11 deletions ...e10-annotations/src/test/java/org/eclipse/jetty/ee10/annotations/TestRunAsAnnotation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
}
}
8 changes: 4 additions & 4 deletions ...y-ee10-plus/src/main/java/org/eclipse/jetty/ee10/plus/webapp/PlusDescriptorProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.getURI());
}
default:
break;
Expand Down Expand Up @@ -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.getURI());
//same in multiple web-fragments, merge the injections
addInjections(context, descriptor, node, jndiName, TypeUtil.fromName(type));
}
Expand Down Expand Up @@ -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.getURI());

//same in multiple web-fragments, merge the injections
addInjections(context, descriptor, node, jndiName, TypeUtil.fromName(type));
Expand Down Expand Up @@ -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.getURI());

//same in multiple web-fragments, merge the injections
addInjections(context, descriptor, node, jndiName, TypeUtil.fromName(type));
Expand Down
Loading