@Decorated pages and Stage.PRODUCTION

40 views
Skip to first unread message

Marcio Endo

unread,
Dec 11, 2011, 1:31:17 PM12/11/11
to Google Sitebricks
Hey guys,

Didn't quite grasp why this was happening, hope you can help.

I was getting a NPE in the DecorateWidget:

// get the extension page by name
PageBook.Page page =
book.forName(DecorateWidget.embedNameFor(templateClass));

// create a dummy respond to collect the output of the embedded
page
StringBuilderRespond sbrespond = new StringBuilderRespond(new
Object());
EmbeddedRespond embedded = new EmbeddedRespond(null, sbrespond);
page.widget().render(bound, embedded);

as page.widget() was returning null.

This does not happen in Stage.DEVELOPMENT as DebugModePageBook reloads
the template during runtime.

Anyways I hacked a quick solution by changing the
ScanAndCompilerBootstrapper. I've made a pull request in Github.

The problem is I couldn't reproduce the issue in the acceptance-tests;
I've ran the tests (or the DecoratorAcceptanceTest) with
Stage.PRODUCTION and it passes.

So basically I'm lost here, can anyone help me figure out what is
happening?

Thanks.

dha...@gmail.com

unread,
Dec 11, 2011, 6:08:02 PM12/11/11
to google-s...@googlegroups.com, marci...@objectos.com.br
Hmm, I'm not entirely sure how this code works, perhaps John can shed some light as he wrote it?

James Wilson

unread,
Dec 12, 2011, 7:54:16 AM12/12/11
to google-s...@googlegroups.com, marci...@objectos.com.br
This sounds like the same issue I ran into.  I think the problem is in embedNameFor.  But I assumed my approach is too weak as a permanent solution to how the enhanced class is given a new name from what is registered in the PageBook.

Marcio Endo

unread,
Jan 9, 2012, 9:58:25 AM1/9/12
to Google Sitebricks
Got some to time to look into this issue. Managed to reproduce it.

Code is in this branch:
https://github.com/objectos/sitebricks/tree/decorated_and_production

Most relevant part:
https://github.com/objectos/sitebricks/commit/c630678786841c063d0d293b5032d1e9a09f581d

Anyways, I'll try and debug it a little this week(end).

But if anyone want/can throw in some suggestions / pointers it will be
much appreciated.

Regards!
Marcio Endo

On 12 dez 2011, 10:54, James Wilson <jw.ma...@gmail.com> wrote:
> This sounds like the same issue I ran into.  I think the problem is in
> embedNameFor.  But I assumed my approach is too weak as a permanent
> solution to how the enhanced class is given a new name from what is
> registered in the PageBook.
>
> http://code.google.com/p/google-sitebricks/issues/detail?id=77&q=embe...

James

unread,
Jan 29, 2012, 10:26:28 AM1/29/12
to Google Sitebricks
Looks like a bug in ScanAndCompileBootstrapper in how the decorated
Page object is (attempted to be) added to a Set<PageBook.Page>. The
DefaultPageBook.PageTuple equals and hashcode methods use the clazz so
pagesToCompile" does not add them.

This is not a problem in when not in Production mode since the Page is
compiled at each access the DebugModePageBook has it's handy reload
method to perform the compile.

upon looking deeper, the pagesToCompile set is (attempted to be)
modified in the extendedPages method.

Fixing this won't be too difficult.

-James


On Jan 9, 8:58 am, Marcio Endo <marcio.e...@objectos.com.br> wrote:
> Got some to time to look into this issue. Managed to reproduce it.
>
> Code is in this branch:https://github.com/objectos/sitebricks/tree/decorated_and_production
>
> Most relevant part:https://github.com/objectos/sitebricks/commit/c630678786841c063d0d293...

Marcio Endo

unread,
Jan 30, 2012, 8:59:09 AM1/30/12
to Google Sitebricks
Thanks for the help! I'll try that.

In fact, it seems quite simple!

Anyways I was having trouble finding the source of the problem so I
went on and started some refactoring in the ScanAndCompileBootstrapper
to see if I could understand more of the code.

I did some work on it the day before yesterday. I think it still needs
a lot of work (tests in particular) so I have a question for you guys:

- Should I proceed with the refactoring?

I guess this commit gives an idea of what I am trying to do:

https://github.com/objectos/sitebricks/commit/8b1228b3c1e1c569940bb8a1e826f98eb7f17f7a

If you guys are OK with it I'll try to do more work on it this
week(end).

And, of course, with James's solution implemented!

James Wilson

unread,
Feb 14, 2012, 6:28:13 PM2/14/12
to google-s...@googlegroups.com
I fixed this with a minimalistic approach in this commit:

Dhanji R. Prasanna

unread,
Feb 14, 2012, 6:30:08 PM2/14/12
to google-s...@googlegroups.com, jw.m...@gmail.com
Can you guys send a pull request when you're happy with the solution please? Thanks!

James Wilson

unread,
Feb 14, 2012, 6:50:17 PM2/14/12
to dha...@gmail.com, google-s...@googlegroups.com

I have to fix the branch first .  But I sure will.

James Wilson

unread,
Feb 15, 2012, 11:59:14 AM2/15/12
to dha...@gmail.com, google-s...@googlegroups.com
Pull submitted.  At least, well, I'm happy with it :)  Marcio, please chime in on the comments of the pull request if you've taken it a different direction with your refactor of ScanAndComplieBootstrapper.

https://github.com/dhanji/sitebricks/pull/17

Marcio Endo

unread,
Feb 16, 2012, 12:05:40 PM2/16/12
to Google Sitebricks
Sorry you guys,

Haven't looked into this since my last msg, been working on getting a
(hopefully) new client.

I'll send a pull request as soon as I have something.

But please, don't mind me.

I'll merge James' fix into my branch and continue with the refactoring
as soon as I can.
Reply all
Reply to author
Forward
0 new messages