Just one suggestion, put the wrappers in a sub-package, perhaps
org.roller.presentation.velocity.pojos.wrappers ?
On 7/4/05, Allen Gilliland <[EMAIL PROTECTED]> 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" <[EMAIL PROTECTED]>
> >>>> To: <[email protected]>
> >>>> 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"
> >>>>>>>> <[EMAIL PROTECTED]>
> >>>>>>>> To: <[email protected]>
> >>>>>>>> Sent: Thursday, June 30, 2005 8:24 AM
> >>>>>>>> Subject: Re: velocity context cleanup
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On 6/29/05, Allen Gilliland <[EMAIL PROTECTED]> 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
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>
> >>>>
> >>
>
