Hi Fabio, I can't see the reply you got on the list. Also I think you've introduced an important bug: whenever plexus instantiates a per-lookup component a new instance is created but you have to release it explicitely as otherwise it won't be released. Which means you'll get an out of memory exception very quickly. per-look must be avoided as much as possible (we don't have any in the other components of xwiki because of the release problem). Also I don't understand why something like XmlTextSpacesRepresenter should not be a singleton. Thanks -Vincent On Jan 6, 2009, at 4:43 PM, fmancinelli (SVN) wrote:
Author: fmancinelli Date: 2009-01-06 16:43:28 +0100 (Tue, 06 Jan 2009) New Revision: 15096
Added: sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiRouter.java Removed: sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiResourceFinder.java Modified: sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiResource.java sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiRestApplication.java sandbox/xwiki-core-rest/src/main/resources/META-INF/plexus/ components.xml Log: Refactoring due to the reply on the Restlet forum concerning object lifecycle (http://restlet.tigris.org/ds/viewMessage.do?dsForumId=4447&dsMessageId=10073... )
Now resources and presenters are instantiated at every lookup.
Code simplification.
Modified: sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/ rest/XWikiResource.java =================================================================== --- sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiResource.java 2009-01-06 14:29:26 UTC (rev 15095) +++ sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiResource.java 2009-01-06 15:43:28 UTC (rev 15096) @@ -18,25 +18,20 @@ */ public class XWikiResource extends WadlResource { - /* This is injected by the component manager and containsall the registered representers */ + /* This is injected by the component manager and contains all the registered representers */ private Map<String, XWikiResourceRepresenter> descriptorToRepresenterMap;
/* This map contains only the the representers for the current resource, indexed by media type */ private Map<MediaType, XWikiResourceRepresenter> mediaTypeToRepresenterMap;
+ /* This is configured in components.xml */ + private String uriPattern; + @Override public void init(Context context, Request request, Response response) { super.init(context, request, response);
- /* - * If the mediaTypeToRepresenterMap is already initialized then return because we already have all the - * representers registered. - */ - if (mediaTypeToRepresenterMap != null) { - return; - } - mediaTypeToRepresenterMap = new HashMap<MediaType, XWikiResourceRepresenter>(); getVariants().clear(); for (String hint : descriptorToRepresenterMap.keySet()) { @@ -45,7 +40,7 @@ if (hint.startsWith(this.getClass().getName())) { getLogger().log( Level.INFO, - String.format("Registering %s representer for resource %s", representer.getMediaType(), this + String.format("Registering '%s' representer for resource %s", representer.getMediaType(), this .getClass().getName())); getVariants().add(new Variant(representer.getMediaType()));
mediaTypeToRepresenterMap.put(representer.getMediaType(), representer); @@ -60,9 +55,21 @@ if (representer != null) { return representer; } else { - /* Flow control should never reach this. */ + /* Flow control should never reach this point. */ + getLogger() + .log( + Level.WARNING, + String + .format( + "'%s' representer for resource %s doesn't exist. This should never happen. Returning a null representer.", + variant.getMediaType(), this.getClass().getName())); return new NullRepresenter(); } }
+ public String getUriPattern() + { + return uriPattern; + } + }
Deleted: sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/ rest/XWikiResourceFinder.java =================================================================== --- sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiResourceFinder.java 2009-01-06 14:29:26 UTC (rev 15095) +++ sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiResourceFinder.java 2009-01-06 15:43:28 UTC (rev 15096) @@ -1,32 +0,0 @@ -package components.org.xwiki.rest; - -import org.restlet.Context; -import org.restlet.Finder; -import org.restlet.Handler; -import org.restlet.data.Request; -import org.restlet.data.Response; - -/** - * @version $Id$ - */ -public class XWikiResourceFinder extends Finder -{ - private Context context; - - private XWikiResource resource; - - public XWikiResourceFinder(Context context, XWikiResource resource) - { - this.context = context; - this.resource = resource; - } - - @Override - protected Handler findTarget(Request request, Response response) - { - resource.init(context, request, response); - - return resource; - } - -}
Modified: sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/ rest/XWikiRestApplication.java =================================================================== --- sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiRestApplication.java 2009-01-06 14:29:26 UTC (rev 15095) +++ sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiRestApplication.java 2009-01-06 15:43:28 UTC (rev 15096) @@ -20,7 +20,7 @@ /* Injected by the component manager */ private ComponentManager componentManager;
- private Map<String, XWikiResource> uriPatternToResourceMap; + private Map<String, XWikiResource> classNameToResourceMap;
public XWikiRestApplication() { @@ -48,18 +48,18 @@ getTunnelService().setEnabled(true); getTunnelService().setExtensionsTunnel(true);
- Router router = new Router(getContext()); + Router router = new XWikiRouter(componentManager, getContext());
/* Register all the resource components */ - for (String uriPattern : uriPatternToResourceMap.keySet()) { - XWikiResource resource = uriPatternToResourceMap.get(uriPattern); + for (String className : classNameToResourceMap.keySet()) { + XWikiResource resource = classNameToResourceMap.get(className);
getLogger().log( Level.INFO, - String - .format("Registering resource %s for URI pattern '%s'", resource.getClass().getName(), uriPattern)); + String.format("Attaching resource %s to URI pattern '%s'", resource.getClass().getName(), resource + .getUriPattern()));
- router.attach(uriPattern, new XWikiResourceFinder(getContext(), resource)); + router.attach(resource.getUriPattern(), resource.getClass()); }
return router; @@ -69,4 +69,5 @@ { this.componentManager = componentManager; } + }
Added: sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/ rest/XWikiRouter.java =================================================================== --- sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiRouter.java (rev 0) +++ sandbox/xwiki-core-rest/src/main/java/components/org/xwiki/rest/ XWikiRouter.java 2009-01-06 15:43:28 UTC (rev 15096) @@ -0,0 +1,56 @@ +package components.org.xwiki.rest; + +import org.restlet.Context; +import org.restlet.Finder; +import org.restlet.Handler; +import org.restlet.Router; +import org.restlet.data.Request; +import org.restlet.data.Response; +import org.restlet.resource.Resource; +import org.xwiki.component.manager.ComponentLookupException; +import org.xwiki.component.manager.ComponentManager; + +public class XWikiRouter extends Router +{ + private ComponentManager componentManager; + + private static class XWikiFinder extends Finder + { + private ComponentManager componentManager; + + public XWikiFinder(ComponentManager componentManager, Context context, Class< ? extends Handler> targetClass) + { + super(context, targetClass); + this.componentManager = componentManager; + } + + @Override + public Handler createTarget(Class< ? extends Handler> targetClass, Request request, Response response) + { + Handler handler = null; + + try { + handler = (Handler) componentManager.lookup(XWikiResource.class.getName(), targetClass.getName()); + handler.init(getContext(), request, response); + } catch (ComponentLookupException e) { + e.printStackTrace(); + } + + return handler; + } + + } + + public XWikiRouter(ComponentManager componentManager, Context context) + { + super(context); + this.componentManager = componentManager; + } + + @Override + protected Finder createFinder(Class< ? extends Resource> targetClass) + { + return new XWikiFinder(componentManager, getContext(), targetClass); + } + +}
Modified: sandbox/xwiki-core-rest/src/main/resources/META-INF/plexus/ components.xml =================================================================== --- sandbox/xwiki-core-rest/src/main/resources/META-INF/plexus/ components.xml 2009-01-06 14:29:26 UTC (rev 15095) +++ sandbox/xwiki-core-rest/src/main/resources/META-INF/plexus/ components.xml 2009-01-06 15:43:28 UTC (rev 15096) @@ -12,19 +12,23 @@ <requirement> <role>components.org.xwiki.rest.XWikiResource </role> - <field-name>uriPatternToResourceMap</field-name> + <field-name>classNameToResourceMap + </field-name> </requirement> </requirements> </component> + <component> <role>components.org.xwiki.rest.XWikiResource </role> <!-- - The role hint is used to specify the URI template this resource is - attached to. + Role hint must be the same as the implementation since it is used to + lookup for the implementation in XWikiRouter.XWikiFinder. --> - <role-hint>/spaces</role-hint> - <instantiation-strategy>singleton</instantiation-strategy> + <role-hint>components.org.xwiki.rest.SpacesResource + </role-hint> + <!-- Resource instantiation strategy should be per-lookup --> + <instantiation-strategy>per-lookup</instantiation-strategy> <implementation>components.org.xwiki.rest.SpacesResource </implementation> <requirements> @@ -33,10 +37,15 @@ </role> <role-hint>components.org.xwiki.rest.SpacesResource </role-hint> - <field-name>descriptorToRepresenterMap</field-name> + <field-name>descriptorToRepresenterMap + </field-name> </requirement> </requirements> + <configuration> + <uriPattern>/spaces</uriPattern> + </configuration> </component> + <component> <role>components.org.xwiki.rest.XWikiResourceRepresenter </role> @@ -53,7 +62,7 @@ --> <role-hint>components.org.xwiki.rest.SpacesResource:text/plain </role-hint> - <instantiation-strategy>singleton</instantiation-strategy> + <instantiation-strategy>per-lookup</instantiation-strategy> <implementation> components.org.xwiki.rest.internal.PlainTextSpacesRepresenter </implementation> @@ -63,7 +72,7 @@ </role> <role-hint>components.org.xwiki.rest.SpacesResource:text/xml </role-hint> - <instantiation-strategy>singleton</instantiation-strategy> + <instantiation-strategy>per-lookup</instantiation-strategy> <implementation> components.org.xwiki.rest.internal.XmlTextSpacesRepresenter </implementation>