Creating forward pages with new instead of context.createPage().

[Finn Bock]

Hi,

I wonder if it isn't possible to relax the restriction that page
instances passed to setForward() must be created by
getContext().createPage(MyPage.class);

Page dependencies must of course still be injected, but it could be
done as a separate step:

  MyPage page = new MyPage(true);
  getContext().initPage(page);
  setForward(page);

The fictional method "initPage()" will perform the injection of page
dependencies currently found in ClickServlet.initPage():
  activatePageInstance
  setHeaders
  setPath
  auto binding
  processPageRequestParams

This way it is possible to use arguments in the constructor to forwarded pages.

If such a change is interesting, I would be happy to write a patch.

regards,
Finn

[Malcolm Edgar-2]

Hi Finn,

What about setting the Page arguments as properties in your page?

 MyPage page = getContext().createPage(MyPage.class);
 page.setSecure(true);
 setForward(page);

regards Malcolm Edgar

On Mon, Aug 31, 2009 at 6:37 AM, Finn Bock<[hidden email]> wrote:

> Hi,
>
> I wonder if it isn't possible to relax the restriction that page
> instances passed to setForward() must be created by
> getContext().createPage(MyPage.class);
>
> Page dependencies must of course still be injected, but it could be
> done as a separate step:
>
>  MyPage page = new MyPage(true);
>  getContext().initPage(page);
>  setForward(page);
>
> The fictional method "initPage()" will perform the injection of page
> dependencies currently found in ClickServlet.initPage():
>  activatePageInstance
>  setHeaders
>  setPath
>  auto binding
>  processPageRequestParams
>
> This way it is possible to use arguments in the constructor to forwarded pages.
>
> If such a change is interesting, I would be happy to write a patch.
>
> regards,
> Finn
>

[Malcolm Edgar-2]

Hi Finn,

What about setting the Page arguments as properties in your page?

 MyPage page = getContext().createPage(MyPage.class);
 page.setSecure(true);
 setForward(page);

regards Malcolm Edgar

On Mon, Aug 31, 2009 at 6:37 AM, Finn Bock<[hidden email]> wrote:

> Hi,
>
> I wonder if it isn't possible to relax the restriction that page
> instances passed to setForward() must be created by
> getContext().createPage(MyPage.class);
>
> Page dependencies must of course still be injected, but it could be
> done as a separate step:
>
>  MyPage page = new MyPage(true);
>  getContext().initPage(page);
>  setForward(page);
>
> The fictional method "initPage()" will perform the injection of page
> dependencies currently found in ClickServlet.initPage():
>  activatePageInstance
>  setHeaders
>  setPath
>  auto binding
>  processPageRequestParams
>
> This way it is possible to use arguments in the constructor to forwarded pages.
>
> If such a change is interesting, I would be happy to write a patch.
>
> regards,
> Finn
>

[Finn Bock]

Malcolm Edgar <[hidden email]>:
> Hi Finn,
>
> What about setting the Page arguments as properties in your page?
>
>  MyPage page = getContext().createPage(MyPage.class);
>  page.setSecure(true);
>  setForward(page);

Yes, but I would like the arguments to be available in the
constructor. So the solution available right now is:

  MyPage page = getContext().createPage(MyPage.class);
  getContext().setRequestAttribute("secure", true);
  setForward(page);

and in the page ctor:

  public MyPage() {
     Boolean secure = (Boolean) getContext().getRequestAttribute("secure");
     ...
  }

and it just feels wrong that it is easier to pass request parameters
into @Bindable fields (which btw is very nice) than it is to pass
ordinary java objects into a class constructor.

regards,
Finn

[Malcolm Edgar-2]

Hi Finn,

OK sounds fair enough. Would you like to create a JIRA item for this
and provide a patch.

regards Malcolm Edgar

On Mon, Aug 31, 2009 at 5:35 PM, Finn Bock<[hidden email]> wrote:

> Malcolm Edgar <[hidden email]>:
>> Hi Finn,
>>
>> What about setting the Page arguments as properties in your page?
>>
>>  MyPage page = getContext().createPage(MyPage.class);
>>  page.setSecure(true);
>>  setForward(page);
>
> Yes, but I would like the arguments to be available in the
> constructor. So the solution available right now is:
>
>  MyPage page = getContext().createPage(MyPage.class);
>  getContext().setRequestAttribute("secure", true);
>  setForward(page);
>
> and in the page ctor:
>
>  public MyPage() {
>     Boolean secure = (Boolean) getContext().getRequestAttribute("secure");
>     ...
>  }
>
> and it just feels wrong that it is easier to pass request parameters
> into @Bindable fields (which btw is very nice) than it is to pass
> ordinary java objects into a class constructor.
>
> regards,
> Finn
>

[Bob Schellink-2]

Guess it comes down to a preference of constructor vs setter injection.

I don't see a big problem with exposing the initPage(Page) method on
Context. Perhaps the name initPage might be confused with onInit, so we
should think about whether initPage is the right name. How about
configurePage? We should also give some thought on what will happen if a
user accidentally invokes configurePage after a createPage call.

Keep in mind that a no-arg Page constructor is still needed if the Page
is to be accessed from the browser, since Click won't know how to
construct a Page which accepts arguments in its constructor.

Finn, perhaps you can open a JIRA and attach your patch?

kind regards

bob


Malcolm Edgar wrote:

> Hi Finn,
>
> What about setting the Page arguments as properties in your page?
>
>  MyPage page = getContext().createPage(MyPage.class);
>  page.setSecure(true);
>  setForward(page);
>
> regards Malcolm Edgar
>
> On Mon, Aug 31, 2009 at 6:37 AM, Finn Bock<[hidden email]> wrote:
>> Hi,
>>
>> I wonder if it isn't possible to relax the restriction that page
>> instances passed to setForward() must be created by
>> getContext().createPage(MyPage.class);
>>
>> Page dependencies must of course still be injected, but it could be
>> done as a separate step:
>>
>>  MyPage page = new MyPage(true);
>>  getContext().initPage(page);
>>  setForward(page);
>>
>> The fictional method "initPage()" will perform the injection of page
>> dependencies currently found in ClickServlet.initPage():
>>  activatePageInstance
>>  setHeaders
>>  setPath
>>  auto binding
>>  processPageRequestParams
>>
>> This way it is possible to use arguments in the constructor to forwarded pages.
>>
>> If such a change is interesting, I would be happy to write a patch.
>>
>> regards,
>> Finn
>>
>

[Finn Bock]

Bob Schellink <[hidden email]>:

> Guess it comes down to a preference of constructor vs setter injection.
>
> I don't see a big problem with exposing the initPage(Page) method on
> Context. Perhaps the name initPage might be confused with onInit, so we
> should think about whether initPage is the right name. How about
> configurePage? We should also give some thought on what will happen if a
> user accidentally invokes configurePage after a createPage call.
>
> Keep in mind that a no-arg Page constructor is still needed if the Page is
> to be accessed from the browser, since Click won't know how to construct a
> Page which accepts arguments in its constructor.
>
> Finn, perhaps you can open a JIRA and attach your patch?

Added as CLK-581. It also includes some unit test, but I'm far from
certain that the test is done right. Please review and comment.

regards,
Finn