FELIX-4060 : Implement HTTP Service Update (RFC-189)
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1655699 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HandlerRegistry.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HandlerRegistry.java
index feb1a1b..3a33ef6 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HandlerRegistry.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/HandlerRegistry.java
@@ -29,9 +29,8 @@
public final class HandlerRegistry
{
- private final ConcurrentMap<Servlet, ServletHandler> servletMap = new ConcurrentHashMap<Servlet, ServletHandler>();
private final ConcurrentMap<Filter, FilterHandler> filterMap = new ConcurrentHashMap<Filter, FilterHandler>();
- private final ConcurrentMap<String, Servlet> aliasMap = new ConcurrentHashMap<String, Servlet>();
+ private final ConcurrentMap<String, ServletHandler> aliasMap = new ConcurrentHashMap<String, ServletHandler>();
private volatile ServletHandler[] servlets;
private volatile FilterHandler[] filters;
@@ -55,18 +54,8 @@
{
handler.init();
- // there is a window of opportunity that the servlet/alias was registered between the
- // previous check and this one, so we have to atomically add if absent here.
- if (servletMap.putIfAbsent(handler.getServlet(), handler) != null)
+ if (aliasMap.putIfAbsent(handler.getAlias(), handler) != null)
{
- // Do not destroy the servlet as the same instance was already registered
- throw new ServletException("Servlet instance already registered");
- }
- if (aliasMap.putIfAbsent(handler.getAlias(), handler.getServlet()) != null)
- {
- // Remove it from the servletmap too
- servletMap.remove(handler.getServlet(), handler);
-
handler.destroy();
throw new NamespaceException("Servlet with alias '" + handler.getAlias() + "' already registered");
}
@@ -91,15 +80,22 @@
public void removeServlet(Servlet servlet, final boolean destroy)
{
- ServletHandler handler = servletMap.remove(servlet);
- if (handler != null)
+ boolean update = false;
+ for (Iterator<ServletHandler> it = aliasMap.values().iterator(); it.hasNext(); )
+ {
+ ServletHandler handler = it.next();
+ if (handler.getServlet() == servlet ) {
+ it.remove();
+ if (destroy)
+ {
+ handler.destroy();
+ }
+ update = true;
+ }
+ }
+ if ( update )
{
updateServletArray();
- aliasMap.remove(handler.getAlias());
- if (destroy)
- {
- handler.destroy();
- }
}
}
@@ -118,7 +114,11 @@
public Servlet getServletByAlias(String alias)
{
- return aliasMap.get(alias);
+ final ServletHandler handler = aliasMap.get(alias);
+ if ( handler != null ) {
+ return handler.getServlet();
+ }
+ return null;
}
public void addErrorServlet(String errorPage, ServletHandler handler) throws ServletException
@@ -128,12 +128,11 @@
public void removeAll()
{
- for (Iterator<ServletHandler> it = servletMap.values().iterator(); it.hasNext(); )
+ for (Iterator<ServletHandler> it = aliasMap.values().iterator(); it.hasNext(); )
{
ServletHandler handler = it.next();
it.remove();
- aliasMap.remove(handler.getAlias());
handler.destroy();
}
@@ -150,14 +149,14 @@
private void updateServletArray()
{
- ServletHandler[] tmp = servletMap.values().toArray(new ServletHandler[servletMap.size()]);
+ final ServletHandler[] tmp = aliasMap.values().toArray(new ServletHandler[aliasMap.size()]);
Arrays.sort(tmp);
servlets = tmp;
}
private void updateFilterArray()
{
- FilterHandler[] tmp = filterMap.values().toArray(new FilterHandler[filterMap.size()]);
+ final FilterHandler[] tmp = filterMap.values().toArray(new FilterHandler[filterMap.size()]);
Arrays.sort(tmp);
filters = tmp;
}
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
index f990685..f93377f 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
@@ -237,25 +237,25 @@
private final String alias;
private final Servlet servlet;
- public ServletHandler(ExtServletContext context, Servlet servlet, String alias, String name)
+ public ServletHandler(final ExtServletContext context,
+ final Servlet servlet,
+ final ServletInfo servletInfo,
+ final String alias)
{
- super(context, name);
- this.alias = alias;
- this.servlet = servlet;
- }
-
- public ServletHandler(ExtServletContext context, Servlet servlet, ServletInfo servletInfo)
- {
- // TODO
super(context, servletInfo.name);
this.servlet = servlet;
- this.alias = servletInfo.patterns[0];
+ this.alias = alias;
}
@Override
public int compareTo(ServletHandler other)
{
- return other.alias.length() - this.alias.length();
+ int result = other.alias.length() - this.alias.length();
+ if ( result == 0 )
+ {
+ result = this.alias.compareTo(other.alias);
+ }
+ return result;
}
public RequestDispatcher createNamedRequestDispatcher()
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java b/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java
index dddb544..55363a5 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java
@@ -72,5 +72,4 @@
* The {@link HttpContext} for the servlet.
*/
public HttpContext context;
-
}
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceImpl.java b/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceImpl.java
index 99b9f64..e937ae3 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceImpl.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/service/HttpServiceImpl.java
@@ -148,11 +148,22 @@
}
}
+ /**
+ * TODO As the servlet can be registered with multiple patterns
+ * we shouldn't pass the servlet object around in order to
+ * be able to get different instances (prototype scope).
+ * Or we do the splitting into single pattern registrations
+ * already before calling registerServlet()
+ * @param servlet
+ * @param servletInfo
+ * @throws ServletException
+ * @throws NamespaceException
+ */
public void registerServlet(Servlet servlet, ServletInfo servletInfo) throws ServletException, NamespaceException
{
if (servlet == null)
{
- throw new IllegalArgumentException("Servlet cannot be null!");
+ throw new IllegalArgumentException("Servlet must not be null");
}
if (servletInfo == null)
{
@@ -167,28 +178,45 @@
servletInfo.name = servlet.getClass().getName();
}
- ServletHandler handler = new ServletHandler(getServletContext(servletInfo.context), servlet, servletInfo);
- handler.setInitParams(servletInfo.initParams);
- this.handlerRegistry.addServlet(handler);
- this.localServlets.add(servlet);
+ for(final String pattern : servletInfo.patterns) {
+ final ServletHandler handler = new ServletHandler(getServletContext(servletInfo.context), servlet, servletInfo, pattern);
+ handler.setInitParams(servletInfo.initParams);
+ this.handlerRegistry.addServlet(handler);
+ this.localServlets.add(servlet);
+ }
}
+ /**
+ * @see org.osgi.service.http.HttpService#registerServlet(java.lang.String, javax.servlet.Servlet, java.util.Dictionary, org.osgi.service.http.HttpContext)
+ */
@Override
public void registerServlet(String alias, Servlet servlet, Dictionary initParams, HttpContext context) throws ServletException, NamespaceException
{
- if (servlet == null)
- {
- throw new IllegalArgumentException("Servlet must not be null");
- }
if (!isAliasValid(alias))
{
throw new IllegalArgumentException("Malformed servlet alias [" + alias + "]");
}
- String servletName = null; // XXX
- ServletHandler handler = new ServletHandler(getServletContext(context), servlet, alias, servletName);
- handler.setInitParams(initParams);
- this.handlerRegistry.addServlet(handler);
- this.localServlets.add(servlet);
+
+ final ServletInfo info = new ServletInfo();
+ if ( initParams != null && initParams.size() > 0 )
+ {
+ info.initParams = new HashMap<String, String>();
+ Enumeration e = initParams.keys();
+ while (e.hasMoreElements())
+ {
+ Object key = e.nextElement();
+ Object value = initParams.get(key);
+
+ if ((key instanceof String) && (value instanceof String))
+ {
+ info.initParams.put((String) key, (String) value);
+ }
+ }
+ }
+ info.patterns = new String[] {alias};
+ info.context = context;
+
+ this.registerServlet(servlet, info);
}
@Override
diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/HandlerRegistryTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/HandlerRegistryTest.java
index 2eb4314..59cdada 100644
--- a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/HandlerRegistryTest.java
+++ b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/HandlerRegistryTest.java
@@ -27,6 +27,7 @@
import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
+import org.apache.felix.http.base.internal.runtime.ServletInfo;
import org.junit.Test;
import org.mockito.Mockito;
import org.mockito.invocation.InvocationOnMock;
@@ -41,27 +42,34 @@
HandlerRegistry hr = new HandlerRegistry();
Servlet servlet = Mockito.mock(Servlet.class);
- ServletHandler handler = new ServletHandler(null, servlet, "/foo", "foo");
+ final ServletInfo info = new ServletInfo();
+ info.name = "foo";
+ ServletHandler handler = new ServletHandler(null, servlet, info, "/foo");
assertEquals("Precondition", 0, hr.getServlets().length);
hr.addServlet(handler);
Mockito.verify(servlet, Mockito.times(1)).init(Mockito.any(ServletConfig.class));
assertEquals(1, hr.getServlets().length);
assertSame(handler, hr.getServlets()[0]);
- ServletHandler handler2 = new ServletHandler(null, servlet, "/bar", "bar");
+ final ServletInfo info2 = new ServletInfo();
+ info2.name = "bar";
+ ServletHandler handler2 = new ServletHandler(null, servlet, info2, "/bar");
try
{
hr.addServlet(handler2);
- fail("Should not have allowed to add the same servlet twice");
+ // TODO
+// fail("Should not have allowed to add the same servlet twice");
}
catch (ServletException se)
{
// good
}
- assertArrayEquals(new ServletHandler[] {handler}, hr.getServlets());
+ assertArrayEquals(new ServletHandler[] {handler2, handler}, hr.getServlets());
- ServletHandler handler3 = new ServletHandler(null, Mockito.mock(Servlet.class),
- "/foo", "zar");
+ final ServletInfo info3 = new ServletInfo();
+ info3.name = "zar";
+ ServletHandler handler3 = new ServletHandler(null, Mockito.mock(Servlet.class), info3,
+ "/foo");
try
{
hr.addServlet(handler3);
@@ -70,13 +78,13 @@
catch (NamespaceException ne) {
// good
}
- assertArrayEquals(new ServletHandler[] {handler}, hr.getServlets());
+ assertArrayEquals(new ServletHandler[] {handler2, handler}, hr.getServlets());
assertSame(servlet, hr.getServletByAlias("/foo"));
Mockito.verify(servlet, Mockito.never()).destroy();
hr.removeServlet(servlet, true);
- Mockito.verify(servlet, Mockito.times(1)).destroy();
+ Mockito.verify(servlet, Mockito.times(2)).destroy();
assertEquals(0, hr.getServlets().length);
}
@@ -86,11 +94,14 @@
final HandlerRegistry hr = new HandlerRegistry();
Servlet servlet = Mockito.mock(Servlet.class);
- final ServletHandler otherHandler = new ServletHandler(null, servlet, "/bar", "bar");
+ final ServletInfo info = new ServletInfo();
+ info.name = "bar";
+ final ServletHandler otherHandler = new ServletHandler(null, servlet, info, "/bar");
Mockito.doAnswer(new Answer<Void>()
{
boolean registered = false;
+ @Override
public Void answer(InvocationOnMock invocation) throws Throwable
{
if (!registered)
@@ -104,20 +115,23 @@
}
}).when(servlet).init(Mockito.any(ServletConfig.class));
- ServletHandler handler = new ServletHandler(null, servlet, "/foo", "foo");
-
+ final ServletInfo info2 = new ServletInfo();
+ info2.name = "foo";
+ ServletHandler handler = new ServletHandler(null, servlet, info2, "/foo");
try
{
hr.addServlet(handler);
- fail("Should not have allowed the servlet to be added as it was already "
- + "added before init was finished");
+
+ // TODO
+// fail("Should not have allowed the servlet to be added as it was already "
+// + "added before init was finished");
}
catch (ServletException ne)
{
// good
}
- assertArrayEquals(new ServletHandler[] {otherHandler}, hr.getServlets());
+ assertArrayEquals(new ServletHandler[] {otherHandler, handler}, hr.getServlets());
}
@Test
@@ -126,11 +140,14 @@
final HandlerRegistry hr = new HandlerRegistry();
Servlet otherServlet = Mockito.mock(Servlet.class);
- final ServletHandler otherHandler = new ServletHandler(null, otherServlet, "/foo", "bar");
+ final ServletInfo info = new ServletInfo();
+ info.name = "bar";
+ final ServletHandler otherHandler = new ServletHandler(null, otherServlet, info, "/foo");
Servlet servlet = Mockito.mock(Servlet.class);
Mockito.doAnswer(new Answer<Void>()
{
+ @Override
public Void answer(InvocationOnMock invocation) throws Throwable
{
// sneakily register another servlet before this one has finished calling init()
@@ -139,7 +156,9 @@
}
}).when(servlet).init(Mockito.any(ServletConfig.class));
- ServletHandler handler = new ServletHandler(null, servlet, "/foo", "foo");
+ final ServletInfo info2 = new ServletInfo();
+ info2.name = "foo";
+ ServletHandler handler = new ServletHandler(null, servlet, info2, "/foo");
try
{
@@ -198,6 +217,7 @@
Mockito.doAnswer(new Answer<Void>()
{
boolean registered = false;
+ @Override
public Void answer(InvocationOnMock invocation) throws Throwable
{
if (!registered)
@@ -232,10 +252,14 @@
HandlerRegistry hr = new HandlerRegistry();
Servlet servlet = Mockito.mock(Servlet.class);
- ServletHandler servletHandler = new ServletHandler(null, servlet, "/f", "f");
+ final ServletInfo info = new ServletInfo();
+ info.name = "f";
+ ServletHandler servletHandler = new ServletHandler(null, servlet, info, "/f");
hr.addServlet(servletHandler);
Servlet servlet2 = Mockito.mock(Servlet.class);
- ServletHandler servletHandler2 = new ServletHandler(null, servlet2, "/ff", "ff");
+ final ServletInfo info2 = new ServletInfo();
+ info2.name = "ff";
+ ServletHandler servletHandler2 = new ServletHandler(null, servlet2, info2, "/ff");
hr.addServlet(servletHandler2);
Filter filter = Mockito.mock(Filter.class);
FilterHandler filterHandler = new FilterHandler(null, filter, "/f", 0, "f");
diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java
index a8c14d2..561496f 100644
--- a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java
+++ b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java
@@ -33,6 +33,7 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.apache.felix.http.base.internal.runtime.ServletInfo;
import org.junit.Before;
import org.junit.Test;
@@ -40,6 +41,7 @@
{
private Servlet servlet;
+ @Override
@Before
public void setUp()
{
@@ -220,6 +222,7 @@
assertTrue(h1.matches("/a/b/c"));
}
+ @Override
protected AbstractHandler createHandler()
{
return createHandler("/dummy");
@@ -227,6 +230,7 @@
private ServletHandler createHandler(String alias)
{
- return new ServletHandler(this.context, this.servlet, alias, null /* name */);
+ final ServletInfo info = new ServletInfo();
+ return new ServletHandler(this.context, this.servlet, info, alias);
}
}