You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by "Dan Haywood (JIRA)" <ji...@apache.org> on 2015/03/02 22:13:04 UTC

[jira] [Updated] (ISIS-1063) View models with mandatory enum properties are not initialized correctly.

     [ https://issues.apache.org/jira/browse/ISIS-1063?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Dan Haywood updated ISIS-1063:
------------------------------
    Description: 
... because deep within Isis, in PersistenceSession#createViewModelInstance, there is some code that initializes all mandatory fields of a pojo just after an adapter has been created for it.

This has the effect of trampling the state on the pojo that was created by from the view model's memento.

here's one possible fix:

{code}
    public ObjectAdapter createViewModelInstance(final ObjectSpecification objectSpec, final String memento) {
        if (LOG.isDebugEnabled()) {
            LOG.debug("creating view model instance of " + objectSpec);
        }
        final Object pojo = objectSpec.createObject();

        final ViewModelFacet facet = objectSpec.getFacet(ViewModelFacet.class);

        //
        // the code below is horrid... we effectively initialize the view model pojo twice from the memento.
        // An alternative would be to not do the default initialize above, but that might change behaviour
        //

        // we initialize the pojo here in order to be able to build the OID
        facet.initialize(pojo, memento);

        final ObjectAdapter adapter = getAdapterManager().adapterFor(pojo);

        // next, (legacy behaviour... remove?) we initialize the pojo (set default values for any non-optional fields)
        // nb: this could have the effect of overwriting some of the state just set up
        objectSpec.initialize(adapter);

        // so finally we re-initialize, in order to reinstate any state that might have been overwritten.
        facet.initialize(pojo, memento);

        return adapter;
    }
{code}

However, perhaps we should simply not do the defaulting for view models at all, on the basis that it's the view model's own responsibility to set up its entire state.

So another possible fix is simply:

{code}
    public ObjectAdapter createViewModelInstance(final ObjectSpecification objectSpec, final String memento) {
        if (LOG.isDebugEnabled()) {
            LOG.debug("creating view model instance of " + objectSpec);
        }
        final Object pojo = objectSpec.createObject();

        final ViewModelFacet facet = objectSpec.getFacet(ViewModelFacet.class);
        facet.initialize(pojo, memento);

        final ObjectAdapter adapter = getAdapterManager().adapterFor(pojo);

        // no longer initialize the pojo for any default values 

        return adapter;
    }
{code}


  was:
... because deep within Isis there is some code that initializes all mandatory fields of a pojo just after an adapter has been created for it.

This has the effect of trampling the state on the pojo that was created by from the view model's memento.

here's one possible fix:

{code}
    public ObjectAdapter createViewModelInstance(final ObjectSpecification objectSpec, final String memento) {
        if (LOG.isDebugEnabled()) {
            LOG.debug("creating view model instance of " + objectSpec);
        }
        final Object pojo = objectSpec.createObject();

        final ViewModelFacet facet = objectSpec.getFacet(ViewModelFacet.class);

        //
        // the code below is horrid... we effectively initialize the view model pojo twice from the memento.
        // An alternative would be to not do the default initialize above, but that might change behaviour
        //

        // we initialize the pojo here in order to be able to build the OID
        facet.initialize(pojo, memento);

        final ObjectAdapter adapter = getAdapterManager().adapterFor(pojo);

        // next, (legacy behaviour... remove?) we initialize the pojo (set default values for any non-optional fields)
        // nb: this could have the effect of overwriting some of the state just set up
        objectSpec.initialize(adapter);

        // so finally we re-initialize, in order to reinstate any state that might have been overwritten.
        facet.initialize(pojo, memento);

        return adapter;
    }
{code}

However, perhaps we should simply not do the defaulting for view models at all, on the basis that it's the view model's own responsibility to set up its entire state.


> View models with mandatory enum properties are not initialized correctly.
> -------------------------------------------------------------------------
>
>                 Key: ISIS-1063
>                 URL: https://issues.apache.org/jira/browse/ISIS-1063
>             Project: Isis
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: core-1.8.0
>            Reporter: Dan Haywood
>            Assignee: Dan Haywood
>             Fix For: core-1.9.0
>
>
> ... because deep within Isis, in PersistenceSession#createViewModelInstance, there is some code that initializes all mandatory fields of a pojo just after an adapter has been created for it.
> This has the effect of trampling the state on the pojo that was created by from the view model's memento.
> here's one possible fix:
> {code}
>     public ObjectAdapter createViewModelInstance(final ObjectSpecification objectSpec, final String memento) {
>         if (LOG.isDebugEnabled()) {
>             LOG.debug("creating view model instance of " + objectSpec);
>         }
>         final Object pojo = objectSpec.createObject();
>         final ViewModelFacet facet = objectSpec.getFacet(ViewModelFacet.class);
>         //
>         // the code below is horrid... we effectively initialize the view model pojo twice from the memento.
>         // An alternative would be to not do the default initialize above, but that might change behaviour
>         //
>         // we initialize the pojo here in order to be able to build the OID
>         facet.initialize(pojo, memento);
>         final ObjectAdapter adapter = getAdapterManager().adapterFor(pojo);
>         // next, (legacy behaviour... remove?) we initialize the pojo (set default values for any non-optional fields)
>         // nb: this could have the effect of overwriting some of the state just set up
>         objectSpec.initialize(adapter);
>         // so finally we re-initialize, in order to reinstate any state that might have been overwritten.
>         facet.initialize(pojo, memento);
>         return adapter;
>     }
> {code}
> However, perhaps we should simply not do the defaulting for view models at all, on the basis that it's the view model's own responsibility to set up its entire state.
> So another possible fix is simply:
> {code}
>     public ObjectAdapter createViewModelInstance(final ObjectSpecification objectSpec, final String memento) {
>         if (LOG.isDebugEnabled()) {
>             LOG.debug("creating view model instance of " + objectSpec);
>         }
>         final Object pojo = objectSpec.createObject();
>         final ViewModelFacet facet = objectSpec.getFacet(ViewModelFacet.class);
>         facet.initialize(pojo, memento);
>         final ObjectAdapter adapter = getAdapterManager().adapterFor(pojo);
>         // no longer initialize the pojo for any default values 
>         return adapter;
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)