Re: Generified Component

[Sven Meier]

Hi,

LazyModel is fairly new, so don't be surprised to find bugs in it :P
I'm using a mixture of LazyModels and AbstractReadOnlyModels in my 
project and this provides enough type information for it to work.

Developing LazyModel I wanted to prevent calling #getObject() on the 
nested model at all costs, but now it is used as a fallback. Of course 
this works only if the contained object is not null.

I've added another test org.wicketstuff.lazymodel.FormComponentTest that 
explicitely tests usage of a LazyModel in a textField.

Best regards
Sven

On 06/14/2013 11:33 PM, tetsuo wrote:
I still have one error: when AbstractTextComponent tries to get the object
class from IObjectClassAwareModel and it returns null, it calls
model.toString() to log a warning message. This triggers
LazyModel.getPath(), that also uses Generics.getClass(type), thus throwing
the
same IllegalArgumentException.

But I'm intrigued... has nobody ever hit this problem? This happened to me
the first time I tried to use it! I thought my wicket code was good, but
now I think I must be doing something very wrong...





On Fri, Jun 14, 2013 at 6:10 PM, Sven Meier <s...@meiers.net> wrote:

Hi,

I gave it another try, there should now be less exceptions thrown.

I'd be interested to see your usecase if you find something else missing.

Thanks
Sven



On 06/14/2013 09:18 PM, tetsuo wrote:

This change kind of works (I had to change getPath() too), but then for
every component I use LazyModel this way (every textfield in my case), I
get three exceptions thrown, then ignored (2 at getObjectClass() and 1 at
getPath()).

I guess using exceptions to flow control isn't very efficient, too... :)

Could Generics.getClass(type) be changed to not throw
IllegalArgumentException when it can't resolve the class? Maybe return
null
and let callers deal with it?







On Fri, Jun 14, 2013 at 2:52 AM, Sven Meier <s...@meiers.net> wrote:

  Hi Tetsuo,
I stand corrected:
For #getObjectClass() the model had to be bound to something revealing
its
type.

We cannot hold a reference to the type because this could lead to all
sorts of class loading problems (see Wicket's IClassResolver).
Furthermore
it would unnecessarily increase the size of LazyModel in most cases (if
everything is properly typed).

I improved LazyModel to behave like PropertyModel, i.e. return the result
object's type as fallback or just return null if everything fails.

Sven


On 06/14/2013 02:24 AM, tetsuo wrote:

  I made the changes to make it work, added a test case, and sent a pull
request on github.
Now it works when you create an unbound model from a class, and then
bind
it to a plain model.


On Thu, Jun 13, 2013 at 6:39 PM, tetsuo <ronald.tet...@gmail.com>
wrote:

   Well, you were the one who said that if I created the unbound model

(from(A.class)) then I could bind it to a plain IModel, without the
extra
type information :)

I just assumed that it would keep the 'A.class' information passed in
the
beginning and use it at runtime.

It seems that the Evaluation holds the initial 'from' type, but it
isn't
passed down to LazyModel when it is created (it only receives target
and
stack), so it has to try to discover the type by itself, and fails.

I'll try to do some experiments here...





On Thu, Jun 13, 2013 at 6:21 PM, Sven Meier <s...@meiers.net> wrote:

   Well, here again LazyModel needs the type of the bound target at

runtime.
Without any runtime information about the model's object type,
LazyModel
cannot derive the type of the evaluation result.

Sven


On 06/13/2013 10:55 PM, tetsuo wrote:

   this test also passes here, but

            assertEquals(B.class, ((IObjectClassAwareModel<B>)
model.bind(a)).getObjectClass(******));

            assertEquals(B.class, ((IObjectClassAwareModel<B>)
model.bind(Model.of(a))).******getObjectClass());




While the first assert passes, but the second one doesn't.

The exception is thrown not when getting the object, but the object
class
(the AbstractTextComponent class calls this before rendering and
while
converting input).





On Thu, Jun 13, 2013 at 5:40 PM, Sven Meier <s...@meiers.net> wrote:

    Strange, works fine here:

         @Test
        public void bindToModelAndGet() {
            LazyModel<B> model = model(from(A.class).getB());

            final A a = new A();
            a.b = new B();

            assertEquals(a.b, model.bind(Model.of(a)).******

getObject());


        }

Sven


On 06/13/2013 10:23 PM, tetsuo wrote:

    Thanks for the response!

  If I use an object instance, it works, but if I do as your third
example
(create a model from the class, then bind to a
non-IObjectClassAwareModel-********model),



it doesn't:

public class TestPage extends WebPage {

         private String text;

         public TestPage(final PageParameters parameters) {

             super(parameters);

             add(new Form<Void>("form")

                 .add(new TextField<String>("text",
model(from(TestPage.class
).getText()).bind(Model.of(********this)))));




         }

         public String getText() { return text; }

         public void setText(String text) { this.text = text; }

}


If I change 'Model.of(this)' to 'this' or an IObjectClassAwareModel
implementation, it works:


         class TestPageModel extends AbstractReadOnlyModel<******
TestPage>

             implements IObjectClassAwareModel<********TestPage> {




             public TestPage getObject() { return TestPage.this; }

             public Class<TestPage> getObjectClass() { return
TestPage.class;
}

         }


Is this a bug? I could create some wrapper models to make it work,
but
if
this is a bug, I'd prefer to wait for a corrected version.


Thanks again


On Thu, Jun 13, 2013 at 4:52 PM, Sven Meier <s...@meiers.net>
wrote:

     Hi,

   LazyModel needs to know the type of the model object to return an

appropriate proxy:

       model(from(a).getB()); // works
       model(from(aModel).getB()); // aModel must be an
IObjectClassAwareModel
       model(from(A.class).getB()).**********bind(aModel); //
works

even if

aModel


does not reveal its object class

Sven


On 06/13/2013 09:35 PM, tetsuo wrote:

     wait, wait, do you actually do something like

   new TextField<String>("name", new IModel<String>(){

          public String getObject() {
              return entity.getName();
          }
          public void setObject(String value) {
              entity.setName(value);
          }
          public void detach(){}
});

for every single field in your system, or you use LazyModel?

Well, I've been trying to use LazyModel, but with it I have to
pass
the
type of every field (the last arg of TextField's constructor)
because
if I
don't, this exception is thrown:

Caused by: java.lang.**********IllegalArgumentException: T is
not a
class or
parameterizedType

at org.wicketstuff.lazymodel.******
****reflect.Generics.getClass(****
Generics.java:78)

at org.wicketstuff.lazymodel.******
****LazyModel.getObjectType(**
LazyModel.java:139)

at org.wicketstuff.lazymodel.******
****LazyModel.getObjectClass(****
LazyModel.java:124)

at org.apache.wicket.markup.html.**********form.**
AbstractTextComponent.****
getModelType(
AbstractTextComponent.java:**********167)

at org.apache.wicket.markup.html.**********form.**
AbstractTextComponent.****
resolveType(
AbstractTextComponent.java:**********152)

at org.apache.wicket.markup.html.**********form.**
AbstractTextComponent.****
onBeforeRender(
AbstractTextComponent.java:**********142)





Any thoughts?

I guess I'll just go back to CompoundPropertyModel... (sigh)


(and no, I don't spend that much time debugging property models.
I
usually
don't rename properties that often, and when I have to do some
refactoring,
usually the structure changes, and I have to revise all pages
anyway...)




On Fri, May 31, 2013 at 10:49 AM, Martin Grigorov <
mgrigo...@apache.org

    wrote:

        On Fri, May 31, 2013 at 4:24 PM, tetsuo <
  ronald.tet...@gmail.com>
wrote:

       Black magic, or code generation? hard choice... :)

     I think I'll try the black magic, let's see how it goes :)
       I personally prefer writing the boilerplate of custom
Model
per
field.

    It is a boring work but it gives me:

  * type safety
* the fastest read/write access possible
* easier debugging

(who knows - maybe I've spent less time to write such models
than
you've
spent to debug your issues with property model after
refactoring)




     On Thu, May 30, 2013 at 8:16 PM, Igor Vaynberg <

   igor.vaynb...@gmail.com

     wrote:

   On Thu, May 30, 2013 at 12:37 PM, tetsuo <

ronald.tet...@gmail.com
     wrote:

      -1000!

   This will be horrible! Even with the current API, most
generics

I

      have
   to

     declare in my code don't add anything to type safety. For
example:

   while i am also not a fan of having component generified i do

believe
the example below is a bit contrived.

first, i hope most people do not use PropertyModels because
they
are
not compile-time-safe. there are plenty of project that
implement
compile-time-safe models, personally i prefer
https://github.com/42Lines/**********metagen<https://github.com/42Lines/********metagen>
<https://github.**com/42Lines/******metagen<https://github.com/42Lines/******metagen>
<https://github.com/**42Lines/******metagen<https://github.com/**42Lines/****metagen>
<https://github.**com/42Lines/****metagen<https://github.com/42Lines/****metagen>
<https://github.com/**42Lines/******metagen<https://github.com/**42Lines/****metagen>
<https://github.**com/**42Lines/**metagen<https://github.com/**42Lines/**metagen>
<https://github.com/**42Lines/****metagen<https://github.com/**42Lines/**metagen>
<https://github.com/**42Lines/**metagen<https://github.com/42Lines/**metagen>
<https://github.com/**42Lines/******metagen<https://github.com/**42Lines/****metagen>
<https://github.**com/**42Lines/**metagen<https://github.com/**42Lines/**metagen>
<https://github.com/******42Lines/metagen<https://github.com/****42Lines/metagen>
<https://**github.com/**42Lines/metagen<https://github.com/**42Lines/metagen>
<https://github.com/**42Lines/****metagen<https://github.com/**42Lines/**metagen>
<https://github.com/****42Lines/metagen<https://github.com/**42Lines/metagen>
<https://github.com/**42Lines/**metagen<https://github.com/**42Lines/metagen>
<https://github.com/**42Lines/metagen<https://github.com/42Lines/metagen>
to
using proxy-based solutions.

further, i hope even less people use compound property models.
they
are even more unsafe then property models and make your code
even
more
fragile. i would hate to refactor code that uses CPMs.

          add(new Form<Person>("form", new
CompoundPropertyModel<Person>(
****

     new

   PropertyModel<Person>(this, "person")))

            .add(new TextField<String>("name"))
            .add(new TextField<Integer>("age"))
            .add(new TextField<Double>("salary"))
            .add(new Button("save", new

     PropertyModel<Person>(this,"**********person")){

                public void onSubmit() {

       repository.save((Person)**********getForm().****


   getDefaultModelObject());

                }

             });
   In my experience, this kind of code is fairly common in
Wicket

applications. Every form component must be declared with a
type,
but

     none

   has *any* kind of type safety gain.

but how often do you declare a form component without adding
any
validators to it? the generic type of component also makes
sure
you
add the correct validator. for example it wont let you add a
validator
that expects strings to a component that produces integers.

also, not sure why you are replicating the model in Button.
first,
the
Button uses its model to fill its label; secondly, in real
code
the
model would be in a final var or field that things like
onsubmit
can
access easily.

-igor


      - The property model uses reflection, so its type can't
be
verified
by

    the

     compiler (this.person could be anything, not just a
Person).

  - Generics will guarantee that the form model will be of type
Person,

     but

   since it's all declared inline, and the real model isn't

verifiable,
it

    just adds lots of verbosity without any real gain.

     - Most form components use the implicit model, that also
uses

      reflection,
   and also can't verify the actual type of the underlying

property,
at

    compilation time. Even in runtime, *the type information is
lost

  due
     erasure

      *, so it can't use it to do any additional verification.

   *- Worse, you can even declare the "name" TextField as

<Integer>
or
<Double> (while maintaining the 'text' attribute as String),
and

     since

   there is no type information at runtime, it doesn't
matter. It

won't
    even

  throw an exception (it will just work normally).* In this
case,
the
type
declaration is simply a lie.

    Just pain, no gain. In my code, I sometimes just add a

      @SuppressWarnings(
   "rawtypes") to the class, and remove all useless generic
type

declarations.

     If everything will be required to declare them, I will
have
do
it

      more

   frequently.

    That said, repeater components benefit greatly from
generics.
So

  do
      custom

      models, validators, and converters. Or the rare cases
that

we

      explicitly

   declare the form component model. But forcing everything
to be

    generic-typed will just make Wicket extremely verbose to
use,

  with
     very

   little benefit.

   On Thu, May 30, 2013 at 4:00 AM, Martin Grigorov <
     mgrigo...@apache.org
   wrote:

       Hi,
  I just pushed some initial work for [1] and [2] in
branch generified-component-4930.
So far it doesn't look nice.

The added generics break somehow setMetaData/getMetaData
methods -

     you

   can

see compilation errors in Component and Page classes. I
think it
is

   caused

by the anonymous instance of MetaDataKey ( new
MetaDataKey<T>(type)
{}

    ).

  Also the visit*** methods do not compile at the moment, but
even
if

   we

    find

  a way to fix their signature I think writing a visitor will
become

   quite

    cumbersome.

       At the moment we have IVisitor
  and org.apache.wicket.util.**********iterator.****
AbstractHierarchyIterator
which

     do

   the

same job. The Iterator API is supposed to be simpler to write
for

   the

       users. Maybe we can drop  IVisitor ... ?!

      I'd like to ask for help with this task. It is supposed
to be
the

      biggest

API break for Wicket 7.0. My current feeling is that the end
result
won't
be very pleasant for the user-land code.

    For example the application code will have to do something
like:

         WebMarkupContainer<Void> wmc = new
WebMarkupContainer<>("id")

It is not that much but we have to decide whether we want
it.
But first let's try to fix the compilation problems.


1. https://issues.apache.org/******
****jira/browse/WICKET-4930<https://issues.apache.org/********jira/browse/WICKET-4930>
<ht**tps://issues.apache.org/********
jira/browse/WICKET-4930<https://issues.apache.org/******jira/browse/WICKET-4930>
<http**s://issues.apache.org/***
*****jira/browse/WICKET-4930<http://issues.apache.org/******jira/browse/WICKET-4930>
<h**ttps://issues.apache.org/******jira/browse/WICKET-4930<https://issues.apache.org/****jira/browse/WICKET-4930>
<https:**//issues.apache.org/***
***jira/**browse/WICKET-4930<http://issues.apache.org/****jira/**browse/WICKET-4930>
<h**ttp://issues.apache.org/****jira/**browse/WICKET-4930<http://issues.apache.org/**jira/**browse/WICKET-4930>
<htt**ps://issues.apache.org/****jira/**browse/WICKET-4930<http://issues.apache.org/**jira/**browse/WICKET-4930>
<htt**ps://issues.apache.org/**jira/**browse/WICKET-4930<https://issues.apache.org/**jira/browse/WICKET-4930>
<https:**//issues.apache.org/***
***jira/**browse/WICKET-4930<http://issues.apache.org/****jira/**browse/WICKET-4930>
<h**ttp://issues.apache.org/****jira/**browse/WICKET-4930<http://issues.apache.org/**jira/**browse/WICKET-4930>
<htt**p://issues.apache.org/**jira/****browse/WICKET-4930<http://issues.apache.org/jira/****browse/WICKET-4930>
<ht**tp://issues.apache.org/jira/****browse/WICKET-4930<http://issues.apache.org/jira/**browse/WICKET-4930>
<http**s://issues.apache.org/****jira/**browse/WICKET-4930<http://issues.apache.org/**jira/**browse/WICKET-4930>
<htt**p://issues.apache.org/jira/****browse/WICKET-4930<http://issues.apache.org/jira/**browse/WICKET-4930>
<http**s://issues.apache.org/**jira/**browse/WICKET-4930<http://issues.apache.org/jira/**browse/WICKET-4930>
<http**s://issues.apache.org/jira/**browse/WICKET-4930<https://issues.apache.org/jira/browse/WICKET-4930>
(Add
generics

     to

       o.a.w.Component)

2.
       https://cwiki.apache.org/*********<https://cwiki.apache.org/*******>

*confluence/display/WICKET/**<**
https://cwiki.apache.org/*******
*confluence/display/WICKET/**<https://cwiki.apache.org/******confluence/display/WICKET/**>
<**https://cwiki.apache.org/********<https://cwiki.apache.org/******>
confluence/display/WICKET/**<h**
ttps://cwiki.apache.org/******confluence/display/WICKET/**<https://cwiki.apache.org/****confluence/display/WICKET/**>
<h**ttps://cwiki.apache.org/******<http://cwiki.apache.org/****>
**confluence/display/WICKET/****<
http://cwiki.apache.org/******confluence/display/WICKET/**<http://cwiki.apache.org/****confluence/display/WICKET/**>
**<https://cwiki.apache.org/****
**confluence/display/WICKET/**<https://cwiki.apache.org/****confluence/display/WICKET/**>
**<https://cwiki.apache.org/****
confluence/display/WICKET/**<https://cwiki.apache.org/**confluence/display/WICKET/**>
    Wicket+7.0+Roadmap#Wicket7.*******
***0Roadmap-Genericsfororg.**

  apache.wicket.Component<https:********//
cwiki.apache.org/**

confluence/** 
<http://cwiki.apache.org/****confluence/**<http://cwiki.apache.org/**confluence/**>
<http://cwiki.**apache.org/confluence/**<http://cwiki.apache.org/confluence/**>
  display/WICKET/Wicket+7.0+********Roadmap#Wicket7.0Roadmap-**
Genericsfororg.apache.wicket.********Component<https://cwiki.**
**
apache.org/confluence/display/******WICKET/Wicket+7.0+Roadmap#*
***<http://apache.org/confluence/display/****WICKET/Wicket+7.0+Roadmap#**>
<http://apache.org/**confluence/display/**WICKET/**
Wicket+7.0+Roadmap#**<http://apache.org/confluence/display/**WICKET/Wicket+7.0+Roadmap#**>
Wicket7.0Roadmap-******Genericsfororg.apache.wicket.***
***Component<

https://cwiki.**apache.org/**confluence/display/**<http://apache.org/confluence/display/**>
WICKET/Wicket+7.0+Roadmap#****Wicket7.0Roadmap-**
Genericsfororg.apache.wicket.****Component<https://cwiki.**
apache.org/confluence/display/**WICKET/Wicket+7.0+Roadmap#**
Wicket7.0Roadmap-**Genericsfororg.apache.wicket.**Component<https://cwiki.apache.org/confluence/display/WICKET/Wicket+7.0+Roadmap#Wicket7.0Roadmap-Genericsfororg.apache.wicket.Component>