On Wed, Jul 7, 2010 at 10:07 AM, Hadrian Zbarcea <[email protected]> wrote:
> Fair enough, let's do that. I think beforeWrap is actually better though. And 
> then we document how it could/should be used.
>

Yeah great. There is already javadoc documentation.

But maybe we should add a wiki page about Policy and DefinitionAwarePolicy.


> On Jul 7, 2010, at 3:51 AM, Claus Ibsen wrote:
>
>> On Wed, Jul 7, 2010 at 9:46 AM, Hadrian Zbarcea <[email protected]> wrote:
>>> That works quite well actually. What would be good though is if we could 
>>> find a better name for the beforeWrap, something that communicates the 
>>> intent, something like updateDefinitions, something that would be more 
>>> intuitive about how the interface should be used. Yeah, not easy to come 
>>> with the right name :(
>>>
>>
>> Yeah naming a new class or method is not always that strait forward.
>>
>> However naming it *update* kinda implies that you *must* update the
>> definition. What if you intent is not to update it, but you may want
>> to do something else.
>> I think the before name in this case is a good name because its
>> invoked before we do the wrap.
>>
>> It could be named onBeforeWrap if people are more into the onXXX naming 
>> style.
>>
>>
>>
>>>
>>> On Jul 7, 2010, at 2:33 AM, Claus Ibsen wrote:
>>>
>>>> Hi
>>>>
>>>> I just had a thought.
>>>>
>>>> I think we should modify the DefinitionAwarePolicy a bit to ensure the
>>>> contract stays the same.
>>>> This is what I propose
>>>>
>>>> public interface DefinitionAwarePolicy extends Policy {
>>>>
>>>>    /**
>>>>     * Callback invoked before the wrap.
>>>>     * <p/>
>>>>     * This allows you to do any custom logic before the processor is
>>>> wrapped. For example to
>>>>     * manipulate the {...@link
>>>> org.apache.camel.model.ProcessorDefinition definiton}
>>>>     *
>>>>     * @param routeContext   the route context
>>>>     * @param definition     the processor definition
>>>>     */
>>>>    void beforeWrap(RouteContext routeContext, ProcessorDefinition<?>
>>>> definition);
>>>>
>>>> }
>>>>
>>>>
>>>> Then we have this beforeWrap method which is invoked before the wrap
>>>> method. This allows Mark / end users to manipulate the definition.
>>>> And then the wrap method from Policy is used to wrap it.
>>>>
>>>> Then the contract stays the same, and there is no surprises.
>>>>
>>>> This allows a nice and clean custom policy.
>>>>
>>>>    public static class MyPolicy implements DefinitionAwarePolicy {
>>>>
>>>>        private final String name;
>>>>        private int invoked;
>>>>
>>>>        public MyPolicy(String name) {
>>>>            this.name = name;
>>>>        }
>>>>
>>>>        public int getInvoked() {
>>>>            return invoked;
>>>>        }
>>>>
>>>>        public void beforeWrap(RouteContext routeContext,
>>>> ProcessorDefinition<?> definition) {
>>>>            SetBodyDefinition bodyDef = (SetBodyDefinition)
>>>> definition.getOutputs().get(0);
>>>>            bodyDef.setExpression(new ConstantExpression("body was 
>>>> altered"));
>>>>        }
>>>>
>>>>        public Processor wrap(final RouteContext routeContext, final
>>>> Processor processor) {
>>>>            return new Processor() {
>>>>                public void process(Exchange exchange) throws Exception {
>>>>                    invoked++;
>>>>                    exchange.getIn().setHeader(name, "was wrapped");
>>>>                    processor.process(exchange);
>>>>                }
>>>>            };
>>>>        }
>>>>
>>>>    }
>>>>
>>>>
>>>>
>>>>
>>>> On Wed, Jul 7, 2010 at 8:18 AM, Claus Ibsen <[email protected]> wrote:
>>>>> On Wed, Jul 7, 2010 at 8:08 AM, Hadrian Zbarcea <[email protected]> 
>>>>> wrote:
>>>>>> Should we continue the discussion here or in the jira?
>>>>>>
>>>>>
>>>>> IMHO mailing list is best as they are easier to search. Also a broader
>>>>> audience may listen and contribute.
>>>>>
>>>>>
>>>>>> Not sure if I get your point. To me a policy is meant exactly for that: 
>>>>>> to wrap a part of a route and inject extra processing. The wrap method 
>>>>>> returns a Processor that does some stuff and at some point delegates to 
>>>>>> the inner Processor that represents the part of the route wrapped by the 
>>>>>> policy. As such one could chose to add another SendProcessor to achieve 
>>>>>> something like a wiretap, or a ThreadsProcessor, or a Throttler, or what 
>>>>>> not. It can be done by instantiating Processors, but not by manipulating 
>>>>>> the ast. Why not? This addition allows one to do in a simpler way, what 
>>>>>> is already possible.
>>>>>>
>>>>>> Why would you -1 this? What's the harm?
>>>>>>
>>>>>
>>>>> The contract is different between the Policy and the DefinitionAwarePolicy
>>>>>
>>>>> In the former the processor parameter is the created processor
>>>>> In the latter the processor parameter is a "dummy delegate" because
>>>>> Mark's goal was to *manipulate* the definition *before* the actual
>>>>> processor is created.
>>>>> So he passes in a "dummy delegate" and then afterwards creates the
>>>>> actual processor, now that he has manipulated the definition.
>>>>>
>>>>> This is not intuitive and leads to surprises!
>>>>>
>>>>>
>>>>> Mark's goal is to manipulate the routes before they are created, to
>>>>> have it adapted to his environment (I assume related to JBoss).
>>>>> On a greater scheme I would rather introduce a SPI factory to register
>>>>> a custom factory where Mark can do his manipulation.
>>>>>
>>>>> All you have to do is to define a Spring bean with the custom hook and
>>>>> Camel will pick it up and use it.
>>>>>
>>>>> <bean id="customFactory"
>>>>> class="org.apache.camel.processor.CustomProcessorFactoryTest$MyFactory"/>
>>>>>
>>>>> Then the end user doesn't have to remember to add a policy or
>>>>> whatever. The routes can *stay as is*.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> On Jul 7, 2010, at 1:05 AM, Claus Ibsen wrote:
>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> In the patch on the ticket
>>>>>>> https://issues.apache.org/activemq/browse/CAMEL-2914
>>>>>>>
>>>>>>> The processor which is passed into the wrap is just a dummy delegate.
>>>>>>> This breaks the contract that its the actual processor being wrapped.
>>>>>>> So now there is a different in semantic between the regular wrap and
>>>>>>> the new wrap with the definition. This is really not good.
>>>>>>>
>>>>>>> I am inclined to vote -1 on this and look for the alternative solution
>>>>>>> with the ProcessorCreater API
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jul 5, 2010 at 5:00 PM, Mark Proctor <[email protected]> 
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> As well as using Policy to wrap the child processors, I'd like to be 
>>>>>>>> able to
>>>>>>>> change/augment the child definitions before they are built. My current 
>>>>>>>> use
>>>>>>>> case for this is so I can augment the DataFormats, with additional
>>>>>>>> configurations. Additionally this Policy also takes care of setting the
>>>>>>>> current context ClassLoader, but that already works fine with the 
>>>>>>>> existing
>>>>>>>> Policy wrap api.
>>>>>>>>
>>>>>>>> API comptability is a problem, but as a sketch to what I'm trying to
>>>>>>>> achieve, here is a change that while a little clunky (due api 
>>>>>>>> compatability)
>>>>>>>> should achieve the job. This allows the wrap to still call the child
>>>>>>>> Process, via the delegate, but to also inspect and change the child
>>>>>>>> Definitions prior to building.
>>>>>>>>
>>>>>>>> public interface DefinitionAwarePolicy extends Policy {
>>>>>>>>    Processor wrap(RouteContext routeContext,
>>>>>>>> ProcessorDefinition<ProcessorDefinition> processorDefinition, Processor
>>>>>>>> processor);
>>>>>>>> }
>>>>>>>>
>>>>>>>> PolicyDefinition.
>>>>>>>>    public Processor createProcessor(RouteContext routeContext) throws
>>>>>>>> Exception {
>>>>>>>>        DelegateProcessor childProcessor = new DelegateProcessor();
>>>>>>>>
>>>>>>>>        Policy policy = resolvePolicy(routeContext);
>>>>>>>>        ObjectHelper.notNull(policy, "policy", this);
>>>>>>>>        Processor target;
>>>>>>>>        if ( policy instanceof Policy ) {
>>>>>>>>            target = policy.wrap(routeContext, childProcessor);
>>>>>>>>        } else {
>>>>>>>>            target = ((DefinitionAwarePolicy)policy).wrap(routeContext,
>>>>>>>> this, childProcessor);
>>>>>>>>        }
>>>>>>>>
>>>>>>>>        childProcessor.setProcessor( 
>>>>>>>> this.createChildProcessor(routeContext,
>>>>>>>> true) );
>>>>>>>>
>>>>>>>>        // wrap the target so it becomes a service and we can manage its
>>>>>>>> lifecycle
>>>>>>>>        WrapProcessor wrap = new WrapProcessor(target, childProcessor);
>>>>>>>>        return wrap;
>>>>>>>>    }
>>>>>>>> --
>>>>>>>> View this message in context: 
>>>>>>>> http://camel.465427.n5.nabble.com/Using-Policy-to-alter-Definitions-before-building-tp696567p696567.html
>>>>>>>> Sent from the Camel - Users mailing list archive at Nabble.com.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Claus Ibsen
>>>>>>> Apache Camel Committer
>>>>>>>
>>>>>>> Author of Camel in Action: http://www.manning.com/ibsen/
>>>>>>> Open Source Integration: http://fusesource.com
>>>>>>> Blog: http://davsclaus.blogspot.com/
>>>>>>> Twitter: http://twitter.com/davsclaus
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Claus Ibsen
>>>>> Apache Camel Committer
>>>>>
>>>>> Author of Camel in Action: http://www.manning.com/ibsen/
>>>>> Open Source Integration: http://fusesource.com
>>>>> Blog: http://davsclaus.blogspot.com/
>>>>> Twitter: http://twitter.com/davsclaus
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Claus Ibsen
>>>> Apache Camel Committer
>>>>
>>>> Author of Camel in Action: http://www.manning.com/ibsen/
>>>> Open Source Integration: http://fusesource.com
>>>> Blog: http://davsclaus.blogspot.com/
>>>> Twitter: http://twitter.com/davsclaus
>>>
>>>
>>
>>
>>
>> --
>> Claus Ibsen
>> Apache Camel Committer
>>
>> Author of Camel in Action: http://www.manning.com/ibsen/
>> Open Source Integration: http://fusesource.com
>> Blog: http://davsclaus.blogspot.com/
>> Twitter: http://twitter.com/davsclaus
>
>



-- 
Claus Ibsen
Apache Camel Committer

Author of Camel in Action: http://www.manning.com/ibsen/
Open Source Integration: http://fusesource.com
Blog: http://davsclaus.blogspot.com/
Twitter: http://twitter.com/davsclaus

Reply via email to