You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@roller.apache.org by David M Johnson <Da...@Sun.COM> on 2005/07/01 16:09:00 UTC
Re: velocity context cleanup
+1!!!
I'm guilty of moving methods like save(), remove(), etc. into the
POJOs. I think everything you've outlined below is basically a good
idea, but I'd like to hold off on it until we merge 2.0 back in.
- Dave
On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
> Okay, so it sounds like a few other people have given this a little
> thought and think that it may be beneficial to make some changes to
> the way the Pojos and PersistentObjects work. I think it would help
> to add a little more detail to the discussion so we know what we are
> talking about. Here's my stab at what changes I would think about
> making ...
>
> - move PersistentObject.save() into Manager classes only
> - move PersistentObject.remove() into Manager classes only
>
> I think those 2 changes would go a long ways toward making it less
> dangerous to make Pojos directly available to users via the velocity
> context. I am in partial agreement that we may not need the
> PersistentObject class at all. Right now I would also consider doing
> ...
>
> - remove PersistentObject.get/setId() (these are not necessarily part
> of all objects)
> - remove PersistentObject.setData() (this can easily be done elsewhere)
> - remove PersistentObject.canSave() (i don't fully understand how this
> is used, but i believe this logic can be in the Manager classes
> save/remove methods)
>
> If we also want to do those last few items then the PersistentObject
> class would basically be useless. I think the first 2 are pretty
> important, but the last 3 are optional. Personally I would probably
> go ahead and ditch the PersistentObject class just because I don't
> think we really need it.
>
> what do others think?
>
> Remember, we are just talking about this right now so please speak up
> and voice your opinion. We aren't going to make any changes right
> away, especially with the fact that Dave has a lot of data model work
> going on for the 2.0 release and we don't want to mess with what he
> has done so far. Once we get a bit more consenus then I will
> formalize a Proposal that can be reviewed again.
>
> -- Allen
>
>
> On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
>> I just wanted to chime in that I really dislike persistence methods
>> being in POJOs also and would be willing to pitch in with moving
>> those out to the appropriate manager classes. In fact, I would even
>> like to see PersistenceObject go as having to extend data objects
>> from it pretty much negates one of the key benefits of Hibernate --
>> its non-intrusiveness into the object model.
>>
>> Max
>>
>> On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
>>
>>>
>>> The remove() method is used in several cases to do some of the
>>> cascading needed to maintain consistency properties. Just make
>>> sure to preserve that logic; if you take out the remove() methods,
>>> this logic needs to be moved into the corresponding manager methods.
>>>
>>> --a.
>>>
>>>
>>> ----- Original Message ----- From: "Lance Lavandowska"
>>> <la...@gmail.com>
>>> To: <ro...@incubator.apache.org>
>>> Sent: Thursday, June 30, 2005 8:24 AM
>>> Subject: Re: velocity context cleanup
>>>
>>>
>>>
>>>> On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>
>>>>
>>>>> *Data.remove() is available to users (try $website.remove() in a
>>>>> template)
>>>>>
>>>>
>>>> This method should probably be removed from the classes. While I
>>>> think even POJOs should contain some business logic, I don't feel
>>>> that
>>>> persistence-related methods are appropriate. Because this is only
>>>> my
>>>> personal gut-check, I've never objected.
>>>>
>>>>
>>>>> PageHelper.evaluateString() is available to users (this one
>>>>> actually bit us in the ass already and a user caught themself in
>>>>> a recursive loop which killed the server)
>>>>>
>>>>
>>>> I'm the one guilty of creating that monstrosity, and I say "get
>>>> rid of
>>>> it". I doubt it is in my real use - but you may break a few pages
>>>> by
>>>> removing it. Perhaps change it to print "THIS MACRO HAS BEEN
>>>> REMOVED"? Note: this is a misguided macro, not a Context value.
>>>>
>>>>
>>>>> Some of these may be a simple case of updating the public,
>>>>> protected, private access levels on methods, but some cases may
>>>>> mean removing objects from the Context and/or removing methods
>>>>> from objects that are part of the Context.
>>>>>
>>>>
>>>> All of the objects placed into the Context are done so to achieve an
>>>> objective in the *.vm templates or the Page templates. As I implied
>>>> above, let's look at what is being exposed by these objects that may
>>>> be 'dangerous' instead.
>>>>
>>>> Lance
>>>
>>
Re: [VOTE] pojo wrapper (was Re: velocity context cleanup)
Posted by Lance Lavandowska <la...@gmail.com>.
+1
On 7/12/05, Dave Johnson <da...@rollerweblogger.org> wrote:
> +1
>
> - Dave
Re: POJO wrappers (was Re: velocity context cleanup)
Posted by Allen Gilliland <Al...@Sun.COM>.
ok, so a slight new twist has entered the mix. i quickly realized that we also need to deal with the fact that some pojo methods actually return other pojos or collecitons of pojos which need to wrapped as well.
i think i've got a solution for this already worked out, but in the spirit of collaboration i thought i'd ask for thoughts from others before settling. here's how it works ...
xdoclet is run and generates a <POJO>Wrapper class for all pojos specified. developers must tag each method that is going to be exposed with one of the lines below ...
/**
* @roller.wrapPojoMethod type="simple"
* @roller.wrapPojoMethod type="pojo"
* @roller.wrapPojoMethod type="pojo-collection" class="org.roller.pojos.pojoclass"
*/
a "simple" wrapped method just returns the same type of object as the original method, so this is meant for most methods that return Strings, ints, etc. a "pojo" wrapped method identifies a method that returns another pojo and this gives xdoclet a chance to alter the method code so that it can return a wrapped pojo instead of the original pojo. a "pojo-collection" is a method which returns some collection of pojos. xdoclet handles "pojo-collection" methods by inserting code that iterates through the contents of the collection and wraps each object.
here is some example output from each of the 3 wrap types ...
// define a simple wrapped method
public java.lang.String getId()
{
return this.pojo.getId();
}
// this method returns another pojo, so we need to wrap that pojo as well
public org.roller.pojos.wrapper.WeblogCategoryDataWrapper getCategory()
{
return new org.roller.pojos.wrapper.WeblogCategoryDataWrapper(this.pojo.getCategory());
}
// this method returns a collection of pojos, so we need to wrap the collection contents
public java.util.Collection getEntryAttributes()
{
// first get the collection
java.util.Set initialCollection = this.pojo.getEntryAttributes();
// iterate through and wrap
// we force the use of an ArrayList because it should be good enough to cover
// for any Collection type we encounter.
java.util.ArrayList wrappedCollection = new java.util.ArrayList(initialCollection.size());
java.util.Iterator it = initialCollection.iterator();
int i = 0;
while(it.hasNext()) {
wrappedCollection.add(i, new org.roller.pojos.wrapper.EntryAttributeDataWrapper((org.roller.pojos.EntryAttributeData) it.next()));
i++;
}
return wrappedCollection;
}
so, what do people think? is that good enough? does anyone have any other ideas on how this might be done better?
-- Allen
On Wed, 2005-07-13 at 07:28, Anil Gangolli wrote:
> Allen:
>
> You raise some good points.
>
> I'm ok with using XDoclet to generate the wrappers directly, and also recognize that there may be more issues with either
> implementation as you get down to the details. I'm happy to leave that to your judgement.
>
> I think we all want low maintenance of the wrapping code. The dynamic proxy + X-Doclet-generated restriction interfaces idea was
> just a means to achieve this, and your suggestion of moving the delegation write into the X-doclet generated code should be fine
> too.
>
> --a.
>
>
> ----- Original Message -----
> From: "Allen Gilliland" <Al...@Sun.COM>
> To: "roller-dev" <ro...@incubator.apache.org>
> Sent: Tuesday, July 12, 2005 3:16 PM
> Subject: Re: velocity context cleanup
>
>
> >I was just thinking about this a little more and I am actually starting to think that the dynamic proxy might not be the better
> >approach. If we are going to have xdoclet auto generate the restricted interfaces, then why not have it generate wrapper classes
> >instead?
> >
> > The big bonus I see with the wrapper classes is that we get compile time validation that a wrapper exists for the pojo and that
> > the coder has applied the right wrapper. With the dynamic proxy you could easily call ctx.put("obj",
> > VelocityWrapper.wrap(myPojo)) but you don't find out if there is something wrong until runtime.
> >
> > My other slight uneasyness with the proxy solution (and this is mainly a pet peeve) is that it makes me uncomfortable when a class
> > implements an interface that is not generated until build time. Auto generated wrapper classes isn't a whole lot better, but I
> > still prefer to have important core code like pojos to not be reliant on auto generated code if possible.
> >
> > I am still okay with the proxy solution if that's what everyone else wants, but my personal preference is to avoid mucking with
> > things at runtime when we don't really need to.
> >
> > -- Allen
> >
> >
> > On Sun, 2005-07-10 at 08:27, Anil Gangolli wrote:
> >> An additional suggestion: the "restriction interfaces" (see below) could be constructed using XDoclet, so that we could just tag
> >> the
> >> bean methods to be exposed. One has to write a suitable XDoclet template for it, though.
> >>
> >> --a.
> >>
> >>
> >>
> >> ----- Original Message -----
> >> From: "Dave Johnson" <da...@rollerweblogger.org>
> >> To: <ro...@incubator.apache.org>
> >> Sent: Friday, July 08, 2005 3:40 PM
> >> Subject: Re: velocity context cleanup
> >>
> >>
> >> >
> >> > +1 on dynamic proxies!
> >> >
> >> > - Dave
> >> >
> >> >
> >> > On Jul 6, 2005, at 11:17 AM, Anil Gangolli wrote:
> >> >
> >> >>
> >> >> Yes, I meant using the dynamic proxy facilities introduced in Java 1.3 (see, e.g.
> >> >> http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
> >> >>
> >> >> I apologize for the terseness. Here's a more complete description.
> >> >>
> >> >> Note that the success of this approach willl hinge on the reflection of the proxy class seen by Velocity as being the
> >> >> restricted
> >> >> interface desired to be exposed. Reading "Proxy Class Properties" in the above document, this should be the case.
> >> >>
> >> >> (1) Define a simple invocation handler class whose invoke() method just does an m.invoke(obj) on the passed in params,
> >> >> and just unwraps InvocationTargetException to throw back any originating Throwable it contains.
> >> >> See the examples in the doc above.
> >> >>
> >> >> (2) Define your restriction interfaces whose names can be derived by convention from the name of the associated POJO
> >> >> (like org.roller.pojos.Template -> org.roller.presentation.velocity.wrappers.Template or TemplateWrapper). You must
> >> >> define an interface to use a dynamic proxy.
> >> >>
> >> >> (3) Define a single proxy factory method wrap() that generates a proxy. [With the following example code it would be
> >> >> VelocityWrapper.wrap(thePojo)]. You just call this and place the result into the Velocity context.
> >> >>
> >> >> Here is APPROXIMATE pseudo-code for illustration. Note: this won't compile for sure without a createWrapperFromPojoName()
> >> >> method
> >> >> defined. You can get the intent anyway.
> >> >>
> >> >> public class VelocityWrapper {
> >> >>
> >> >> public static Object wrap(Object pojo) {
> >> >> // Determine the wrapper interface class name from the pojo class name
> >> >> String restrictionInterfaceName = createWrapperNameFromPojoName(pojo.getClass().getName());
> >> >> // Get that class an return a proxy instance with the invocation handler below.
> >> >> Class restrictionInterface = pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName);
> >> >> SimpleInvocationHandler handler = new SimpleInvocationHandler(pojo);
> >> >> return Proxy.newProxyInstance(pojo.getClass().getClassLoader,
> >> >> new Class[] { restrictionInterface },
> >> >> handler);
> >> >> }
> >> >>
> >> >> public static class SimpleInvocationHandler implements InvocationHandler {
> >> >> private Object theWrappedPojo;
> >> >>
> >> >> SimpleInvocationHandler(Object pojo) {
> >> >> // When constructed, remember the instance we were constructed with
> >> >> theWrappedPojo = pojo;
> >> >> }
> >> >>
> >> >> public Object invoke(Object proxy, method m, Object[] args) throws Throwable {
> >> >> try {
> >> >> return m.invoke(theWrappedPojo, args);
> >> >> } catch (InvocationTargetException e) {
> >> >> // rethrow the original exception to make the wrapping transparent
> >> >> throw e.getTargetException();
> >> >> }
> >> >> }
> >> >> }
> >> >> }
> >> >>
> >> >>
> >> >>
> >> >>
> >> >>
> >> >> ----- Original Message ----- From: "Rudman Max" <mr...@steelbrick.com>
> >> >> To: <ro...@incubator.apache.org>
> >> >> Sent: Tuesday, July 05, 2005 10:07 PM
> >> >> Subject: Re: velocity context cleanup
> >> >>
> >> >>
> >> >>> I think he is talking about dynamic proxy facilities provided by java.lang.reflect.Proxy. Instead of creating a wrapper
> >> >>> class
> >> >>> for each POJO you could deposit a dynamically proxy class in the Velocity context constructed with InvocationHandler
> >> >>> implementation which blocks all method calls unless they begin with "set/is". I personally think this a really good idea.
> >> >>>
> >> >>> Max
> >> >>>
> >> >>> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
> >> >>>
> >> >>>> I'm not sure what you mean by dynamic proxy. Could you give more info.
> >> >>>>
> >> >>>> -- Allen
> >> >>>>
> >> >>>>
> >> >>>> Anil Gangolli wrote:
> >> >>>>
> >> >>>>
> >> >>>>>
> >> >>>>> Just a quick note, and I admit I haven't followed the latest discussion, but if the wrappers are merely restrictions by a
> >> >>>>> specified interface, it seems like a single dynamic proxy could implement all of them.
> >> >>>>>
> >> >>>>> --a.
> >> >>>>>
> >> >>>>> ----- Original Message ----- From: "Allen Gilliland" <Al...@Sun.COM>
> >> >>>>> To: <ro...@incubator.apache.org>
> >> >>>>> Sent: Tuesday, July 05, 2005 3:31 PM
> >> >>>>> Subject: Re: velocity context cleanup
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>>> agreed. so the convention will be ...
> >> >>>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
> >> >>>>>>
> >> >>>>>> will act as a wrapper class for a <POJO Class> normally found in org.roller.pojos
> >> >>>>>>
> >> >>>>>> -- Allen
> >> >>>>>>
> >> >>>>>>
> >> >>>>>> Lance Lavandowska wrote:
> >> >>>>>>
> >> >>>>>>
> >> >>>>>>> Ooops, you caught me not paying sufficient attention, even whilst I
> >> >>>>>>> was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
> >> >>>>>>> better, less confusion with the "real" "pojos".
> >> >>>>>>>
> >> >>>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
> >> >>>>>>>
> >> >>>>>>>
> >> >>>>>>>> i can do that, but org.roller.presentation.velocity.pojos *is* a new
> >> >>>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers would be
> >> >>>>>>>> more clear?
> >> >>>>>>>>
> >> >>>>>>>> -- Allen
> >> >>>>>>>>
> >> >>>>>>>>
> >> >>>>>>>> Lance Lavandowska wrote:
> >> >>>>>>>>
> >> >>>>>>>>
> >> >>>>>>>>
> >> >>>>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
> >> >>>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
> >> >>>>>>>>>
> >> >>>>>>>>>
> >> >>>>>>
> >> >>>>>>
> >> >>>>>
> >> >>
> >> >
> >>
> >
>
POJO wrappers (was Re: velocity context cleanup)
Posted by Anil Gangolli <an...@busybuddha.org>.
Allen:
You raise some good points.
I'm ok with using XDoclet to generate the wrappers directly, and also recognize that there may be more issues with either
implementation as you get down to the details. I'm happy to leave that to your judgement.
I think we all want low maintenance of the wrapping code. The dynamic proxy + X-Doclet-generated restriction interfaces idea was
just a means to achieve this, and your suggestion of moving the delegation write into the X-doclet generated code should be fine
too.
--a.
----- Original Message -----
From: "Allen Gilliland" <Al...@Sun.COM>
To: "roller-dev" <ro...@incubator.apache.org>
Sent: Tuesday, July 12, 2005 3:16 PM
Subject: Re: velocity context cleanup
>I was just thinking about this a little more and I am actually starting to think that the dynamic proxy might not be the better
>approach. If we are going to have xdoclet auto generate the restricted interfaces, then why not have it generate wrapper classes
>instead?
>
> The big bonus I see with the wrapper classes is that we get compile time validation that a wrapper exists for the pojo and that
> the coder has applied the right wrapper. With the dynamic proxy you could easily call ctx.put("obj",
> VelocityWrapper.wrap(myPojo)) but you don't find out if there is something wrong until runtime.
>
> My other slight uneasyness with the proxy solution (and this is mainly a pet peeve) is that it makes me uncomfortable when a class
> implements an interface that is not generated until build time. Auto generated wrapper classes isn't a whole lot better, but I
> still prefer to have important core code like pojos to not be reliant on auto generated code if possible.
>
> I am still okay with the proxy solution if that's what everyone else wants, but my personal preference is to avoid mucking with
> things at runtime when we don't really need to.
>
> -- Allen
>
>
> On Sun, 2005-07-10 at 08:27, Anil Gangolli wrote:
>> An additional suggestion: the "restriction interfaces" (see below) could be constructed using XDoclet, so that we could just tag
>> the
>> bean methods to be exposed. One has to write a suitable XDoclet template for it, though.
>>
>> --a.
>>
>>
>>
>> ----- Original Message -----
>> From: "Dave Johnson" <da...@rollerweblogger.org>
>> To: <ro...@incubator.apache.org>
>> Sent: Friday, July 08, 2005 3:40 PM
>> Subject: Re: velocity context cleanup
>>
>>
>> >
>> > +1 on dynamic proxies!
>> >
>> > - Dave
>> >
>> >
>> > On Jul 6, 2005, at 11:17 AM, Anil Gangolli wrote:
>> >
>> >>
>> >> Yes, I meant using the dynamic proxy facilities introduced in Java 1.3 (see, e.g.
>> >> http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
>> >>
>> >> I apologize for the terseness. Here's a more complete description.
>> >>
>> >> Note that the success of this approach willl hinge on the reflection of the proxy class seen by Velocity as being the
>> >> restricted
>> >> interface desired to be exposed. Reading "Proxy Class Properties" in the above document, this should be the case.
>> >>
>> >> (1) Define a simple invocation handler class whose invoke() method just does an m.invoke(obj) on the passed in params,
>> >> and just unwraps InvocationTargetException to throw back any originating Throwable it contains.
>> >> See the examples in the doc above.
>> >>
>> >> (2) Define your restriction interfaces whose names can be derived by convention from the name of the associated POJO
>> >> (like org.roller.pojos.Template -> org.roller.presentation.velocity.wrappers.Template or TemplateWrapper). You must
>> >> define an interface to use a dynamic proxy.
>> >>
>> >> (3) Define a single proxy factory method wrap() that generates a proxy. [With the following example code it would be
>> >> VelocityWrapper.wrap(thePojo)]. You just call this and place the result into the Velocity context.
>> >>
>> >> Here is APPROXIMATE pseudo-code for illustration. Note: this won't compile for sure without a createWrapperFromPojoName()
>> >> method
>> >> defined. You can get the intent anyway.
>> >>
>> >> public class VelocityWrapper {
>> >>
>> >> public static Object wrap(Object pojo) {
>> >> // Determine the wrapper interface class name from the pojo class name
>> >> String restrictionInterfaceName = createWrapperNameFromPojoName(pojo.getClass().getName());
>> >> // Get that class an return a proxy instance with the invocation handler below.
>> >> Class restrictionInterface = pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName);
>> >> SimpleInvocationHandler handler = new SimpleInvocationHandler(pojo);
>> >> return Proxy.newProxyInstance(pojo.getClass().getClassLoader,
>> >> new Class[] { restrictionInterface },
>> >> handler);
>> >> }
>> >>
>> >> public static class SimpleInvocationHandler implements InvocationHandler {
>> >> private Object theWrappedPojo;
>> >>
>> >> SimpleInvocationHandler(Object pojo) {
>> >> // When constructed, remember the instance we were constructed with
>> >> theWrappedPojo = pojo;
>> >> }
>> >>
>> >> public Object invoke(Object proxy, method m, Object[] args) throws Throwable {
>> >> try {
>> >> return m.invoke(theWrappedPojo, args);
>> >> } catch (InvocationTargetException e) {
>> >> // rethrow the original exception to make the wrapping transparent
>> >> throw e.getTargetException();
>> >> }
>> >> }
>> >> }
>> >> }
>> >>
>> >>
>> >>
>> >>
>> >>
>> >> ----- Original Message ----- From: "Rudman Max" <mr...@steelbrick.com>
>> >> To: <ro...@incubator.apache.org>
>> >> Sent: Tuesday, July 05, 2005 10:07 PM
>> >> Subject: Re: velocity context cleanup
>> >>
>> >>
>> >>> I think he is talking about dynamic proxy facilities provided by java.lang.reflect.Proxy. Instead of creating a wrapper
>> >>> class
>> >>> for each POJO you could deposit a dynamically proxy class in the Velocity context constructed with InvocationHandler
>> >>> implementation which blocks all method calls unless they begin with "set/is". I personally think this a really good idea.
>> >>>
>> >>> Max
>> >>>
>> >>> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
>> >>>
>> >>>> I'm not sure what you mean by dynamic proxy. Could you give more info.
>> >>>>
>> >>>> -- Allen
>> >>>>
>> >>>>
>> >>>> Anil Gangolli wrote:
>> >>>>
>> >>>>
>> >>>>>
>> >>>>> Just a quick note, and I admit I haven't followed the latest discussion, but if the wrappers are merely restrictions by a
>> >>>>> specified interface, it seems like a single dynamic proxy could implement all of them.
>> >>>>>
>> >>>>> --a.
>> >>>>>
>> >>>>> ----- Original Message ----- From: "Allen Gilliland" <Al...@Sun.COM>
>> >>>>> To: <ro...@incubator.apache.org>
>> >>>>> Sent: Tuesday, July 05, 2005 3:31 PM
>> >>>>> Subject: Re: velocity context cleanup
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>>> agreed. so the convention will be ...
>> >>>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>> >>>>>>
>> >>>>>> will act as a wrapper class for a <POJO Class> normally found in org.roller.pojos
>> >>>>>>
>> >>>>>> -- Allen
>> >>>>>>
>> >>>>>>
>> >>>>>> Lance Lavandowska wrote:
>> >>>>>>
>> >>>>>>
>> >>>>>>> Ooops, you caught me not paying sufficient attention, even whilst I
>> >>>>>>> was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
>> >>>>>>> better, less confusion with the "real" "pojos".
>> >>>>>>>
>> >>>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>> i can do that, but org.roller.presentation.velocity.pojos *is* a new
>> >>>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers would be
>> >>>>>>>> more clear?
>> >>>>>>>>
>> >>>>>>>> -- Allen
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> Lance Lavandowska wrote:
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
>> >>>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
>> >>>>>>>>>
>> >>>>>>>>>
>> >>>>>>
>> >>>>>>
>> >>>>>
>> >>
>> >
>>
>
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
I was just thinking about this a little more and I am actually starting to think that the dynamic proxy might not be the better approach. If we are going to have xdoclet auto generate the restricted interfaces, then why not have it generate wrapper classes instead?
The big bonus I see with the wrapper classes is that we get compile time validation that a wrapper exists for the pojo and that the coder has applied the right wrapper. With the dynamic proxy you could easily call ctx.put("obj", VelocityWrapper.wrap(myPojo)) but you don't find out if there is something wrong until runtime.
My other slight uneasyness with the proxy solution (and this is mainly a pet peeve) is that it makes me uncomfortable when a class implements an interface that is not generated until build time. Auto generated wrapper classes isn't a whole lot better, but I still prefer to have important core code like pojos to not be reliant on auto generated code if possible.
I am still okay with the proxy solution if that's what everyone else wants, but my personal preference is to avoid mucking with things at runtime when we don't really need to.
-- Allen
On Sun, 2005-07-10 at 08:27, Anil Gangolli wrote:
> An additional suggestion: the "restriction interfaces" (see below) could be constructed using XDoclet, so that we could just tag the
> bean methods to be exposed. One has to write a suitable XDoclet template for it, though.
>
> --a.
>
>
>
> ----- Original Message -----
> From: "Dave Johnson" <da...@rollerweblogger.org>
> To: <ro...@incubator.apache.org>
> Sent: Friday, July 08, 2005 3:40 PM
> Subject: Re: velocity context cleanup
>
>
> >
> > +1 on dynamic proxies!
> >
> > - Dave
> >
> >
> > On Jul 6, 2005, at 11:17 AM, Anil Gangolli wrote:
> >
> >>
> >> Yes, I meant using the dynamic proxy facilities introduced in Java 1.3 (see, e.g.
> >> http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
> >>
> >> I apologize for the terseness. Here's a more complete description.
> >>
> >> Note that the success of this approach willl hinge on the reflection of the proxy class seen by Velocity as being the restricted
> >> interface desired to be exposed. Reading "Proxy Class Properties" in the above document, this should be the case.
> >>
> >> (1) Define a simple invocation handler class whose invoke() method just does an m.invoke(obj) on the passed in params,
> >> and just unwraps InvocationTargetException to throw back any originating Throwable it contains.
> >> See the examples in the doc above.
> >>
> >> (2) Define your restriction interfaces whose names can be derived by convention from the name of the associated POJO
> >> (like org.roller.pojos.Template -> org.roller.presentation.velocity.wrappers.Template or TemplateWrapper). You must
> >> define an interface to use a dynamic proxy.
> >>
> >> (3) Define a single proxy factory method wrap() that generates a proxy. [With the following example code it would be
> >> VelocityWrapper.wrap(thePojo)]. You just call this and place the result into the Velocity context.
> >>
> >> Here is APPROXIMATE pseudo-code for illustration. Note: this won't compile for sure without a createWrapperFromPojoName() method
> >> defined. You can get the intent anyway.
> >>
> >> public class VelocityWrapper {
> >>
> >> public static Object wrap(Object pojo) {
> >> // Determine the wrapper interface class name from the pojo class name
> >> String restrictionInterfaceName = createWrapperNameFromPojoName(pojo.getClass().getName());
> >> // Get that class an return a proxy instance with the invocation handler below.
> >> Class restrictionInterface = pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName);
> >> SimpleInvocationHandler handler = new SimpleInvocationHandler(pojo);
> >> return Proxy.newProxyInstance(pojo.getClass().getClassLoader,
> >> new Class[] { restrictionInterface },
> >> handler);
> >> }
> >>
> >> public static class SimpleInvocationHandler implements InvocationHandler {
> >> private Object theWrappedPojo;
> >>
> >> SimpleInvocationHandler(Object pojo) {
> >> // When constructed, remember the instance we were constructed with
> >> theWrappedPojo = pojo;
> >> }
> >>
> >> public Object invoke(Object proxy, method m, Object[] args) throws Throwable {
> >> try {
> >> return m.invoke(theWrappedPojo, args);
> >> } catch (InvocationTargetException e) {
> >> // rethrow the original exception to make the wrapping transparent
> >> throw e.getTargetException();
> >> }
> >> }
> >> }
> >> }
> >>
> >>
> >>
> >>
> >>
> >> ----- Original Message ----- From: "Rudman Max" <mr...@steelbrick.com>
> >> To: <ro...@incubator.apache.org>
> >> Sent: Tuesday, July 05, 2005 10:07 PM
> >> Subject: Re: velocity context cleanup
> >>
> >>
> >>> I think he is talking about dynamic proxy facilities provided by java.lang.reflect.Proxy. Instead of creating a wrapper class
> >>> for each POJO you could deposit a dynamically proxy class in the Velocity context constructed with InvocationHandler
> >>> implementation which blocks all method calls unless they begin with "set/is". I personally think this a really good idea.
> >>>
> >>> Max
> >>>
> >>> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
> >>>
> >>>> I'm not sure what you mean by dynamic proxy. Could you give more info.
> >>>>
> >>>> -- Allen
> >>>>
> >>>>
> >>>> Anil Gangolli wrote:
> >>>>
> >>>>
> >>>>>
> >>>>> Just a quick note, and I admit I haven't followed the latest discussion, but if the wrappers are merely restrictions by a
> >>>>> specified interface, it seems like a single dynamic proxy could implement all of them.
> >>>>>
> >>>>> --a.
> >>>>>
> >>>>> ----- Original Message ----- From: "Allen Gilliland" <Al...@Sun.COM>
> >>>>> To: <ro...@incubator.apache.org>
> >>>>> Sent: Tuesday, July 05, 2005 3:31 PM
> >>>>> Subject: Re: velocity context cleanup
> >>>>>
> >>>>>
> >>>>>
> >>>>>> agreed. so the convention will be ...
> >>>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
> >>>>>>
> >>>>>> will act as a wrapper class for a <POJO Class> normally found in org.roller.pojos
> >>>>>>
> >>>>>> -- Allen
> >>>>>>
> >>>>>>
> >>>>>> Lance Lavandowska wrote:
> >>>>>>
> >>>>>>
> >>>>>>> Ooops, you caught me not paying sufficient attention, even whilst I
> >>>>>>> was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
> >>>>>>> better, less confusion with the "real" "pojos".
> >>>>>>>
> >>>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>> i can do that, but org.roller.presentation.velocity.pojos *is* a new
> >>>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers would be
> >>>>>>>> more clear?
> >>>>>>>>
> >>>>>>>> -- Allen
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Lance Lavandowska wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
> >>>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>
> >
>
[VOTE] pojo wrapper (was Re: velocity context cleanup)
Posted by Dave Johnson <da...@rollerweblogger.org>.
+1
- Dave
On Jul 12, 2005, at 5:41 PM, Allen Gilliland wrote:
> Okay, so it sounds like we are all pretty much agreed on doing a
> dynamic proxy wrapper for the pojos. This is probably simple enough
> that we didn't need a formal proposal, but I collected the relevant
> bits and put them on the wiki.
>
> http://www.rollerweblogger.org/wiki/Wiki.jsp?page=PojoWrapper
>
> how about a final vote to make sure?
>
> Also, unless someone else wants to tackle this reasonably soon then
> I'll go ahead and commit to it. I think this is a pretty important
> item and i'd like to have it included in our next release for
> blogs.sun.com.
>
> -- Allen
>
>
> On Sun, 2005-07-10 at 08:27, Anil Gangolli wrote:
>> An additional suggestion: the "restriction interfaces" (see below)
>> could be constructed using XDoclet, so that we could just tag the
>> bean methods to be exposed. One has to write a suitable XDoclet
>> template for it, though.
>>
>> --a.
>>
>>
>>
>> ----- Original Message -----
>> From: "Dave Johnson" <da...@rollerweblogger.org>
>> To: <ro...@incubator.apache.org>
>> Sent: Friday, July 08, 2005 3:40 PM
>> Subject: Re: velocity context cleanup
>>
>>
>>>
>>> +1 on dynamic proxies!
>>>
>>> - Dave
>>>
>>>
>>> On Jul 6, 2005, at 11:17 AM, Anil Gangolli wrote:
>>>
>>>>
>>>> Yes, I meant using the dynamic proxy facilities introduced in Java
>>>> 1.3 (see, e.g.
>>>> http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
>>>>
>>>> I apologize for the terseness. Here's a more complete description.
>>>>
>>>> Note that the success of this approach willl hinge on the
>>>> reflection of the proxy class seen by Velocity as being the
>>>> restricted
>>>> interface desired to be exposed. Reading "Proxy Class Properties"
>>>> in the above document, this should be the case.
>>>>
>>>> (1) Define a simple invocation handler class whose invoke() method
>>>> just does an m.invoke(obj) on the passed in params,
>>>> and just unwraps InvocationTargetException to throw back any
>>>> originating Throwable it contains.
>>>> See the examples in the doc above.
>>>>
>>>> (2) Define your restriction interfaces whose names can be derived
>>>> by convention from the name of the associated POJO
>>>> (like org.roller.pojos.Template ->
>>>> org.roller.presentation.velocity.wrappers.Template or
>>>> TemplateWrapper). You must
>>>> define an interface to use a dynamic proxy.
>>>>
>>>> (3) Define a single proxy factory method wrap() that generates a
>>>> proxy. [With the following example code it would be
>>>> VelocityWrapper.wrap(thePojo)]. You just call this and place the
>>>> result into the Velocity context.
>>>>
>>>> Here is APPROXIMATE pseudo-code for illustration. Note: this won't
>>>> compile for sure without a createWrapperFromPojoName() method
>>>> defined. You can get the intent anyway.
>>>>
>>>> public class VelocityWrapper {
>>>>
>>>> public static Object wrap(Object pojo) {
>>>> // Determine the wrapper interface class name from the
>>>> pojo class name
>>>> String restrictionInterfaceName =
>>>> createWrapperNameFromPojoName(pojo.getClass().getName());
>>>> // Get that class an return a proxy instance with the
>>>> invocation handler below.
>>>> Class restrictionInterface =
>>>> pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName)
>>>> ;
>>>> SimpleInvocationHandler handler = new
>>>> SimpleInvocationHandler(pojo);
>>>> return
>>>> Proxy.newProxyInstance(pojo.getClass().getClassLoader,
>>>> new
>>>> Class[] { restrictionInterface },
>>>> handler);
>>>> }
>>>>
>>>> public static class SimpleInvocationHandler implements
>>>> InvocationHandler {
>>>> private Object theWrappedPojo;
>>>>
>>>> SimpleInvocationHandler(Object pojo) {
>>>> // When constructed, remember the instance we
>>>> were constructed with
>>>> theWrappedPojo = pojo;
>>>> }
>>>>
>>>> public Object invoke(Object proxy, method m,
>>>> Object[] args) throws Throwable {
>>>> try {
>>>> return m.invoke(theWrappedPojo, args);
>>>> } catch (InvocationTargetException e) {
>>>> // rethrow the original exception to make
>>>> the wrapping transparent
>>>> throw e.getTargetException();
>>>> }
>>>> }
>>>> }
>>>> }
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> ----- Original Message ----- From: "Rudman Max"
>>>> <mr...@steelbrick.com>
>>>> To: <ro...@incubator.apache.org>
>>>> Sent: Tuesday, July 05, 2005 10:07 PM
>>>> Subject: Re: velocity context cleanup
>>>>
>>>>
>>>>> I think he is talking about dynamic proxy facilities provided by
>>>>> java.lang.reflect.Proxy. Instead of creating a wrapper class
>>>>> for each POJO you could deposit a dynamically proxy class in the
>>>>> Velocity context constructed with InvocationHandler
>>>>> implementation which blocks all method calls unless they begin
>>>>> with "set/is". I personally think this a really good idea.
>>>>>
>>>>> Max
>>>>>
>>>>> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
>>>>>
>>>>>> I'm not sure what you mean by dynamic proxy. Could you give more
>>>>>> info.
>>>>>>
>>>>>> -- Allen
>>>>>>
>>>>>>
>>>>>> Anil Gangolli wrote:
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Just a quick note, and I admit I haven't followed the latest
>>>>>>> discussion, but if the wrappers are merely restrictions by a
>>>>>>> specified interface, it seems like a single dynamic proxy could
>>>>>>> implement all of them.
>>>>>>>
>>>>>>> --a.
>>>>>>>
>>>>>>> ----- Original Message ----- From: "Allen Gilliland"
>>>>>>> <Al...@Sun.COM>
>>>>>>> To: <ro...@incubator.apache.org>
>>>>>>> Sent: Tuesday, July 05, 2005 3:31 PM
>>>>>>> Subject: Re: velocity context cleanup
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> agreed. so the convention will be ...
>>>>>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>>>>>>>>
>>>>>>>> will act as a wrapper class for a <POJO Class> normally found
>>>>>>>> in org.roller.pojos
>>>>>>>>
>>>>>>>> -- Allen
>>>>>>>>
>>>>>>>>
>>>>>>>> Lance Lavandowska wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> Ooops, you caught me not paying sufficient attention, even
>>>>>>>>> whilst I
>>>>>>>>> was typing out the package name! Hmm, I think I like
>>>>>>>>> o.r.p.v.wrappers
>>>>>>>>> better, less confusion with the "real" "pojos".
>>>>>>>>>
>>>>>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> i can do that, but org.roller.presentation.velocity.pojos
>>>>>>>>>> *is* a new
>>>>>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers
>>>>>>>>>> would be
>>>>>>>>>> more clear?
>>>>>>>>>>
>>>>>>>>>> -- Allen
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Lance Lavandowska wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> Just one suggestion, put the wrappers in a sub-package,
>>>>>>>>>>> perhaps
>>>>>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>
>>>
>>
>
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
Okay, so it sounds like we are all pretty much agreed on doing a dynamic proxy wrapper for the pojos. This is probably simple enough that we didn't need a formal proposal, but I collected the relevant bits and put them on the wiki.
http://www.rollerweblogger.org/wiki/Wiki.jsp?page=PojoWrapper
how about a final vote to make sure?
Also, unless someone else wants to tackle this reasonably soon then I'll go ahead and commit to it. I think this is a pretty important item and i'd like to have it included in our next release for blogs.sun.com.
-- Allen
On Sun, 2005-07-10 at 08:27, Anil Gangolli wrote:
> An additional suggestion: the "restriction interfaces" (see below) could be constructed using XDoclet, so that we could just tag the
> bean methods to be exposed. One has to write a suitable XDoclet template for it, though.
>
> --a.
>
>
>
> ----- Original Message -----
> From: "Dave Johnson" <da...@rollerweblogger.org>
> To: <ro...@incubator.apache.org>
> Sent: Friday, July 08, 2005 3:40 PM
> Subject: Re: velocity context cleanup
>
>
> >
> > +1 on dynamic proxies!
> >
> > - Dave
> >
> >
> > On Jul 6, 2005, at 11:17 AM, Anil Gangolli wrote:
> >
> >>
> >> Yes, I meant using the dynamic proxy facilities introduced in Java 1.3 (see, e.g.
> >> http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
> >>
> >> I apologize for the terseness. Here's a more complete description.
> >>
> >> Note that the success of this approach willl hinge on the reflection of the proxy class seen by Velocity as being the restricted
> >> interface desired to be exposed. Reading "Proxy Class Properties" in the above document, this should be the case.
> >>
> >> (1) Define a simple invocation handler class whose invoke() method just does an m.invoke(obj) on the passed in params,
> >> and just unwraps InvocationTargetException to throw back any originating Throwable it contains.
> >> See the examples in the doc above.
> >>
> >> (2) Define your restriction interfaces whose names can be derived by convention from the name of the associated POJO
> >> (like org.roller.pojos.Template -> org.roller.presentation.velocity.wrappers.Template or TemplateWrapper). You must
> >> define an interface to use a dynamic proxy.
> >>
> >> (3) Define a single proxy factory method wrap() that generates a proxy. [With the following example code it would be
> >> VelocityWrapper.wrap(thePojo)]. You just call this and place the result into the Velocity context.
> >>
> >> Here is APPROXIMATE pseudo-code for illustration. Note: this won't compile for sure without a createWrapperFromPojoName() method
> >> defined. You can get the intent anyway.
> >>
> >> public class VelocityWrapper {
> >>
> >> public static Object wrap(Object pojo) {
> >> // Determine the wrapper interface class name from the pojo class name
> >> String restrictionInterfaceName = createWrapperNameFromPojoName(pojo.getClass().getName());
> >> // Get that class an return a proxy instance with the invocation handler below.
> >> Class restrictionInterface = pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName);
> >> SimpleInvocationHandler handler = new SimpleInvocationHandler(pojo);
> >> return Proxy.newProxyInstance(pojo.getClass().getClassLoader,
> >> new Class[] { restrictionInterface },
> >> handler);
> >> }
> >>
> >> public static class SimpleInvocationHandler implements InvocationHandler {
> >> private Object theWrappedPojo;
> >>
> >> SimpleInvocationHandler(Object pojo) {
> >> // When constructed, remember the instance we were constructed with
> >> theWrappedPojo = pojo;
> >> }
> >>
> >> public Object invoke(Object proxy, method m, Object[] args) throws Throwable {
> >> try {
> >> return m.invoke(theWrappedPojo, args);
> >> } catch (InvocationTargetException e) {
> >> // rethrow the original exception to make the wrapping transparent
> >> throw e.getTargetException();
> >> }
> >> }
> >> }
> >> }
> >>
> >>
> >>
> >>
> >>
> >> ----- Original Message ----- From: "Rudman Max" <mr...@steelbrick.com>
> >> To: <ro...@incubator.apache.org>
> >> Sent: Tuesday, July 05, 2005 10:07 PM
> >> Subject: Re: velocity context cleanup
> >>
> >>
> >>> I think he is talking about dynamic proxy facilities provided by java.lang.reflect.Proxy. Instead of creating a wrapper class
> >>> for each POJO you could deposit a dynamically proxy class in the Velocity context constructed with InvocationHandler
> >>> implementation which blocks all method calls unless they begin with "set/is". I personally think this a really good idea.
> >>>
> >>> Max
> >>>
> >>> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
> >>>
> >>>> I'm not sure what you mean by dynamic proxy. Could you give more info.
> >>>>
> >>>> -- Allen
> >>>>
> >>>>
> >>>> Anil Gangolli wrote:
> >>>>
> >>>>
> >>>>>
> >>>>> Just a quick note, and I admit I haven't followed the latest discussion, but if the wrappers are merely restrictions by a
> >>>>> specified interface, it seems like a single dynamic proxy could implement all of them.
> >>>>>
> >>>>> --a.
> >>>>>
> >>>>> ----- Original Message ----- From: "Allen Gilliland" <Al...@Sun.COM>
> >>>>> To: <ro...@incubator.apache.org>
> >>>>> Sent: Tuesday, July 05, 2005 3:31 PM
> >>>>> Subject: Re: velocity context cleanup
> >>>>>
> >>>>>
> >>>>>
> >>>>>> agreed. so the convention will be ...
> >>>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
> >>>>>>
> >>>>>> will act as a wrapper class for a <POJO Class> normally found in org.roller.pojos
> >>>>>>
> >>>>>> -- Allen
> >>>>>>
> >>>>>>
> >>>>>> Lance Lavandowska wrote:
> >>>>>>
> >>>>>>
> >>>>>>> Ooops, you caught me not paying sufficient attention, even whilst I
> >>>>>>> was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
> >>>>>>> better, less confusion with the "real" "pojos".
> >>>>>>>
> >>>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>> i can do that, but org.roller.presentation.velocity.pojos *is* a new
> >>>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers would be
> >>>>>>>> more clear?
> >>>>>>>>
> >>>>>>>> -- Allen
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Lance Lavandowska wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
> >>>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>
> >
>
Re: velocity context cleanup
Posted by Anil Gangolli <an...@busybuddha.org>.
An additional suggestion: the "restriction interfaces" (see below) could be constructed using XDoclet, so that we could just tag the
bean methods to be exposed. One has to write a suitable XDoclet template for it, though.
--a.
----- Original Message -----
From: "Dave Johnson" <da...@rollerweblogger.org>
To: <ro...@incubator.apache.org>
Sent: Friday, July 08, 2005 3:40 PM
Subject: Re: velocity context cleanup
>
> +1 on dynamic proxies!
>
> - Dave
>
>
> On Jul 6, 2005, at 11:17 AM, Anil Gangolli wrote:
>
>>
>> Yes, I meant using the dynamic proxy facilities introduced in Java 1.3 (see, e.g.
>> http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
>>
>> I apologize for the terseness. Here's a more complete description.
>>
>> Note that the success of this approach willl hinge on the reflection of the proxy class seen by Velocity as being the restricted
>> interface desired to be exposed. Reading "Proxy Class Properties" in the above document, this should be the case.
>>
>> (1) Define a simple invocation handler class whose invoke() method just does an m.invoke(obj) on the passed in params,
>> and just unwraps InvocationTargetException to throw back any originating Throwable it contains.
>> See the examples in the doc above.
>>
>> (2) Define your restriction interfaces whose names can be derived by convention from the name of the associated POJO
>> (like org.roller.pojos.Template -> org.roller.presentation.velocity.wrappers.Template or TemplateWrapper). You must
>> define an interface to use a dynamic proxy.
>>
>> (3) Define a single proxy factory method wrap() that generates a proxy. [With the following example code it would be
>> VelocityWrapper.wrap(thePojo)]. You just call this and place the result into the Velocity context.
>>
>> Here is APPROXIMATE pseudo-code for illustration. Note: this won't compile for sure without a createWrapperFromPojoName() method
>> defined. You can get the intent anyway.
>>
>> public class VelocityWrapper {
>>
>> public static Object wrap(Object pojo) {
>> // Determine the wrapper interface class name from the pojo class name
>> String restrictionInterfaceName = createWrapperNameFromPojoName(pojo.getClass().getName());
>> // Get that class an return a proxy instance with the invocation handler below.
>> Class restrictionInterface = pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName);
>> SimpleInvocationHandler handler = new SimpleInvocationHandler(pojo);
>> return Proxy.newProxyInstance(pojo.getClass().getClassLoader,
>> new Class[] { restrictionInterface },
>> handler);
>> }
>>
>> public static class SimpleInvocationHandler implements InvocationHandler {
>> private Object theWrappedPojo;
>>
>> SimpleInvocationHandler(Object pojo) {
>> // When constructed, remember the instance we were constructed with
>> theWrappedPojo = pojo;
>> }
>>
>> public Object invoke(Object proxy, method m, Object[] args) throws Throwable {
>> try {
>> return m.invoke(theWrappedPojo, args);
>> } catch (InvocationTargetException e) {
>> // rethrow the original exception to make the wrapping transparent
>> throw e.getTargetException();
>> }
>> }
>> }
>> }
>>
>>
>>
>>
>>
>> ----- Original Message ----- From: "Rudman Max" <mr...@steelbrick.com>
>> To: <ro...@incubator.apache.org>
>> Sent: Tuesday, July 05, 2005 10:07 PM
>> Subject: Re: velocity context cleanup
>>
>>
>>> I think he is talking about dynamic proxy facilities provided by java.lang.reflect.Proxy. Instead of creating a wrapper class
>>> for each POJO you could deposit a dynamically proxy class in the Velocity context constructed with InvocationHandler
>>> implementation which blocks all method calls unless they begin with "set/is". I personally think this a really good idea.
>>>
>>> Max
>>>
>>> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
>>>
>>>> I'm not sure what you mean by dynamic proxy. Could you give more info.
>>>>
>>>> -- Allen
>>>>
>>>>
>>>> Anil Gangolli wrote:
>>>>
>>>>
>>>>>
>>>>> Just a quick note, and I admit I haven't followed the latest discussion, but if the wrappers are merely restrictions by a
>>>>> specified interface, it seems like a single dynamic proxy could implement all of them.
>>>>>
>>>>> --a.
>>>>>
>>>>> ----- Original Message ----- From: "Allen Gilliland" <Al...@Sun.COM>
>>>>> To: <ro...@incubator.apache.org>
>>>>> Sent: Tuesday, July 05, 2005 3:31 PM
>>>>> Subject: Re: velocity context cleanup
>>>>>
>>>>>
>>>>>
>>>>>> agreed. so the convention will be ...
>>>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>>>>>>
>>>>>> will act as a wrapper class for a <POJO Class> normally found in org.roller.pojos
>>>>>>
>>>>>> -- Allen
>>>>>>
>>>>>>
>>>>>> Lance Lavandowska wrote:
>>>>>>
>>>>>>
>>>>>>> Ooops, you caught me not paying sufficient attention, even whilst I
>>>>>>> was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
>>>>>>> better, less confusion with the "real" "pojos".
>>>>>>>
>>>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> i can do that, but org.roller.presentation.velocity.pojos *is* a new
>>>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers would be
>>>>>>>> more clear?
>>>>>>>>
>>>>>>>> -- Allen
>>>>>>>>
>>>>>>>>
>>>>>>>> Lance Lavandowska wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
>>>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
>>>>>>>>>
>>>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>
>
Re: velocity context cleanup
Posted by Dave Johnson <da...@rollerweblogger.org>.
+1 on dynamic proxies!
- Dave
On Jul 6, 2005, at 11:17 AM, Anil Gangolli wrote:
>
> Yes, I meant using the dynamic proxy facilities introduced in Java 1.3
> (see, e.g.
> http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
>
> I apologize for the terseness. Here's a more complete description.
>
> Note that the success of this approach willl hinge on the reflection
> of the proxy class seen by Velocity as being the restricted interface
> desired to be exposed. Reading "Proxy Class Properties" in the above
> document, this should be the case.
>
> (1) Define a simple invocation handler class whose invoke() method
> just does an m.invoke(obj) on the passed in params,
> and just unwraps InvocationTargetException to throw back any
> originating Throwable it contains.
> See the examples in the doc above.
>
> (2) Define your restriction interfaces whose names can be derived by
> convention from the name of the associated POJO
> (like org.roller.pojos.Template ->
> org.roller.presentation.velocity.wrappers.Template or
> TemplateWrapper). You must
> define an interface to use a dynamic proxy.
>
> (3) Define a single proxy factory method wrap() that generates a
> proxy. [With the following example code it would be
> VelocityWrapper.wrap(thePojo)]. You just call this and place the
> result into the Velocity context.
>
> Here is APPROXIMATE pseudo-code for illustration. Note: this won't
> compile for sure without a createWrapperFromPojoName() method defined.
> You can get the intent anyway.
>
> public class VelocityWrapper {
>
> public static Object wrap(Object pojo) {
> // Determine the wrapper interface class name from the
> pojo class name
> String restrictionInterfaceName =
> createWrapperNameFromPojoName(pojo.getClass().getName());
> // Get that class an return a proxy instance with the
> invocation handler below.
> Class restrictionInterface =
> pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName);
> SimpleInvocationHandler handler = new
> SimpleInvocationHandler(pojo);
> return
> Proxy.newProxyInstance(pojo.getClass().getClassLoader,
> new Class[]
> { restrictionInterface },
> handler);
> }
>
> public static class SimpleInvocationHandler implements
> InvocationHandler {
> private Object theWrappedPojo;
>
> SimpleInvocationHandler(Object pojo) {
> // When constructed, remember the instance we were
> constructed with
> theWrappedPojo = pojo;
> }
>
> public Object invoke(Object proxy, method m, Object[]
> args) throws Throwable {
> try {
> return m.invoke(theWrappedPojo, args);
> } catch (InvocationTargetException e) {
> // rethrow the original exception to make the
> wrapping transparent
> throw e.getTargetException();
> }
> }
> }
> }
>
>
>
>
>
> ----- Original Message ----- From: "Rudman Max"
> <mr...@steelbrick.com>
> To: <ro...@incubator.apache.org>
> Sent: Tuesday, July 05, 2005 10:07 PM
> Subject: Re: velocity context cleanup
>
>
>> I think he is talking about dynamic proxy facilities provided by
>> java.lang.reflect.Proxy. Instead of creating a wrapper class for each
>> POJO you could deposit a dynamically proxy class in the Velocity
>> context constructed with InvocationHandler implementation which
>> blocks all method calls unless they begin with "set/is". I personally
>> think this a really good idea.
>>
>> Max
>>
>> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
>>
>>> I'm not sure what you mean by dynamic proxy. Could you give more
>>> info.
>>>
>>> -- Allen
>>>
>>>
>>> Anil Gangolli wrote:
>>>
>>>
>>>>
>>>> Just a quick note, and I admit I haven't followed the latest
>>>> discussion, but if the wrappers are merely restrictions by a
>>>> specified interface, it seems like a single dynamic proxy could
>>>> implement all of them.
>>>>
>>>> --a.
>>>>
>>>> ----- Original Message ----- From: "Allen Gilliland"
>>>> <Al...@Sun.COM>
>>>> To: <ro...@incubator.apache.org>
>>>> Sent: Tuesday, July 05, 2005 3:31 PM
>>>> Subject: Re: velocity context cleanup
>>>>
>>>>
>>>>
>>>>> agreed. so the convention will be ...
>>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>>>>>
>>>>> will act as a wrapper class for a <POJO Class> normally found in
>>>>> org.roller.pojos
>>>>>
>>>>> -- Allen
>>>>>
>>>>>
>>>>> Lance Lavandowska wrote:
>>>>>
>>>>>
>>>>>> Ooops, you caught me not paying sufficient attention, even whilst
>>>>>> I
>>>>>> was typing out the package name! Hmm, I think I like
>>>>>> o.r.p.v.wrappers
>>>>>> better, less confusion with the "real" "pojos".
>>>>>>
>>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>>
>>>>>>
>>>>>>> i can do that, but org.roller.presentation.velocity.pojos *is*
>>>>>>> a new
>>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers
>>>>>>> would be
>>>>>>> more clear?
>>>>>>>
>>>>>>> -- Allen
>>>>>>>
>>>>>>>
>>>>>>> Lance Lavandowska wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
>>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>>
>
Re: velocity context cleanup
Posted by Anil Gangolli <an...@busybuddha.org>.
Yes, I meant using the dynamic proxy facilities introduced in Java 1.3 (see, e.g.
http://java.sun.com/j2se/1.3/docs/guide/reflection/proxy.html).
I apologize for the terseness. Here's a more complete description.
Note that the success of this approach willl hinge on the reflection of the proxy class seen by Velocity as being the restricted
interface desired to be exposed. Reading "Proxy Class Properties" in the above document, this should be the case.
(1) Define a simple invocation handler class whose invoke() method just does an m.invoke(obj) on the passed in params,
and just unwraps InvocationTargetException to throw back any originating Throwable it contains.
See the examples in the doc above.
(2) Define your restriction interfaces whose names can be derived by convention from the name of the associated POJO
(like org.roller.pojos.Template -> org.roller.presentation.velocity.wrappers.Template or TemplateWrapper). You must
define an interface to use a dynamic proxy.
(3) Define a single proxy factory method wrap() that generates a proxy. [With the following example code it would be
VelocityWrapper.wrap(thePojo)]. You just call this and place the result into the Velocity context.
Here is APPROXIMATE pseudo-code for illustration. Note: this won't compile for sure without a createWrapperFromPojoName() method
defined. You can get the intent anyway.
public class VelocityWrapper {
public static Object wrap(Object pojo) {
// Determine the wrapper interface class name from the pojo class name
String restrictionInterfaceName = createWrapperNameFromPojoName(pojo.getClass().getName());
// Get that class an return a proxy instance with the invocation handler below.
Class restrictionInterface = pojo.getClass().getClassLoader().loadClass(restrictionInterfaceName);
SimpleInvocationHandler handler = new SimpleInvocationHandler(pojo);
return Proxy.newProxyInstance(pojo.getClass().getClassLoader,
new Class[] { restrictionInterface },
handler);
}
public static class SimpleInvocationHandler implements InvocationHandler {
private Object theWrappedPojo;
SimpleInvocationHandler(Object pojo) {
// When constructed, remember the instance we were constructed with
theWrappedPojo = pojo;
}
public Object invoke(Object proxy, method m, Object[] args) throws Throwable {
try {
return m.invoke(theWrappedPojo, args);
} catch (InvocationTargetException e) {
// rethrow the original exception to make the wrapping transparent
throw e.getTargetException();
}
}
}
}
----- Original Message -----
From: "Rudman Max" <mr...@steelbrick.com>
To: <ro...@incubator.apache.org>
Sent: Tuesday, July 05, 2005 10:07 PM
Subject: Re: velocity context cleanup
>I think he is talking about dynamic proxy facilities provided by java.lang.reflect.Proxy. Instead of creating a wrapper class for
>each POJO you could deposit a dynamically proxy class in the Velocity context constructed with InvocationHandler implementation
>which blocks all method calls unless they begin with "set/is". I personally think this a really good idea.
>
> Max
>
> On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
>
>> I'm not sure what you mean by dynamic proxy. Could you give more info.
>>
>> -- Allen
>>
>>
>> Anil Gangolli wrote:
>>
>>
>>>
>>> Just a quick note, and I admit I haven't followed the latest discussion, but if the wrappers are merely restrictions by a
>>> specified interface, it seems like a single dynamic proxy could implement all of them.
>>>
>>> --a.
>>>
>>> ----- Original Message ----- From: "Allen Gilliland" <Al...@Sun.COM>
>>> To: <ro...@incubator.apache.org>
>>> Sent: Tuesday, July 05, 2005 3:31 PM
>>> Subject: Re: velocity context cleanup
>>>
>>>
>>>
>>>> agreed. so the convention will be ...
>>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>>>>
>>>> will act as a wrapper class for a <POJO Class> normally found in org.roller.pojos
>>>>
>>>> -- Allen
>>>>
>>>>
>>>> Lance Lavandowska wrote:
>>>>
>>>>
>>>>> Ooops, you caught me not paying sufficient attention, even whilst I
>>>>> was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
>>>>> better, less confusion with the "real" "pojos".
>>>>>
>>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>
>>>>>
>>>>>> i can do that, but org.roller.presentation.velocity.pojos *is* a new
>>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers would be
>>>>>> more clear?
>>>>>>
>>>>>> -- Allen
>>>>>>
>>>>>>
>>>>>> Lance Lavandowska wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
>>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
>>>>>>>
>>>>>>>
>>>>
>>>>
>>>
>>
Re: velocity context cleanup
Posted by Rudman Max <mr...@steelbrick.com>.
I think he is talking about dynamic proxy facilities provided by
java.lang.reflect.Proxy. Instead of creating a wrapper class for each
POJO you could deposit a dynamically proxy class in the Velocity
context constructed with InvocationHandler implementation which
blocks all method calls unless they begin with "set/is". I personally
think this a really good idea.
Max
On Jul 6, 2005, at 12:50 AM, Allen Gilliland wrote:
> I'm not sure what you mean by dynamic proxy. Could you give more
> info.
>
> -- Allen
>
>
> Anil Gangolli wrote:
>
>
>>
>> Just a quick note, and I admit I haven't followed the latest
>> discussion, but if the wrappers are merely restrictions by a
>> specified interface, it seems like a single dynamic proxy could
>> implement all of them.
>>
>> --a.
>>
>> ----- Original Message ----- From: "Allen Gilliland"
>> <Al...@Sun.COM>
>> To: <ro...@incubator.apache.org>
>> Sent: Tuesday, July 05, 2005 3:31 PM
>> Subject: Re: velocity context cleanup
>>
>>
>>
>>> agreed. so the convention will be ...
>>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>>>
>>> will act as a wrapper class for a <POJO Class> normally found in
>>> org.roller.pojos
>>>
>>> -- Allen
>>>
>>>
>>> Lance Lavandowska wrote:
>>>
>>>
>>>> Ooops, you caught me not paying sufficient attention, even whilst I
>>>> was typing out the package name! Hmm, I think I like
>>>> o.r.p.v.wrappers
>>>> better, less confusion with the "real" "pojos".
>>>>
>>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>
>>>>
>>>>> i can do that, but org.roller.presentation.velocity.pojos *is*
>>>>> a new
>>>>> sub-package. maybe org.roller.presentation.velocity.wrappers
>>>>> would be
>>>>> more clear?
>>>>>
>>>>> -- Allen
>>>>>
>>>>>
>>>>> Lance Lavandowska wrote:
>>>>>
>>>>>
>>>>>
>>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
>>>>>> org.roller.presentation.velocity.pojos.wrappers ?
>>>>>>
>>>>>>
>>>
>>>
>>
>
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
I'm not sure what you mean by dynamic proxy. Could you give more info.
-- Allen
Anil Gangolli wrote:
>
> Just a quick note, and I admit I haven't followed the latest
> discussion, but if the wrappers are merely restrictions by a specified
> interface, it seems like a single dynamic proxy could implement all of
> them.
>
> --a.
>
> ----- Original Message ----- From: "Allen Gilliland"
> <Al...@Sun.COM>
> To: <ro...@incubator.apache.org>
> Sent: Tuesday, July 05, 2005 3:31 PM
> Subject: Re: velocity context cleanup
>
>
>> agreed. so the convention will be ...
>> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>>
>> will act as a wrapper class for a <POJO Class> normally found in
>> org.roller.pojos
>>
>> -- Allen
>>
>>
>> Lance Lavandowska wrote:
>>
>>> Ooops, you caught me not paying sufficient attention, even whilst I
>>> was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
>>> better, less confusion with the "real" "pojos".
>>>
>>> On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>>>
>>>> i can do that, but org.roller.presentation.velocity.pojos *is* a new
>>>> sub-package. maybe org.roller.presentation.velocity.wrappers would be
>>>> more clear?
>>>>
>>>> -- Allen
>>>>
>>>>
>>>> Lance Lavandowska wrote:
>>>>
>>>>
>>>>> Just one suggestion, put the wrappers in a sub-package, perhaps
>>>>> org.roller.presentation.velocity.pojos.wrappers ?
>>>>>
>>
>
Re: velocity context cleanup
Posted by Anil Gangolli <an...@busybuddha.org>.
Just a quick note, and I admit I haven't followed the latest discussion, but if the wrappers are merely restrictions by a specified
interface, it seems like a single dynamic proxy could implement all of them.
--a.
----- Original Message -----
From: "Allen Gilliland" <Al...@Sun.COM>
To: <ro...@incubator.apache.org>
Sent: Tuesday, July 05, 2005 3:31 PM
Subject: Re: velocity context cleanup
> agreed. so the convention will be ...
> org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
>
> will act as a wrapper class for a <POJO Class> normally found in org.roller.pojos
>
> -- Allen
>
>
> Lance Lavandowska wrote:
>
>>Ooops, you caught me not paying sufficient attention, even whilst I
>>was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
>>better, less confusion with the "real" "pojos".
>>
>>On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>>
>>>i can do that, but org.roller.presentation.velocity.pojos *is* a new
>>>sub-package. maybe org.roller.presentation.velocity.wrappers would be
>>>more clear?
>>>
>>>-- Allen
>>>
>>>
>>>Lance Lavandowska wrote:
>>>
>>>
>>>>Just one suggestion, put the wrappers in a sub-package, perhaps
>>>>org.roller.presentation.velocity.pojos.wrappers ?
>>>>
>
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
agreed. so the convention will be ...
org.roller.presentation.velocity.wrappers.<POJO Class>Wrapper
will act as a wrapper class for a <POJO Class> normally found in
org.roller.pojos
-- Allen
Lance Lavandowska wrote:
>Ooops, you caught me not paying sufficient attention, even whilst I
>was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
>better, less confusion with the "real" "pojos".
>
>On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
>
>
>>i can do that, but org.roller.presentation.velocity.pojos *is* a new
>>sub-package. maybe org.roller.presentation.velocity.wrappers would be
>>more clear?
>>
>>-- Allen
>>
>>
>>Lance Lavandowska wrote:
>>
>>
>>
>>>Just one suggestion, put the wrappers in a sub-package, perhaps
>>>org.roller.presentation.velocity.pojos.wrappers ?
>>>
>>>
Re: velocity context cleanup
Posted by Lance Lavandowska <la...@gmail.com>.
Ooops, you caught me not paying sufficient attention, even whilst I
was typing out the package name! Hmm, I think I like o.r.p.v.wrappers
better, less confusion with the "real" "pojos".
On 7/5/05, Allen Gilliland <Al...@sun.com> wrote:
> i can do that, but org.roller.presentation.velocity.pojos *is* a new
> sub-package. maybe org.roller.presentation.velocity.wrappers would be
> more clear?
>
> -- Allen
>
>
> Lance Lavandowska wrote:
>
> >Just one suggestion, put the wrappers in a sub-package, perhaps
> >org.roller.presentation.velocity.pojos.wrappers ?
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
i can do that, but org.roller.presentation.velocity.pojos *is* a new
sub-package. maybe org.roller.presentation.velocity.wrappers would be
more clear?
-- Allen
Lance Lavandowska wrote:
>Just one suggestion, put the wrappers in a sub-package, perhaps
>org.roller.presentation.velocity.pojos.wrappers ?
>
>On 7/4/05, Allen Gilliland <Al...@sun.com> wrote:
>
>
>>ok, I realize why this happens now. I wasn't thinking clearly on my
>>last message.
>>
>>anyways, I verified on the velocity-user list that there isn't really a
>>way to deal with this other than creating wrapper classes, so it sounds
>>like that's what we'll have to do. I've tested this with some of the
>>changes I made for the theme management stuff and it's really not that
>>tough to do ... just requires extra code which is just a little annoying.
>>
>>how about a convention like this which basically just contains getXXX()
>>methods ...
>> org.roller.presentation.velocity.pojos.<POJO>Wrapper
>>
>>then when you put something in the velocity context you do ...
>> context.put("name", new <POJO>Wrapper(<POJO>));
>>
>>I've created a TemplateWrapper class to try this out and it's working
>>great. If everyone is comfortable with this approach then I'd like to
>>keep working to get these wrappers implemented in the 1.3 release
>>because I think this is pretty important and relatively easy to do.
>>
>>-- Allen
>>
>>
>>Allen Gilliland wrote:
>>
>>
>>
>>>hmmm ... well that's a bummer. i'd like to do some testing to double
>>>check that though because that seems like a strange approach on their
>>>part. part of the purpose of polymorphism is to hide methods in
>>>certain cases and if velocity is bypassing that using reflection then
>>>we would never be able to overcome that :(
>>>
>>>-- Allen
>>>
>>>
>>>Dave Johnson wrote:
>>>
>>>
>>>
>>>>I think that's a good idea, but (as far as I know) it won't keep
>>>>weblog template authors from accessing fields on WebsiteData.
>>>>Velocity uses reflection, so they won't be restricted to just the
>>>>Website methods/fields.
>>>>
>>>>What's the best way to restrict template authors to just the methods
>>>>in the interface? Some sort of proxy class perhaps?
>>>>
>>>>- Dave
>>>>
>>>>
>>>>
>>>>On Jul 1, 2005, at 12:50 PM, Allen Gilliland wrote:
>>>>
>>>>
>>>>
>>>>>okay, so it sounds like everyone agrees that this is a good thing to
>>>>>do at some point. i'll go ahead and work on a more formal proposal
>>>>>so we can see exactly what exact changes we would make.
>>>>>
>>>>>in addition to removing some really dangerous methods like
>>>>>website.remove() i think we'll also need a plan to prevent users
>>>>>from having access to <object>.setXXX(). i've run the same tests as
>>>>>i did with website.remove() and basically any user can set any
>>>>>property of any of the objects in the velocity context to whatever
>>>>>they want and these changes will go directly back to the db. while
>>>>>this would be a pretty stupid and non-productive thing for a user to
>>>>>do it is still a potential risk that i think we should try and avoid.
>>>>>
>>>>>what i am now thinking may be the best idea is to setup either
>>>>>interfaces or base/abstract versions of each pojo that will go into
>>>>>the velocity context, and then when we are doing the loading of the
>>>>>velocity context we can simply cast the real pojos into their more
>>>>>limited versions. here's an example of what i mean ...
>>>>>
>>>>>- create interface Website which only contains getXXX() methods
>>>>>- the velocity context gets, ctx.put("website", (Website)
>>>>>WebsiteDataObject)
>>>>>- XXXManager classes still pass around XXXData objects as usual
>>>>>
>>>>>this would mean we could control what methods are available to users
>>>>>via Velocity without actually changing any of our existing pojos.
>>>>>the only work would be to create the new interfaces, have each pojo
>>>>>implement it's respective interface, then modify the context loading
>>>>>methods to cast the pojos into the interface.
>>>>>
>>>>>the other cool thing is that this is not intrusive on any existing
>>>>>functionality, so i believe i could start adding this in for the 1.3
>>>>>release without causing any problems for Dave in his 2.0 branch.
>>>>>
>>>>>what do people think?
>>>>>
>>>>>-- Allen
>>>>>
>>>>>
>>>>>On Fri, 2005-07-01 at 08:18, Anil Gangolli wrote:
>>>>>
>>>>>
>>>>>
>>>>>>I agree with this general direction of separating the persistence
>>>>>>behavior from the data holder beans; let pojos be pojos.
>>>>>>
>>>>>>Putting the logic in managers does necessitate the need always to
>>>>>>know about the association between the class and its manager. In
>>>>>>methods that deal with multiple different classes of objects we do
>>>>>>persist, that can become an issue.
>>>>>>
>>>>>>However, if we decide we want to be able to deal with instances
>>>>>>that know how to save themselves, or determining if the current user
>>>>>>can save them, etc, that behavior should really be in classes that
>>>>>>derive from or wrap/delegate to the pojo data holders, not the
>>>>>>other way around as it is now.
>>>>>>
>>>>>>So Allen, +1 to this idea
>>>>>>
>>>>>>--a.
>>>>>>
>>>>>>
>>>>>>----- Original Message -----
>>>>>>From: "David M Johnson" <Da...@Sun.COM>
>>>>>>To: <ro...@incubator.apache.org>
>>>>>>Sent: Friday, July 01, 2005 7:09 AM
>>>>>>Subject: Re: velocity context cleanup
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>+1!!!
>>>>>>>
>>>>>>>I'm guilty of moving methods like save(), remove(), etc. into the
>>>>>>>POJOs. I think everything you've outlined below is basically a
>>>>>>>good idea, but I'd like to hold off on it until we merge 2.0 back in.
>>>>>>>
>>>>>>>- Dave
>>>>>>>
>>>>>>>
>>>>>>>On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>Okay, so it sounds like a few other people have given this a
>>>>>>>>little thought and think that it may be beneficial to make some
>>>>>>>>changes to the way the Pojos and PersistentObjects work. I think
>>>>>>>>it would help to add a little more detail to the discussion so
>>>>>>>>we know what we are talking about. Here's my stab at what
>>>>>>>>changes I would think about making ...
>>>>>>>>
>>>>>>>>- move PersistentObject.save() into Manager classes only
>>>>>>>>- move PersistentObject.remove() into Manager classes only
>>>>>>>>
>>>>>>>>I think those 2 changes would go a long ways toward making it
>>>>>>>>less dangerous to make Pojos directly available to users via the
>>>>>>>>velocity context. I am in partial agreement that we may not need
>>>>>>>>the PersistentObject class at all. Right now I would also
>>>>>>>>consider doing ...
>>>>>>>>
>>>>>>>>- remove PersistentObject.get/setId() (these are not necessarily
>>>>>>>>part of all objects)
>>>>>>>>- remove PersistentObject.setData() (this can easily be done
>>>>>>>>elsewhere)
>>>>>>>>- remove PersistentObject.canSave() (i don't fully understand how
>>>>>>>>this is used, but i believe this logic can be in the Manager
>>>>>>>>classes save/remove methods)
>>>>>>>>
>>>>>>>>If we also want to do those last few items then the
>>>>>>>>PersistentObject class would basically be useless. I think the
>>>>>>>>first 2 are
>>>>>>>>pretty important, but the last 3 are optional. Personally I
>>>>>>>>would probably go ahead and ditch the PersistentObject class just
>>>>>>>>because I don't think we really need it.
>>>>>>>>
>>>>>>>>what do others think?
>>>>>>>>
>>>>>>>>Remember, we are just talking about this right now so please
>>>>>>>>speak up and voice your opinion. We aren't going to make any
>>>>>>>>changes right away, especially with the fact that Dave has a lot
>>>>>>>>of data model work going on for the 2.0 release and we don't
>>>>>>>>want to mess with what he has done so far. Once we get a bit
>>>>>>>>more consenus then I will formalize a Proposal that can be reviewed
>>>>>>>>again.
>>>>>>>>
>>>>>>>>-- Allen
>>>>>>>>
>>>>>>>>
>>>>>>>>On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>I just wanted to chime in that I really dislike persistence methods
>>>>>>>>>being in POJOs also and would be willing to pitch in with moving
>>>>>>>>>those out to the appropriate manager classes. In fact, I would even
>>>>>>>>>like to see PersistenceObject go as having to extend data objects
>>>>>>>>>from it pretty much negates one of the key benefits of Hibernate --
>>>>>>>>>its non-intrusiveness into the object model.
>>>>>>>>>
>>>>>>>>>Max
>>>>>>>>>
>>>>>>>>>On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>The remove() method is used in several cases to do some of the
>>>>>>>>>>cascading needed to maintain consistency properties. Just make
>>>>>>>>>>sure to preserve that logic; if you take out the remove() methods,
>>>>>>>>>>this logic needs to be moved into the corresponding manager
>>>>>>>>>>methods.
>>>>>>>>>>
>>>>>>>>>>--a.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>----- Original Message ----- From: "Lance Lavandowska"
>>>>>>>>>><la...@gmail.com>
>>>>>>>>>>To: <ro...@incubator.apache.org>
>>>>>>>>>>Sent: Thursday, June 30, 2005 8:24 AM
>>>>>>>>>>Subject: Re: velocity context cleanup
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>*Data.remove() is available to users (try $website.remove() in a
>>>>>>>>>>>>template)
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>This method should probably be removed from the classes. While I
>>>>>>>>>>>think even POJOs should contain some business logic, I don't feel
>>>>>>>>>>>that
>>>>>>>>>>>persistence-related methods are appropriate. Because this is
>>>>>>>>>>>only my
>>>>>>>>>>>personal gut-check, I've never objected.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>PageHelper.evaluateString() is available to users (this one
>>>>>>>>>>>>actually bit us in the ass already and a user caught themself in
>>>>>>>>>>>>a recursive loop which killed the server)
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>I'm the one guilty of creating that monstrosity, and I say "get
>>>>>>>>>>>rid of
>>>>>>>>>>>it". I doubt it is in my real use - but you may break a few
>>>>>>>>>>>pages by
>>>>>>>>>>>removing it. Perhaps change it to print "THIS MACRO HAS BEEN
>>>>>>>>>>>REMOVED"? Note: this is a misguided macro, not a Context value.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>Some of these may be a simple case of updating the public,
>>>>>>>>>>>>protected, private access levels on methods, but some cases may
>>>>>>>>>>>>mean removing objects from the Context and/or removing methods
>>>>>>>>>>>>from objects that are part of the Context.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>All of the objects placed into the Context are done so to
>>>>>>>>>>>achieve an
>>>>>>>>>>>objective in the *.vm templates or the Page templates. As I
>>>>>>>>>>>implied
>>>>>>>>>>>above, let's look at what is being exposed by these objects
>>>>>>>>>>>that may
>>>>>>>>>>>be 'dangerous' instead.
>>>>>>>>>>>
>>>>>>>>>>>Lance
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
Re: velocity context cleanup
Posted by Lance Lavandowska <la...@gmail.com>.
Just one suggestion, put the wrappers in a sub-package, perhaps
org.roller.presentation.velocity.pojos.wrappers ?
On 7/4/05, Allen Gilliland <Al...@sun.com> wrote:
> ok, I realize why this happens now. I wasn't thinking clearly on my
> last message.
>
> anyways, I verified on the velocity-user list that there isn't really a
> way to deal with this other than creating wrapper classes, so it sounds
> like that's what we'll have to do. I've tested this with some of the
> changes I made for the theme management stuff and it's really not that
> tough to do ... just requires extra code which is just a little annoying.
>
> how about a convention like this which basically just contains getXXX()
> methods ...
> org.roller.presentation.velocity.pojos.<POJO>Wrapper
>
> then when you put something in the velocity context you do ...
> context.put("name", new <POJO>Wrapper(<POJO>));
>
> I've created a TemplateWrapper class to try this out and it's working
> great. If everyone is comfortable with this approach then I'd like to
> keep working to get these wrappers implemented in the 1.3 release
> because I think this is pretty important and relatively easy to do.
>
> -- Allen
>
>
> Allen Gilliland wrote:
>
> > hmmm ... well that's a bummer. i'd like to do some testing to double
> > check that though because that seems like a strange approach on their
> > part. part of the purpose of polymorphism is to hide methods in
> > certain cases and if velocity is bypassing that using reflection then
> > we would never be able to overcome that :(
> >
> > -- Allen
> >
> >
> > Dave Johnson wrote:
> >
> >> I think that's a good idea, but (as far as I know) it won't keep
> >> weblog template authors from accessing fields on WebsiteData.
> >> Velocity uses reflection, so they won't be restricted to just the
> >> Website methods/fields.
> >>
> >> What's the best way to restrict template authors to just the methods
> >> in the interface? Some sort of proxy class perhaps?
> >>
> >> - Dave
> >>
> >>
> >>
> >> On Jul 1, 2005, at 12:50 PM, Allen Gilliland wrote:
> >>
> >>> okay, so it sounds like everyone agrees that this is a good thing to
> >>> do at some point. i'll go ahead and work on a more formal proposal
> >>> so we can see exactly what exact changes we would make.
> >>>
> >>> in addition to removing some really dangerous methods like
> >>> website.remove() i think we'll also need a plan to prevent users
> >>> from having access to <object>.setXXX(). i've run the same tests as
> >>> i did with website.remove() and basically any user can set any
> >>> property of any of the objects in the velocity context to whatever
> >>> they want and these changes will go directly back to the db. while
> >>> this would be a pretty stupid and non-productive thing for a user to
> >>> do it is still a potential risk that i think we should try and avoid.
> >>>
> >>> what i am now thinking may be the best idea is to setup either
> >>> interfaces or base/abstract versions of each pojo that will go into
> >>> the velocity context, and then when we are doing the loading of the
> >>> velocity context we can simply cast the real pojos into their more
> >>> limited versions. here's an example of what i mean ...
> >>>
> >>> - create interface Website which only contains getXXX() methods
> >>> - the velocity context gets, ctx.put("website", (Website)
> >>> WebsiteDataObject)
> >>> - XXXManager classes still pass around XXXData objects as usual
> >>>
> >>> this would mean we could control what methods are available to users
> >>> via Velocity without actually changing any of our existing pojos.
> >>> the only work would be to create the new interfaces, have each pojo
> >>> implement it's respective interface, then modify the context loading
> >>> methods to cast the pojos into the interface.
> >>>
> >>> the other cool thing is that this is not intrusive on any existing
> >>> functionality, so i believe i could start adding this in for the 1.3
> >>> release without causing any problems for Dave in his 2.0 branch.
> >>>
> >>> what do people think?
> >>>
> >>> -- Allen
> >>>
> >>>
> >>> On Fri, 2005-07-01 at 08:18, Anil Gangolli wrote:
> >>>
> >>>> I agree with this general direction of separating the persistence
> >>>> behavior from the data holder beans; let pojos be pojos.
> >>>>
> >>>> Putting the logic in managers does necessitate the need always to
> >>>> know about the association between the class and its manager. In
> >>>> methods that deal with multiple different classes of objects we do
> >>>> persist, that can become an issue.
> >>>>
> >>>> However, if we decide we want to be able to deal with instances
> >>>> that know how to save themselves, or determining if the current user
> >>>> can save them, etc, that behavior should really be in classes that
> >>>> derive from or wrap/delegate to the pojo data holders, not the
> >>>> other way around as it is now.
> >>>>
> >>>> So Allen, +1 to this idea
> >>>>
> >>>> --a.
> >>>>
> >>>>
> >>>> ----- Original Message -----
> >>>> From: "David M Johnson" <Da...@Sun.COM>
> >>>> To: <ro...@incubator.apache.org>
> >>>> Sent: Friday, July 01, 2005 7:09 AM
> >>>> Subject: Re: velocity context cleanup
> >>>>
> >>>>
> >>>>> +1!!!
> >>>>>
> >>>>> I'm guilty of moving methods like save(), remove(), etc. into the
> >>>>> POJOs. I think everything you've outlined below is basically a
> >>>>> good idea, but I'd like to hold off on it until we merge 2.0 back in.
> >>>>>
> >>>>> - Dave
> >>>>>
> >>>>>
> >>>>> On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
> >>>>>
> >>>>>> Okay, so it sounds like a few other people have given this a
> >>>>>> little thought and think that it may be beneficial to make some
> >>>>>> changes to the way the Pojos and PersistentObjects work. I think
> >>>>>> it would help to add a little more detail to the discussion so
> >>>>>> we know what we are talking about. Here's my stab at what
> >>>>>> changes I would think about making ...
> >>>>>>
> >>>>>> - move PersistentObject.save() into Manager classes only
> >>>>>> - move PersistentObject.remove() into Manager classes only
> >>>>>>
> >>>>>> I think those 2 changes would go a long ways toward making it
> >>>>>> less dangerous to make Pojos directly available to users via the
> >>>>>> velocity context. I am in partial agreement that we may not need
> >>>>>> the PersistentObject class at all. Right now I would also
> >>>>>> consider doing ...
> >>>>>>
> >>>>>> - remove PersistentObject.get/setId() (these are not necessarily
> >>>>>> part of all objects)
> >>>>>> - remove PersistentObject.setData() (this can easily be done
> >>>>>> elsewhere)
> >>>>>> - remove PersistentObject.canSave() (i don't fully understand how
> >>>>>> this is used, but i believe this logic can be in the Manager
> >>>>>> classes save/remove methods)
> >>>>>>
> >>>>>> If we also want to do those last few items then the
> >>>>>> PersistentObject class would basically be useless. I think the
> >>>>>> first 2 are
> >>>>>> pretty important, but the last 3 are optional. Personally I
> >>>>>> would probably go ahead and ditch the PersistentObject class just
> >>>>>> because I don't think we really need it.
> >>>>>>
> >>>>>> what do others think?
> >>>>>>
> >>>>>> Remember, we are just talking about this right now so please
> >>>>>> speak up and voice your opinion. We aren't going to make any
> >>>>>> changes right away, especially with the fact that Dave has a lot
> >>>>>> of data model work going on for the 2.0 release and we don't
> >>>>>> want to mess with what he has done so far. Once we get a bit
> >>>>>> more consenus then I will formalize a Proposal that can be reviewed
> >>>>>> again.
> >>>>>>
> >>>>>> -- Allen
> >>>>>>
> >>>>>>
> >>>>>> On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
> >>>>>>
> >>>>>>> I just wanted to chime in that I really dislike persistence methods
> >>>>>>> being in POJOs also and would be willing to pitch in with moving
> >>>>>>> those out to the appropriate manager classes. In fact, I would even
> >>>>>>> like to see PersistenceObject go as having to extend data objects
> >>>>>>> from it pretty much negates one of the key benefits of Hibernate --
> >>>>>>> its non-intrusiveness into the object model.
> >>>>>>>
> >>>>>>> Max
> >>>>>>>
> >>>>>>> On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
> >>>>>>>
> >>>>>>>>
> >>>>>>>> The remove() method is used in several cases to do some of the
> >>>>>>>> cascading needed to maintain consistency properties. Just make
> >>>>>>>> sure to preserve that logic; if you take out the remove() methods,
> >>>>>>>> this logic needs to be moved into the corresponding manager
> >>>>>>>> methods.
> >>>>>>>>
> >>>>>>>> --a.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> ----- Original Message ----- From: "Lance Lavandowska"
> >>>>>>>> <la...@gmail.com>
> >>>>>>>> To: <ro...@incubator.apache.org>
> >>>>>>>> Sent: Thursday, June 30, 2005 8:24 AM
> >>>>>>>> Subject: Re: velocity context cleanup
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> *Data.remove() is available to users (try $website.remove() in a
> >>>>>>>>>> template)
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> This method should probably be removed from the classes. While I
> >>>>>>>>> think even POJOs should contain some business logic, I don't feel
> >>>>>>>>> that
> >>>>>>>>> persistence-related methods are appropriate. Because this is
> >>>>>>>>> only my
> >>>>>>>>> personal gut-check, I've never objected.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> PageHelper.evaluateString() is available to users (this one
> >>>>>>>>>> actually bit us in the ass already and a user caught themself in
> >>>>>>>>>> a recursive loop which killed the server)
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> I'm the one guilty of creating that monstrosity, and I say "get
> >>>>>>>>> rid of
> >>>>>>>>> it". I doubt it is in my real use - but you may break a few
> >>>>>>>>> pages by
> >>>>>>>>> removing it. Perhaps change it to print "THIS MACRO HAS BEEN
> >>>>>>>>> REMOVED"? Note: this is a misguided macro, not a Context value.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> Some of these may be a simple case of updating the public,
> >>>>>>>>>> protected, private access levels on methods, but some cases may
> >>>>>>>>>> mean removing objects from the Context and/or removing methods
> >>>>>>>>>> from objects that are part of the Context.
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> All of the objects placed into the Context are done so to
> >>>>>>>>> achieve an
> >>>>>>>>> objective in the *.vm templates or the Page templates. As I
> >>>>>>>>> implied
> >>>>>>>>> above, let's look at what is being exposed by these objects
> >>>>>>>>> that may
> >>>>>>>>> be 'dangerous' instead.
> >>>>>>>>>
> >>>>>>>>> Lance
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>
> >>>>
> >>
>
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
ok, I realize why this happens now. I wasn't thinking clearly on my
last message.
anyways, I verified on the velocity-user list that there isn't really a
way to deal with this other than creating wrapper classes, so it sounds
like that's what we'll have to do. I've tested this with some of the
changes I made for the theme management stuff and it's really not that
tough to do ... just requires extra code which is just a little annoying.
how about a convention like this which basically just contains getXXX()
methods ...
org.roller.presentation.velocity.pojos.<POJO>Wrapper
then when you put something in the velocity context you do ...
context.put("name", new <POJO>Wrapper(<POJO>));
I've created a TemplateWrapper class to try this out and it's working
great. If everyone is comfortable with this approach then I'd like to
keep working to get these wrappers implemented in the 1.3 release
because I think this is pretty important and relatively easy to do.
-- Allen
Allen Gilliland wrote:
> hmmm ... well that's a bummer. i'd like to do some testing to double
> check that though because that seems like a strange approach on their
> part. part of the purpose of polymorphism is to hide methods in
> certain cases and if velocity is bypassing that using reflection then
> we would never be able to overcome that :(
>
> -- Allen
>
>
> Dave Johnson wrote:
>
>> I think that's a good idea, but (as far as I know) it won't keep
>> weblog template authors from accessing fields on WebsiteData.
>> Velocity uses reflection, so they won't be restricted to just the
>> Website methods/fields.
>>
>> What's the best way to restrict template authors to just the methods
>> in the interface? Some sort of proxy class perhaps?
>>
>> - Dave
>>
>>
>>
>> On Jul 1, 2005, at 12:50 PM, Allen Gilliland wrote:
>>
>>> okay, so it sounds like everyone agrees that this is a good thing to
>>> do at some point. i'll go ahead and work on a more formal proposal
>>> so we can see exactly what exact changes we would make.
>>>
>>> in addition to removing some really dangerous methods like
>>> website.remove() i think we'll also need a plan to prevent users
>>> from having access to <object>.setXXX(). i've run the same tests as
>>> i did with website.remove() and basically any user can set any
>>> property of any of the objects in the velocity context to whatever
>>> they want and these changes will go directly back to the db. while
>>> this would be a pretty stupid and non-productive thing for a user to
>>> do it is still a potential risk that i think we should try and avoid.
>>>
>>> what i am now thinking may be the best idea is to setup either
>>> interfaces or base/abstract versions of each pojo that will go into
>>> the velocity context, and then when we are doing the loading of the
>>> velocity context we can simply cast the real pojos into their more
>>> limited versions. here's an example of what i mean ...
>>>
>>> - create interface Website which only contains getXXX() methods
>>> - the velocity context gets, ctx.put("website", (Website)
>>> WebsiteDataObject)
>>> - XXXManager classes still pass around XXXData objects as usual
>>>
>>> this would mean we could control what methods are available to users
>>> via Velocity without actually changing any of our existing pojos.
>>> the only work would be to create the new interfaces, have each pojo
>>> implement it's respective interface, then modify the context loading
>>> methods to cast the pojos into the interface.
>>>
>>> the other cool thing is that this is not intrusive on any existing
>>> functionality, so i believe i could start adding this in for the 1.3
>>> release without causing any problems for Dave in his 2.0 branch.
>>>
>>> what do people think?
>>>
>>> -- Allen
>>>
>>>
>>> On Fri, 2005-07-01 at 08:18, Anil Gangolli wrote:
>>>
>>>> I agree with this general direction of separating the persistence
>>>> behavior from the data holder beans; let pojos be pojos.
>>>>
>>>> Putting the logic in managers does necessitate the need always to
>>>> know about the association between the class and its manager. In
>>>> methods that deal with multiple different classes of objects we do
>>>> persist, that can become an issue.
>>>>
>>>> However, if we decide we want to be able to deal with instances
>>>> that know how to save themselves, or determining if the current user
>>>> can save them, etc, that behavior should really be in classes that
>>>> derive from or wrap/delegate to the pojo data holders, not the
>>>> other way around as it is now.
>>>>
>>>> So Allen, +1 to this idea
>>>>
>>>> --a.
>>>>
>>>>
>>>> ----- Original Message -----
>>>> From: "David M Johnson" <Da...@Sun.COM>
>>>> To: <ro...@incubator.apache.org>
>>>> Sent: Friday, July 01, 2005 7:09 AM
>>>> Subject: Re: velocity context cleanup
>>>>
>>>>
>>>>> +1!!!
>>>>>
>>>>> I'm guilty of moving methods like save(), remove(), etc. into the
>>>>> POJOs. I think everything you've outlined below is basically a
>>>>> good idea, but I'd like to hold off on it until we merge 2.0 back in.
>>>>>
>>>>> - Dave
>>>>>
>>>>>
>>>>> On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
>>>>>
>>>>>> Okay, so it sounds like a few other people have given this a
>>>>>> little thought and think that it may be beneficial to make some
>>>>>> changes to the way the Pojos and PersistentObjects work. I think
>>>>>> it would help to add a little more detail to the discussion so
>>>>>> we know what we are talking about. Here's my stab at what
>>>>>> changes I would think about making ...
>>>>>>
>>>>>> - move PersistentObject.save() into Manager classes only
>>>>>> - move PersistentObject.remove() into Manager classes only
>>>>>>
>>>>>> I think those 2 changes would go a long ways toward making it
>>>>>> less dangerous to make Pojos directly available to users via the
>>>>>> velocity context. I am in partial agreement that we may not need
>>>>>> the PersistentObject class at all. Right now I would also
>>>>>> consider doing ...
>>>>>>
>>>>>> - remove PersistentObject.get/setId() (these are not necessarily
>>>>>> part of all objects)
>>>>>> - remove PersistentObject.setData() (this can easily be done
>>>>>> elsewhere)
>>>>>> - remove PersistentObject.canSave() (i don't fully understand how
>>>>>> this is used, but i believe this logic can be in the Manager
>>>>>> classes save/remove methods)
>>>>>>
>>>>>> If we also want to do those last few items then the
>>>>>> PersistentObject class would basically be useless. I think the
>>>>>> first 2 are
>>>>>> pretty important, but the last 3 are optional. Personally I
>>>>>> would probably go ahead and ditch the PersistentObject class just
>>>>>> because I don't think we really need it.
>>>>>>
>>>>>> what do others think?
>>>>>>
>>>>>> Remember, we are just talking about this right now so please
>>>>>> speak up and voice your opinion. We aren't going to make any
>>>>>> changes right away, especially with the fact that Dave has a lot
>>>>>> of data model work going on for the 2.0 release and we don't
>>>>>> want to mess with what he has done so far. Once we get a bit
>>>>>> more consenus then I will formalize a Proposal that can be reviewed
>>>>>> again.
>>>>>>
>>>>>> -- Allen
>>>>>>
>>>>>>
>>>>>> On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
>>>>>>
>>>>>>> I just wanted to chime in that I really dislike persistence methods
>>>>>>> being in POJOs also and would be willing to pitch in with moving
>>>>>>> those out to the appropriate manager classes. In fact, I would even
>>>>>>> like to see PersistenceObject go as having to extend data objects
>>>>>>> from it pretty much negates one of the key benefits of Hibernate --
>>>>>>> its non-intrusiveness into the object model.
>>>>>>>
>>>>>>> Max
>>>>>>>
>>>>>>> On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> The remove() method is used in several cases to do some of the
>>>>>>>> cascading needed to maintain consistency properties. Just make
>>>>>>>> sure to preserve that logic; if you take out the remove() methods,
>>>>>>>> this logic needs to be moved into the corresponding manager
>>>>>>>> methods.
>>>>>>>>
>>>>>>>> --a.
>>>>>>>>
>>>>>>>>
>>>>>>>> ----- Original Message ----- From: "Lance Lavandowska"
>>>>>>>> <la...@gmail.com>
>>>>>>>> To: <ro...@incubator.apache.org>
>>>>>>>> Sent: Thursday, June 30, 2005 8:24 AM
>>>>>>>> Subject: Re: velocity context cleanup
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>> On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> *Data.remove() is available to users (try $website.remove() in a
>>>>>>>>>> template)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This method should probably be removed from the classes. While I
>>>>>>>>> think even POJOs should contain some business logic, I don't feel
>>>>>>>>> that
>>>>>>>>> persistence-related methods are appropriate. Because this is
>>>>>>>>> only my
>>>>>>>>> personal gut-check, I've never objected.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> PageHelper.evaluateString() is available to users (this one
>>>>>>>>>> actually bit us in the ass already and a user caught themself in
>>>>>>>>>> a recursive loop which killed the server)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'm the one guilty of creating that monstrosity, and I say "get
>>>>>>>>> rid of
>>>>>>>>> it". I doubt it is in my real use - but you may break a few
>>>>>>>>> pages by
>>>>>>>>> removing it. Perhaps change it to print "THIS MACRO HAS BEEN
>>>>>>>>> REMOVED"? Note: this is a misguided macro, not a Context value.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Some of these may be a simple case of updating the public,
>>>>>>>>>> protected, private access levels on methods, but some cases may
>>>>>>>>>> mean removing objects from the Context and/or removing methods
>>>>>>>>>> from objects that are part of the Context.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> All of the objects placed into the Context are done so to
>>>>>>>>> achieve an
>>>>>>>>> objective in the *.vm templates or the Page templates. As I
>>>>>>>>> implied
>>>>>>>>> above, let's look at what is being exposed by these objects
>>>>>>>>> that may
>>>>>>>>> be 'dangerous' instead.
>>>>>>>>>
>>>>>>>>> Lance
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
hmmm ... well that's a bummer. i'd like to do some testing to double
check that though because that seems like a strange approach on their
part. part of the purpose of polymorphism is to hide methods in certain
cases and if velocity is bypassing that using reflection then we would
never be able to overcome that :(
-- Allen
Dave Johnson wrote:
> I think that's a good idea, but (as far as I know) it won't keep
> weblog template authors from accessing fields on WebsiteData. Velocity
> uses reflection, so they won't be restricted to just the Website
> methods/fields.
>
> What's the best way to restrict template authors to just the methods
> in the interface? Some sort of proxy class perhaps?
>
> - Dave
>
>
>
> On Jul 1, 2005, at 12:50 PM, Allen Gilliland wrote:
>
>> okay, so it sounds like everyone agrees that this is a good thing to
>> do at some point. i'll go ahead and work on a more formal proposal
>> so we can see exactly what exact changes we would make.
>>
>> in addition to removing some really dangerous methods like
>> website.remove() i think we'll also need a plan to prevent users from
>> having access to <object>.setXXX(). i've run the same tests as i did
>> with website.remove() and basically any user can set any property of
>> any of the objects in the velocity context to whatever they want and
>> these changes will go directly back to the db. while this would be a
>> pretty stupid and non-productive thing for a user to do it is still a
>> potential risk that i think we should try and avoid.
>>
>> what i am now thinking may be the best idea is to setup either
>> interfaces or base/abstract versions of each pojo that will go into
>> the velocity context, and then when we are doing the loading of the
>> velocity context we can simply cast the real pojos into their more
>> limited versions. here's an example of what i mean ...
>>
>> - create interface Website which only contains getXXX() methods
>> - the velocity context gets, ctx.put("website", (Website)
>> WebsiteDataObject)
>> - XXXManager classes still pass around XXXData objects as usual
>>
>> this would mean we could control what methods are available to users
>> via Velocity without actually changing any of our existing pojos.
>> the only work would be to create the new interfaces, have each pojo
>> implement it's respective interface, then modify the context loading
>> methods to cast the pojos into the interface.
>>
>> the other cool thing is that this is not intrusive on any existing
>> functionality, so i believe i could start adding this in for the 1.3
>> release without causing any problems for Dave in his 2.0 branch.
>>
>> what do people think?
>>
>> -- Allen
>>
>>
>> On Fri, 2005-07-01 at 08:18, Anil Gangolli wrote:
>>
>>> I agree with this general direction of separating the persistence
>>> behavior from the data holder beans; let pojos be pojos.
>>>
>>> Putting the logic in managers does necessitate the need always to
>>> know about the association between the class and its manager. In
>>> methods that deal with multiple different classes of objects we do
>>> persist, that can become an issue.
>>>
>>> However, if we decide we want to be able to deal with instances that
>>> know how to save themselves, or determining if the current user
>>> can save them, etc, that behavior should really be in classes that
>>> derive from or wrap/delegate to the pojo data holders, not the
>>> other way around as it is now.
>>>
>>> So Allen, +1 to this idea
>>>
>>> --a.
>>>
>>>
>>> ----- Original Message -----
>>> From: "David M Johnson" <Da...@Sun.COM>
>>> To: <ro...@incubator.apache.org>
>>> Sent: Friday, July 01, 2005 7:09 AM
>>> Subject: Re: velocity context cleanup
>>>
>>>
>>>> +1!!!
>>>>
>>>> I'm guilty of moving methods like save(), remove(), etc. into the
>>>> POJOs. I think everything you've outlined below is basically a
>>>> good idea, but I'd like to hold off on it until we merge 2.0 back in.
>>>>
>>>> - Dave
>>>>
>>>>
>>>> On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
>>>>
>>>>> Okay, so it sounds like a few other people have given this a
>>>>> little thought and think that it may be beneficial to make some
>>>>> changes to the way the Pojos and PersistentObjects work. I think
>>>>> it would help to add a little more detail to the discussion so
>>>>> we know what we are talking about. Here's my stab at what changes
>>>>> I would think about making ...
>>>>>
>>>>> - move PersistentObject.save() into Manager classes only
>>>>> - move PersistentObject.remove() into Manager classes only
>>>>>
>>>>> I think those 2 changes would go a long ways toward making it less
>>>>> dangerous to make Pojos directly available to users via the
>>>>> velocity context. I am in partial agreement that we may not need
>>>>> the PersistentObject class at all. Right now I would also
>>>>> consider doing ...
>>>>>
>>>>> - remove PersistentObject.get/setId() (these are not necessarily
>>>>> part of all objects)
>>>>> - remove PersistentObject.setData() (this can easily be done
>>>>> elsewhere)
>>>>> - remove PersistentObject.canSave() (i don't fully understand how
>>>>> this is used, but i believe this logic can be in the Manager
>>>>> classes save/remove methods)
>>>>>
>>>>> If we also want to do those last few items then the
>>>>> PersistentObject class would basically be useless. I think the
>>>>> first 2 are
>>>>> pretty important, but the last 3 are optional. Personally I would
>>>>> probably go ahead and ditch the PersistentObject class just
>>>>> because I don't think we really need it.
>>>>>
>>>>> what do others think?
>>>>>
>>>>> Remember, we are just talking about this right now so please speak
>>>>> up and voice your opinion. We aren't going to make any
>>>>> changes right away, especially with the fact that Dave has a lot
>>>>> of data model work going on for the 2.0 release and we don't
>>>>> want to mess with what he has done so far. Once we get a bit more
>>>>> consenus then I will formalize a Proposal that can be reviewed
>>>>> again.
>>>>>
>>>>> -- Allen
>>>>>
>>>>>
>>>>> On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
>>>>>
>>>>>> I just wanted to chime in that I really dislike persistence methods
>>>>>> being in POJOs also and would be willing to pitch in with moving
>>>>>> those out to the appropriate manager classes. In fact, I would even
>>>>>> like to see PersistenceObject go as having to extend data objects
>>>>>> from it pretty much negates one of the key benefits of Hibernate --
>>>>>> its non-intrusiveness into the object model.
>>>>>>
>>>>>> Max
>>>>>>
>>>>>> On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
>>>>>>
>>>>>>>
>>>>>>> The remove() method is used in several cases to do some of the
>>>>>>> cascading needed to maintain consistency properties. Just make
>>>>>>> sure to preserve that logic; if you take out the remove() methods,
>>>>>>> this logic needs to be moved into the corresponding manager
>>>>>>> methods.
>>>>>>>
>>>>>>> --a.
>>>>>>>
>>>>>>>
>>>>>>> ----- Original Message ----- From: "Lance Lavandowska"
>>>>>>> <la...@gmail.com>
>>>>>>> To: <ro...@incubator.apache.org>
>>>>>>> Sent: Thursday, June 30, 2005 8:24 AM
>>>>>>> Subject: Re: velocity context cleanup
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> *Data.remove() is available to users (try $website.remove() in a
>>>>>>>>> template)
>>>>>>>>>
>>>>>>>>
>>>>>>>> This method should probably be removed from the classes. While I
>>>>>>>> think even POJOs should contain some business logic, I don't feel
>>>>>>>> that
>>>>>>>> persistence-related methods are appropriate. Because this is
>>>>>>>> only my
>>>>>>>> personal gut-check, I've never objected.
>>>>>>>>
>>>>>>>>
>>>>>>>>> PageHelper.evaluateString() is available to users (this one
>>>>>>>>> actually bit us in the ass already and a user caught themself in
>>>>>>>>> a recursive loop which killed the server)
>>>>>>>>>
>>>>>>>>
>>>>>>>> I'm the one guilty of creating that monstrosity, and I say "get
>>>>>>>> rid of
>>>>>>>> it". I doubt it is in my real use - but you may break a few
>>>>>>>> pages by
>>>>>>>> removing it. Perhaps change it to print "THIS MACRO HAS BEEN
>>>>>>>> REMOVED"? Note: this is a misguided macro, not a Context value.
>>>>>>>>
>>>>>>>>
>>>>>>>>> Some of these may be a simple case of updating the public,
>>>>>>>>> protected, private access levels on methods, but some cases may
>>>>>>>>> mean removing objects from the Context and/or removing methods
>>>>>>>>> from objects that are part of the Context.
>>>>>>>>>
>>>>>>>>
>>>>>>>> All of the objects placed into the Context are done so to
>>>>>>>> achieve an
>>>>>>>> objective in the *.vm templates or the Page templates. As I
>>>>>>>> implied
>>>>>>>> above, let's look at what is being exposed by these objects
>>>>>>>> that may
>>>>>>>> be 'dangerous' instead.
>>>>>>>>
>>>>>>>> Lance
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>
Re: velocity context cleanup
Posted by Dave Johnson <da...@rollerweblogger.org>.
I think that's a good idea, but (as far as I know) it won't keep weblog
template authors from accessing fields on WebsiteData. Velocity uses
reflection, so they won't be restricted to just the Website
methods/fields.
What's the best way to restrict template authors to just the methods in
the interface? Some sort of proxy class perhaps?
- Dave
On Jul 1, 2005, at 12:50 PM, Allen Gilliland wrote:
> okay, so it sounds like everyone agrees that this is a good thing to
> do at some point. i'll go ahead and work on a more formal proposal so
> we can see exactly what exact changes we would make.
>
> in addition to removing some really dangerous methods like
> website.remove() i think we'll also need a plan to prevent users from
> having access to <object>.setXXX(). i've run the same tests as i did
> with website.remove() and basically any user can set any property of
> any of the objects in the velocity context to whatever they want and
> these changes will go directly back to the db. while this would be a
> pretty stupid and non-productive thing for a user to do it is still a
> potential risk that i think we should try and avoid.
>
> what i am now thinking may be the best idea is to setup either
> interfaces or base/abstract versions of each pojo that will go into
> the velocity context, and then when we are doing the loading of the
> velocity context we can simply cast the real pojos into their more
> limited versions. here's an example of what i mean ...
>
> - create interface Website which only contains getXXX() methods
> - the velocity context gets, ctx.put("website", (Website)
> WebsiteDataObject)
> - XXXManager classes still pass around XXXData objects as usual
>
> this would mean we could control what methods are available to users
> via Velocity without actually changing any of our existing pojos. the
> only work would be to create the new interfaces, have each pojo
> implement it's respective interface, then modify the context loading
> methods to cast the pojos into the interface.
>
> the other cool thing is that this is not intrusive on any existing
> functionality, so i believe i could start adding this in for the 1.3
> release without causing any problems for Dave in his 2.0 branch.
>
> what do people think?
>
> -- Allen
>
>
> On Fri, 2005-07-01 at 08:18, Anil Gangolli wrote:
>> I agree with this general direction of separating the persistence
>> behavior from the data holder beans; let pojos be pojos.
>>
>> Putting the logic in managers does necessitate the need always to
>> know about the association between the class and its manager. In
>> methods that deal with multiple different classes of objects we do
>> persist, that can become an issue.
>>
>> However, if we decide we want to be able to deal with instances that
>> know how to save themselves, or determining if the current user
>> can save them, etc, that behavior should really be in classes that
>> derive from or wrap/delegate to the pojo data holders, not the
>> other way around as it is now.
>>
>> So Allen, +1 to this idea
>>
>> --a.
>>
>>
>> ----- Original Message -----
>> From: "David M Johnson" <Da...@Sun.COM>
>> To: <ro...@incubator.apache.org>
>> Sent: Friday, July 01, 2005 7:09 AM
>> Subject: Re: velocity context cleanup
>>
>>
>>> +1!!!
>>>
>>> I'm guilty of moving methods like save(), remove(), etc. into the
>>> POJOs. I think everything you've outlined below is basically a
>>> good idea, but I'd like to hold off on it until we merge 2.0 back in.
>>>
>>> - Dave
>>>
>>>
>>> On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
>>>
>>>> Okay, so it sounds like a few other people have given this a little
>>>> thought and think that it may be beneficial to make some
>>>> changes to the way the Pojos and PersistentObjects work. I think
>>>> it would help to add a little more detail to the discussion so
>>>> we know what we are talking about. Here's my stab at what changes
>>>> I would think about making ...
>>>>
>>>> - move PersistentObject.save() into Manager classes only
>>>> - move PersistentObject.remove() into Manager classes only
>>>>
>>>> I think those 2 changes would go a long ways toward making it less
>>>> dangerous to make Pojos directly available to users via the
>>>> velocity context. I am in partial agreement that we may not need
>>>> the PersistentObject class at all. Right now I would also
>>>> consider doing ...
>>>>
>>>> - remove PersistentObject.get/setId() (these are not necessarily
>>>> part of all objects)
>>>> - remove PersistentObject.setData() (this can easily be done
>>>> elsewhere)
>>>> - remove PersistentObject.canSave() (i don't fully understand how
>>>> this is used, but i believe this logic can be in the Manager
>>>> classes save/remove methods)
>>>>
>>>> If we also want to do those last few items then the
>>>> PersistentObject class would basically be useless. I think the
>>>> first 2 are
>>>> pretty important, but the last 3 are optional. Personally I would
>>>> probably go ahead and ditch the PersistentObject class just
>>>> because I don't think we really need it.
>>>>
>>>> what do others think?
>>>>
>>>> Remember, we are just talking about this right now so please speak
>>>> up and voice your opinion. We aren't going to make any
>>>> changes right away, especially with the fact that Dave has a lot of
>>>> data model work going on for the 2.0 release and we don't
>>>> want to mess with what he has done so far. Once we get a bit more
>>>> consenus then I will formalize a Proposal that can be reviewed
>>>> again.
>>>>
>>>> -- Allen
>>>>
>>>>
>>>> On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
>>>>> I just wanted to chime in that I really dislike persistence methods
>>>>> being in POJOs also and would be willing to pitch in with moving
>>>>> those out to the appropriate manager classes. In fact, I would even
>>>>> like to see PersistenceObject go as having to extend data objects
>>>>> from it pretty much negates one of the key benefits of Hibernate --
>>>>> its non-intrusiveness into the object model.
>>>>>
>>>>> Max
>>>>>
>>>>> On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
>>>>>
>>>>>>
>>>>>> The remove() method is used in several cases to do some of the
>>>>>> cascading needed to maintain consistency properties. Just make
>>>>>> sure to preserve that logic; if you take out the remove() methods,
>>>>>> this logic needs to be moved into the corresponding manager
>>>>>> methods.
>>>>>>
>>>>>> --a.
>>>>>>
>>>>>>
>>>>>> ----- Original Message ----- From: "Lance Lavandowska"
>>>>>> <la...@gmail.com>
>>>>>> To: <ro...@incubator.apache.org>
>>>>>> Sent: Thursday, June 30, 2005 8:24 AM
>>>>>> Subject: Re: velocity context cleanup
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> *Data.remove() is available to users (try $website.remove() in a
>>>>>>>> template)
>>>>>>>>
>>>>>>>
>>>>>>> This method should probably be removed from the classes. While I
>>>>>>> think even POJOs should contain some business logic, I don't feel
>>>>>>> that
>>>>>>> persistence-related methods are appropriate. Because this is
>>>>>>> only my
>>>>>>> personal gut-check, I've never objected.
>>>>>>>
>>>>>>>
>>>>>>>> PageHelper.evaluateString() is available to users (this one
>>>>>>>> actually bit us in the ass already and a user caught themself in
>>>>>>>> a recursive loop which killed the server)
>>>>>>>>
>>>>>>>
>>>>>>> I'm the one guilty of creating that monstrosity, and I say "get
>>>>>>> rid of
>>>>>>> it". I doubt it is in my real use - but you may break a few
>>>>>>> pages by
>>>>>>> removing it. Perhaps change it to print "THIS MACRO HAS BEEN
>>>>>>> REMOVED"? Note: this is a misguided macro, not a Context value.
>>>>>>>
>>>>>>>
>>>>>>>> Some of these may be a simple case of updating the public,
>>>>>>>> protected, private access levels on methods, but some cases may
>>>>>>>> mean removing objects from the Context and/or removing methods
>>>>>>>> from objects that are part of the Context.
>>>>>>>>
>>>>>>>
>>>>>>> All of the objects placed into the Context are done so to
>>>>>>> achieve an
>>>>>>> objective in the *.vm templates or the Page templates. As I
>>>>>>> implied
>>>>>>> above, let's look at what is being exposed by these objects that
>>>>>>> may
>>>>>>> be 'dangerous' instead.
>>>>>>>
>>>>>>> Lance
>>>>>>
>>>>>
>>>
>>
Re: velocity context cleanup
Posted by Allen Gilliland <Al...@Sun.COM>.
okay, so it sounds like everyone agrees that this is a good thing to do at some point. i'll go ahead and work on a more formal proposal so we can see exactly what exact changes we would make.
in addition to removing some really dangerous methods like website.remove() i think we'll also need a plan to prevent users from having access to <object>.setXXX(). i've run the same tests as i did with website.remove() and basically any user can set any property of any of the objects in the velocity context to whatever they want and these changes will go directly back to the db. while this would be a pretty stupid and non-productive thing for a user to do it is still a potential risk that i think we should try and avoid.
what i am now thinking may be the best idea is to setup either interfaces or base/abstract versions of each pojo that will go into the velocity context, and then when we are doing the loading of the velocity context we can simply cast the real pojos into their more limited versions. here's an example of what i mean ...
- create interface Website which only contains getXXX() methods
- the velocity context gets, ctx.put("website", (Website) WebsiteDataObject)
- XXXManager classes still pass around XXXData objects as usual
this would mean we could control what methods are available to users via Velocity without actually changing any of our existing pojos. the only work would be to create the new interfaces, have each pojo implement it's respective interface, then modify the context loading methods to cast the pojos into the interface.
the other cool thing is that this is not intrusive on any existing functionality, so i believe i could start adding this in for the 1.3 release without causing any problems for Dave in his 2.0 branch.
what do people think?
-- Allen
On Fri, 2005-07-01 at 08:18, Anil Gangolli wrote:
> I agree with this general direction of separating the persistence behavior from the data holder beans; let pojos be pojos.
>
> Putting the logic in managers does necessitate the need always to know about the association between the class and its manager. In
> methods that deal with multiple different classes of objects we do persist, that can become an issue.
>
> However, if we decide we want to be able to deal with instances that know how to save themselves, or determining if the current user
> can save them, etc, that behavior should really be in classes that derive from or wrap/delegate to the pojo data holders, not the
> other way around as it is now.
>
> So Allen, +1 to this idea
>
> --a.
>
>
> ----- Original Message -----
> From: "David M Johnson" <Da...@Sun.COM>
> To: <ro...@incubator.apache.org>
> Sent: Friday, July 01, 2005 7:09 AM
> Subject: Re: velocity context cleanup
>
>
> > +1!!!
> >
> > I'm guilty of moving methods like save(), remove(), etc. into the POJOs. I think everything you've outlined below is basically a
> > good idea, but I'd like to hold off on it until we merge 2.0 back in.
> >
> > - Dave
> >
> >
> > On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
> >
> >> Okay, so it sounds like a few other people have given this a little thought and think that it may be beneficial to make some
> >> changes to the way the Pojos and PersistentObjects work. I think it would help to add a little more detail to the discussion so
> >> we know what we are talking about. Here's my stab at what changes I would think about making ...
> >>
> >> - move PersistentObject.save() into Manager classes only
> >> - move PersistentObject.remove() into Manager classes only
> >>
> >> I think those 2 changes would go a long ways toward making it less dangerous to make Pojos directly available to users via the
> >> velocity context. I am in partial agreement that we may not need the PersistentObject class at all. Right now I would also
> >> consider doing ...
> >>
> >> - remove PersistentObject.get/setId() (these are not necessarily part of all objects)
> >> - remove PersistentObject.setData() (this can easily be done elsewhere)
> >> - remove PersistentObject.canSave() (i don't fully understand how this is used, but i believe this logic can be in the Manager
> >> classes save/remove methods)
> >>
> >> If we also want to do those last few items then the PersistentObject class would basically be useless. I think the first 2 are
> >> pretty important, but the last 3 are optional. Personally I would probably go ahead and ditch the PersistentObject class just
> >> because I don't think we really need it.
> >>
> >> what do others think?
> >>
> >> Remember, we are just talking about this right now so please speak up and voice your opinion. We aren't going to make any
> >> changes right away, especially with the fact that Dave has a lot of data model work going on for the 2.0 release and we don't
> >> want to mess with what he has done so far. Once we get a bit more consenus then I will formalize a Proposal that can be reviewed
> >> again.
> >>
> >> -- Allen
> >>
> >>
> >> On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
> >>> I just wanted to chime in that I really dislike persistence methods
> >>> being in POJOs also and would be willing to pitch in with moving
> >>> those out to the appropriate manager classes. In fact, I would even
> >>> like to see PersistenceObject go as having to extend data objects
> >>> from it pretty much negates one of the key benefits of Hibernate --
> >>> its non-intrusiveness into the object model.
> >>>
> >>> Max
> >>>
> >>> On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
> >>>
> >>>>
> >>>> The remove() method is used in several cases to do some of the
> >>>> cascading needed to maintain consistency properties. Just make
> >>>> sure to preserve that logic; if you take out the remove() methods,
> >>>> this logic needs to be moved into the corresponding manager methods.
> >>>>
> >>>> --a.
> >>>>
> >>>>
> >>>> ----- Original Message ----- From: "Lance Lavandowska"
> >>>> <la...@gmail.com>
> >>>> To: <ro...@incubator.apache.org>
> >>>> Sent: Thursday, June 30, 2005 8:24 AM
> >>>> Subject: Re: velocity context cleanup
> >>>>
> >>>>
> >>>>
> >>>>> On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
> >>>>>
> >>>>>
> >>>>>> *Data.remove() is available to users (try $website.remove() in a
> >>>>>> template)
> >>>>>>
> >>>>>
> >>>>> This method should probably be removed from the classes. While I
> >>>>> think even POJOs should contain some business logic, I don't feel
> >>>>> that
> >>>>> persistence-related methods are appropriate. Because this is only my
> >>>>> personal gut-check, I've never objected.
> >>>>>
> >>>>>
> >>>>>> PageHelper.evaluateString() is available to users (this one
> >>>>>> actually bit us in the ass already and a user caught themself in
> >>>>>> a recursive loop which killed the server)
> >>>>>>
> >>>>>
> >>>>> I'm the one guilty of creating that monstrosity, and I say "get
> >>>>> rid of
> >>>>> it". I doubt it is in my real use - but you may break a few pages by
> >>>>> removing it. Perhaps change it to print "THIS MACRO HAS BEEN
> >>>>> REMOVED"? Note: this is a misguided macro, not a Context value.
> >>>>>
> >>>>>
> >>>>>> Some of these may be a simple case of updating the public,
> >>>>>> protected, private access levels on methods, but some cases may
> >>>>>> mean removing objects from the Context and/or removing methods
> >>>>>> from objects that are part of the Context.
> >>>>>>
> >>>>>
> >>>>> All of the objects placed into the Context are done so to achieve an
> >>>>> objective in the *.vm templates or the Page templates. As I implied
> >>>>> above, let's look at what is being exposed by these objects that may
> >>>>> be 'dangerous' instead.
> >>>>>
> >>>>> Lance
> >>>>
> >>>
> >
>
Re: velocity context cleanup
Posted by Anil Gangolli <an...@busybuddha.org>.
I agree with this general direction of separating the persistence behavior from the data holder beans; let pojos be pojos.
Putting the logic in managers does necessitate the need always to know about the association between the class and its manager. In
methods that deal with multiple different classes of objects we do persist, that can become an issue.
However, if we decide we want to be able to deal with instances that know how to save themselves, or determining if the current user
can save them, etc, that behavior should really be in classes that derive from or wrap/delegate to the pojo data holders, not the
other way around as it is now.
So Allen, +1 to this idea
--a.
----- Original Message -----
From: "David M Johnson" <Da...@Sun.COM>
To: <ro...@incubator.apache.org>
Sent: Friday, July 01, 2005 7:09 AM
Subject: Re: velocity context cleanup
> +1!!!
>
> I'm guilty of moving methods like save(), remove(), etc. into the POJOs. I think everything you've outlined below is basically a
> good idea, but I'd like to hold off on it until we merge 2.0 back in.
>
> - Dave
>
>
> On Jun 30, 2005, at 10:50 AM, Allen Gilliland wrote:
>
>> Okay, so it sounds like a few other people have given this a little thought and think that it may be beneficial to make some
>> changes to the way the Pojos and PersistentObjects work. I think it would help to add a little more detail to the discussion so
>> we know what we are talking about. Here's my stab at what changes I would think about making ...
>>
>> - move PersistentObject.save() into Manager classes only
>> - move PersistentObject.remove() into Manager classes only
>>
>> I think those 2 changes would go a long ways toward making it less dangerous to make Pojos directly available to users via the
>> velocity context. I am in partial agreement that we may not need the PersistentObject class at all. Right now I would also
>> consider doing ...
>>
>> - remove PersistentObject.get/setId() (these are not necessarily part of all objects)
>> - remove PersistentObject.setData() (this can easily be done elsewhere)
>> - remove PersistentObject.canSave() (i don't fully understand how this is used, but i believe this logic can be in the Manager
>> classes save/remove methods)
>>
>> If we also want to do those last few items then the PersistentObject class would basically be useless. I think the first 2 are
>> pretty important, but the last 3 are optional. Personally I would probably go ahead and ditch the PersistentObject class just
>> because I don't think we really need it.
>>
>> what do others think?
>>
>> Remember, we are just talking about this right now so please speak up and voice your opinion. We aren't going to make any
>> changes right away, especially with the fact that Dave has a lot of data model work going on for the 2.0 release and we don't
>> want to mess with what he has done so far. Once we get a bit more consenus then I will formalize a Proposal that can be reviewed
>> again.
>>
>> -- Allen
>>
>>
>> On Thu, 2005-06-30 at 10:12, Rudman Max wrote:
>>> I just wanted to chime in that I really dislike persistence methods
>>> being in POJOs also and would be willing to pitch in with moving
>>> those out to the appropriate manager classes. In fact, I would even
>>> like to see PersistenceObject go as having to extend data objects
>>> from it pretty much negates one of the key benefits of Hibernate --
>>> its non-intrusiveness into the object model.
>>>
>>> Max
>>>
>>> On Jun 30, 2005, at 11:53 AM, Anil Gangolli wrote:
>>>
>>>>
>>>> The remove() method is used in several cases to do some of the
>>>> cascading needed to maintain consistency properties. Just make
>>>> sure to preserve that logic; if you take out the remove() methods,
>>>> this logic needs to be moved into the corresponding manager methods.
>>>>
>>>> --a.
>>>>
>>>>
>>>> ----- Original Message ----- From: "Lance Lavandowska"
>>>> <la...@gmail.com>
>>>> To: <ro...@incubator.apache.org>
>>>> Sent: Thursday, June 30, 2005 8:24 AM
>>>> Subject: Re: velocity context cleanup
>>>>
>>>>
>>>>
>>>>> On 6/29/05, Allen Gilliland <Al...@sun.com> wrote:
>>>>>
>>>>>
>>>>>> *Data.remove() is available to users (try $website.remove() in a
>>>>>> template)
>>>>>>
>>>>>
>>>>> This method should probably be removed from the classes. While I
>>>>> think even POJOs should contain some business logic, I don't feel
>>>>> that
>>>>> persistence-related methods are appropriate. Because this is only my
>>>>> personal gut-check, I've never objected.
>>>>>
>>>>>
>>>>>> PageHelper.evaluateString() is available to users (this one
>>>>>> actually bit us in the ass already and a user caught themself in
>>>>>> a recursive loop which killed the server)
>>>>>>
>>>>>
>>>>> I'm the one guilty of creating that monstrosity, and I say "get
>>>>> rid of
>>>>> it". I doubt it is in my real use - but you may break a few pages by
>>>>> removing it. Perhaps change it to print "THIS MACRO HAS BEEN
>>>>> REMOVED"? Note: this is a misguided macro, not a Context value.
>>>>>
>>>>>
>>>>>> Some of these may be a simple case of updating the public,
>>>>>> protected, private access levels on methods, but some cases may
>>>>>> mean removing objects from the Context and/or removing methods
>>>>>> from objects that are part of the Context.
>>>>>>
>>>>>
>>>>> All of the objects placed into the Context are done so to achieve an
>>>>> objective in the *.vm templates or the Page templates. As I implied
>>>>> above, let's look at what is being exposed by these objects that may
>>>>> be 'dangerous' instead.
>>>>>
>>>>> Lance
>>>>
>>>
>