You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Adam Winer (JIRA)" <ji...@apache.org> on 2008/10/03 00:59:47 UTC
[jira] Created: (SHINDIG-641) Simplify DataRequestHandler
dispatching
Simplify DataRequestHandler dispatching
---------------------------------------
Key: SHINDIG-641
URL: https://issues.apache.org/jira/browse/SHINDIG-641
Project: Shindig
Issue Type: Improvement
Components: RESTful API (Java)
Reporter: Adam Winer
Priority: Minor
SHINDIG-638 cleaned up DataRequestHandler creation, but still left us with a HandleProvider API that you need to subclass to manipulate, and that also codifies a service-name-to-handler Map as the dispatching mechanism.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-641) Simplify DataRequestHandler
dispatching
Posted by "Adam Winer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-641?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adam Winer updated SHINDIG-641:
-------------------------------
Attachment: shindig-641-2.patch
Attached revised patch incorporating Evan's comments.
> Simplify DataRequestHandler dispatching
> ---------------------------------------
>
> Key: SHINDIG-641
> URL: https://issues.apache.org/jira/browse/SHINDIG-641
> Project: Shindig
> Issue Type: Improvement
> Components: RESTful API (Java)
> Reporter: Adam Winer
> Priority: Minor
> Attachments: shindig-641-2.patch, shindig-641.patch
>
>
> SHINDIG-638 cleaned up DataRequestHandler creation, but still left us with a HandleProvider API that you need to subclass to manipulate, and that also codifies a service-name-to-handler Map as the dispatching mechanism.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (SHINDIG-641) Simplify DataRequestHandler
dispatching
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-641?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paul Lindner resolved SHINDIG-641.
----------------------------------
Resolution: Fixed
Code is in there and works fine.
> Simplify DataRequestHandler dispatching
> ---------------------------------------
>
> Key: SHINDIG-641
> URL: https://issues.apache.org/jira/browse/SHINDIG-641
> Project: Shindig
> Issue Type: Improvement
> Components: RESTful API (Java)
> Reporter: Adam Winer
> Priority: Minor
> Attachments: shindig-641-2.patch, shindig-641.patch
>
>
> SHINDIG-638 cleaned up DataRequestHandler creation, but still left us with a HandleProvider API that you need to subclass to manipulate, and that also codifies a service-name-to-handler Map as the dispatching mechanism.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-641) Simplify DataRequestHandler
dispatching
Posted by "Adam Winer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12637110#action_12637110 ]
Adam Winer commented on SHINDIG-641:
------------------------------------
I'll add the "what to do after you sync" to this issue.
You already don't need to subclass to configure: see the provider that adds SampleContainerHandler. There's no subclass of DefaultHandlerDispatcher.
On the numbered comments:
1. Will do, Standard is a more accurate description.
2. The default constructor cannot be a helper static - it needs to be an @Inject so that it can be used as a default implementation. I can change the existing private constructor taking the map to public, though the class hardly does much when used that way.
3. There's already addHandler(String, Provider)
4. Woo-hoo!
I agree that SampleContainerHandler should not be depended on by SocialApiGuiceModule. Looks like a separate issue, though - the dependency was already there.
I'm not that enthused with exposing Provider in the HandlerDispatcher API, as it makes the API less minimal, and anyone that wanted to be really lazy about getting the handler can just be lazy about calling the HandlerDispatcher.
> Simplify DataRequestHandler dispatching
> ---------------------------------------
>
> Key: SHINDIG-641
> URL: https://issues.apache.org/jira/browse/SHINDIG-641
> Project: Shindig
> Issue Type: Improvement
> Components: RESTful API (Java)
> Reporter: Adam Winer
> Priority: Minor
> Attachments: shindig-641.patch
>
>
> SHINDIG-638 cleaned up DataRequestHandler creation, but still left us with a HandleProvider API that you need to subclass to manipulate, and that also codifies a service-name-to-handler Map as the dispatching mechanism.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-641) Simplify DataRequestHandler
dispatching
Posted by "Adam Winer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-641?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adam Winer updated SHINDIG-641:
-------------------------------
Attachment: shindig-641.patch
Patch that gets rid of HandlerProvider, and replaces it with:
public interface HandlerDispatcher {
DataRequestHandler getHandler(String service);
}
... which makes dispatching more flexible, and lets you override dispatching with a decorator around the default instead of a subclass.
> Simplify DataRequestHandler dispatching
> ---------------------------------------
>
> Key: SHINDIG-641
> URL: https://issues.apache.org/jira/browse/SHINDIG-641
> Project: Shindig
> Issue Type: Improvement
> Components: RESTful API (Java)
> Reporter: Adam Winer
> Priority: Minor
> Attachments: shindig-641.patch
>
>
> SHINDIG-638 cleaned up DataRequestHandler creation, but still left us with a HandleProvider API that you need to subclass to manipulate, and that also codifies a service-name-to-handler Map as the dispatching mechanism.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-641) Simplify DataRequestHandler
dispatching
Posted by "Evan Gilbert (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12637098#action_12637098 ]
Evan Gilbert commented on SHINDIG-641:
--------------------------------------
Few notes on the patch:
- Since this patch changes the API for adding additional handlers, would be good to have a brief set of instructions of "what to do after you sync" to fix issues.
- Think that with a few small changes, clients could create an instance of DefaultHandlerDispatcher instead of subclassing to configure Shindig. Possible path to this:
1. Rename DefaultHandlerDispatcher to StandardHandlerDispatcher or similar
2. Have a constructor that takes a Map<String, Provider<? extends DataRequestHandler>>, and move the default constructor to a helper static that gets the default provider.s
3. (optional) Have functions to addDispatcher(String, Provider<? extends DataRequestHandler>) and addDispatcher(String, Class<? extends DataRequestHandler>) to allow for distributed initialization.
4. Profit.
- Moving the provider of SampleContainerHandler from org.apache.opensocial.social.sample.service -> SocialApiGuiceModule creates dependency from from our core package on the sample package. We should probably not have this dependency.
- I like the simplicity of HandlerDispatcher.getHandler() returning an actual handler instance. I can also see use cases where it might be useful to have it return a provider - this would allow the client code to choose when to get an instance. For example, if we decided for non-batch cases to create one servlet per handler at initialization time instead of having a dispatcher servlet (slightly contrived case), we could have each servlet be passed in a Provider. No need to change this unless you think it's a good idea - we can always add later.
> Simplify DataRequestHandler dispatching
> ---------------------------------------
>
> Key: SHINDIG-641
> URL: https://issues.apache.org/jira/browse/SHINDIG-641
> Project: Shindig
> Issue Type: Improvement
> Components: RESTful API (Java)
> Reporter: Adam Winer
> Priority: Minor
> Attachments: shindig-641.patch
>
>
> SHINDIG-638 cleaned up DataRequestHandler creation, but still left us with a HandleProvider API that you need to subclass to manipulate, and that also codifies a service-name-to-handler Map as the dispatching mechanism.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-641) Simplify DataRequestHandler
dispatching
Posted by "Adam Winer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12637313#action_12637313 ]
Adam Winer commented on SHINDIG-641:
------------------------------------
As Evan requested, the "what to do after you sync" doc:
Config code changes from pointing at a subclass of HandlerProvider
bind(HandlerProvider.class).to(SampleContainerHandlerProvider.class);
... to a Provider for HandlerDispatcher:
bind(HandlerDispatcher.class).toProvider(HandlerDispatcherProvider.class);
The provider changes from a subclass of HandlerProvider:
public class SampleContainerHandlerProvider extends HandlerProvider {
public SampleContainerHandlerProvider() {
super(true);
addHandler("samplecontainer", SampleContainerHandler.class);
}
}
... to a Provider<HandlerDispatcher> implementation:
static class HandlerDispatcherProvider implements Provider<HandlerDispatcher> {
private final HandlerDispatcher dispatcher;
@Inject
public HandlerDispatcherProvider(StandardHandlerDispatcher dispatcher,
Provider<SampleContainerHandler> sampleHandler) {
dispatcher.addHandler("samplecontainer", sampleHandler);
this.dispatcher = dispatcher;
}
public HandlerDispatcher get() {
return dispatcher;
}
}
Instead of subclassing HandlerProvider, you just inject StandardHandlerDispatcher. And instead of adding a Class object, you add a Provider that Guice has created on your behalf.
> Simplify DataRequestHandler dispatching
> ---------------------------------------
>
> Key: SHINDIG-641
> URL: https://issues.apache.org/jira/browse/SHINDIG-641
> Project: Shindig
> Issue Type: Improvement
> Components: RESTful API (Java)
> Reporter: Adam Winer
> Priority: Minor
> Attachments: shindig-641-2.patch, shindig-641.patch
>
>
> SHINDIG-638 cleaned up DataRequestHandler creation, but still left us with a HandleProvider API that you need to subclass to manipulate, and that also codifies a service-name-to-handler Map as the dispatching mechanism.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.