comments inline ...
Dave Johnson wrote:
Comments inline...
*snip*
Unfortunately this means we basically would need to leave the existing
ContextLoading stuff as is, but consider it deprecated. Then moving
forward we redo our velocity themes/templates to use the new model
objects.
Disagree. We need to clean this stuff up into a nice little bundles
(the page models) so Roller can work without it. I'll explain that below.
I agree and disagree. I agree that cleanup is important, but I disagree
that we have to put the old ContextLoading stuff into the new PageModel
stuff.
More details below.
> 3) we create three different page models for our three different
rendering
> situations: weblog display, weblog entry/comments display and feed
display.
> First, all page models implement this interface because they're
pluggable:
>
> interface PageModel
> getModelName()
> init(request, response, TemplateContext)
>
This is a fairly different approach than what I was thinking about. In
this approach you aren't really using Models just to provide rendering
functionality, instead they are used in large part for providing context
loading functionality. I would prefer that we keep those 2 things
separate actually.
I totally agree that, moving forward, our page models should not put
things into the context .
But some page model developers might not share our great love of
page models, so why not leave the Context argument in the init()
method so developers can decide for themselves what goes in context
with their page model? Plus, we need it ourselves.
I think I am confused. If moving forward we don't want to let models
put things in the context, then why give them the opportunity?
Personally, I think that page model developers can get along fine
without having to place objects in the context directly. To me the
point of using the PageModel is to provide some encapsulation so that
instead of having a context with this in it ...
obj1
obj2
obj3
etc
etc2
we have something more organized like this ...
mymodel.obj1
mymodel.obj2
mymodel.obj3
mymodel.etc
mymodel.etc2
you can still access the same exact data and methods through a model
that you can by putting the objects directly into the context.
An example is the way we use Models in the admin/authoring UI struts
pages. The struts actions load up the model (jsp scopes like request,
page, session) with any objects or Models that are to be used at
rendering time. However the Models are not allowed to write their own
stuff to the jsp scopes. I would prefer we follow this approach.
What I am proposing is that instead of setting up the VelocityContext
like we have in the past, where it's littered with dozens and dozens of
variables everywhere. Instead we just load it with Models moving
forward and all rendering functionality is used via the models.
Yes and that's exactly why I started the move to page models in the
editor UI, I hate littering the request with typeless attributes.
> 4) change the PageServlet and FlavorServlet classes to load the
appropriate
> page models.
>
I still agree with this sentiment that the Servlets should load only the
models which are needed/allowed to render the content, but I am not on
board with all the various Models that you listed.
Do you really object to the three models: weblog, entry and feed -- or are
you only objecting to moving context loader stuff to page models?
No, i don't object completely. I object to the approach that we need to
split up the various page models in order organize the context loading
process.
If these various page models really are going to offer access to
different data and different methods then that's fine.
Ok, so I guess the best argument I have for doing this work is:
If we tuck the context loading stuff away into the page models, make the
page models configurable as I have suggested then folks who do not want
to use our page models can simply provide their own and they will not have
their velocity context's littered with the ContextLoader crap.
This sounds like a double edged sword to me. On one hand I agree it's
nice for someone to plug in their own page model and ditch all of our
old crud, but there are big ramifications to that. There is a large
potential for someone to mess up and replace our page model with a
custom one when they still need all of our legacy context stuff.
If we want to let someone EOL our old context loading stuff then I
suggest we do it apart from the page models.
We're doing to have to live with the old page models for a while, I'm
documenting them now and we're using them in 3.0. So why not treat
them systematically and make it possible for people to completely
opt-out of them if they want to provide thier own page models.
I agree that the page models should be fully pluggable, but I see more
danger than benefit in letting people remove all the old stuff loaded
into the context.
If you still object to this approach, maybe you could suggest an
alternative
plan for accomplishing these goals?
My plan would simply be to deprecate all of the old stuff in the context
rather than try and EOL it. We'd do that in these general steps ...
1. Round up the current context loading stuff and any objects it uses
and group it all together somewhere that is identified as legacy context
loading crud. This includes the current page model, page helper, etc.
Something like ui.rendering.legacy or ui.rendering.old.
2. Leave the legacy context loading process in place on the page
servlet, which is the only place that users have templates that use that
stuff. Remove it from everywhere else, feed, search, etc.
3. Define completely new page models to be used moving forward. These
models will be totally self contained and designed how we want things to
work moving foward.
4. Rewrite our existing velocity templates and themes to use only the
new page models and any other supporting objects that we want to
continue supporting for the long term. To lighten this burden I suggest
that we make 3.0 the point at which we EOL all the old themes and pick
just a couple new ones to refactor and ship with Roller for 3.0 and beyond.
5. Optionally we can define some kind of mechanism to turn off the old
context loading stuff. Probably the best way to do this is to begin
tracking what version of Roller the person began their installation
with. So starting with new installations 3.0 we could have the old
context loading stuff disabled by default. This is still a possible
problem though, if someone gets their hands on old templates/themes in a
3.0+ install they will no longer work, so we need to tread carefully here.
So the basic approach is to isolate all of the old stuff and consider it
deprecated. Then change our existing stuff to only use the new code
which we want to use moving forward.
The other fairly cool thing about this is that we would only document
the new models and objects that we really want to support as we create
them. We don't want to tell people about the old stuff and give them a
chance to start doing things the wrong way.
-- Allen
- Dave
