Two bugs I'm working on

[Adam Howard <ho...@gmail.com>]

Hi Dan,

I forked the Isis repo on github and I'm working on the two "hiding"
issues I had mentioned earlier [1].

The @NotInServicesMenu check won't work as I described. When I get
into the NotInServicesMenuFacetAnnotation.hides() method
ic.getTarget() is the PojoAdapter for the Repository, not for the
object who is receiving the contributed action. Example:

interface ProjectRepository {
  Project newProject();

  @NotInServicesMenu
  List<Project> projectFor(Contact contact);
}

When building the representation of the service this code works:

public class NotInServicesMenuFacetAnnotation ... {

  @Override
  public String hides(final VisibilityContext<? extends VisibilityEvent> ic) {
    if (ic.getTarget().getSpecification().isService()) {
      return "@NotInServiceMenu annotation present";
    }
    return null;
  }
}

The target is the repository which is a service so the action is hidden. Good.

But when building the representation for the Contact object there is a twist.

DomainObjectReprRenderer calls action.isVisible(session,
objectAdapter) for each action (including contributed.) objectAdapter
is the PojoAdapter for the Contact.

action.isVisible turns around and calls super.isVisible(session,
realTarget(objectAdapter)). It is this realTarget() call that changes
from operating on the Contact to the Repository. If it was left out
then I think the hides method above would correctly allow the action
to be visible on the Contact.

Can you give me an idea of why we need to get the real target in this
case? And why the service is the real target for a contributed action.

The fix for keeping @Hidden objects out of list representations (like
the list of services) was just as simple as I thought. So I'm ready to
commit that change (3 lines.) Are there any steps I should take before
committing, pushing to github and submitting a pull request? Build the
whole project? Write a test case? Anything special I should include in
the commit comment?

Thanks.
--
Adam

[1] http://mail-archives.apache.org/mod_mbox/incubator-isis-dev/201207.mbox/%3CCALJOYLEzOEnYHqx8JUK_ygoKN1-ET2Q%3DXnPrfy%3DGOiRdKJ%3DuqA%40mail.gmail.com%3E

Re: Two bugs I'm working on

[Dan Haywood <da...@haywood-associates.co.uk>]

On 9 July 2012 07:02, Adam Howard <ho...@gmail.com> wrote:

>
> I forked the Isis repo on github and I'm working on the two "hiding"
> issues I had mentioned earlier [1].
>
> The @NotInServicesMenu check won't work as I described. When I get
> into the NotInServicesMenuFacetAnnotation.hides() method
> ic.getTarget() is the PojoAdapter for the Repository, not for the
> object who is receiving the contributed action. Example:
>
> interface ProjectRepository {
>   Project newProject();
>
>   @NotInServicesMenu
>   List<Project> projectFor(Contact contact);
> }
>
> When building the representation of the service this code works:
>
> public class NotInServicesMenuFacetAnnotation ... {
>
>   @Override
>   public String hides(final VisibilityContext<? extends VisibilityEvent>
> ic) {
>     if (ic.getTarget().getSpecification().isService()) {
>       return "@NotInServiceMenu annotation present";
>     }
>     return null;
>   }
> }
>
> The target is the repository which is a service so the action is hidden.
> Good.
>
> But when building the representation for the Contact object there is a
> twist.
>
> DomainObjectReprRenderer calls action.isVisible(session,
> objectAdapter) for each action (including contributed.) objectAdapter
> is the PojoAdapter for the Contact.
>
> action.isVisible turns around and calls super.isVisible(session,
> realTarget(objectAdapter)). It is this realTarget() call that changes
> from operating on the Contact to the Repository. If it was left out
> then I think the hides method above would correctly allow the action
> to be visible on the Contact.
>
> Can you give me an idea of why we need to get the real target in this
> case? And why the service is the real target for a contributed action.
>

Huh.  Sorry, then, cos I've given you a bum steer.

The reason for this realTarget() thing is because at least some of the
facets involve an interaction with a method on the contributing service.
 For example, the validate method is like this.  eg,


class ProjectRepositoryImpl {
  Project newProject();

  List<Project> projectFor(Contact contact);
  String validateProjectFor(Contact contact) {
    return contact.isBlacklisted()? "Blacklisted contacts can't have
projects": null;
  }
}

In this case, the ActionValidationFacetViaMethod facet needs to be invoked
on the repo instance, not the contact (because the underlying method is on
the repo).

However, I can see you arguing, probably correctly, that in this case there
is no method to be invoked, and so the facet ought to be invoked against
the contributee, not the contributor.

I can't see an immediate solution for this, other than to extend the Facet
API (or maybe, more narrowly, the *InteractionAdvisor APIs) in some way in
order that the facet impl can indicate the means by which it should be
evaluated.

~~~
Going back to the original requirement, though, maybe this can be solved
differently.  In the RO viewer currently uses the same
DomainObjectReprRenderer for both the /objects URLs and for the /services
URLs.  Perhaps another - slightly clunkier - solution is to have a
DomainServiceReprRenderer, which has slightly different logic for the list
of actions; ie it honours the @NotInServiceMenu.

Hmm... having written that down, I think prefer the metamodel solution
though (rather than hacking it in a viewer).



>
> The fix for keeping @Hidden objects out of list representations (like
> the list of services) was just as simple as I thought. So I'm ready to
> commit that change (3 lines.) Are there any steps I should take before
> committing, pushing to github and submitting a pull request? Build the
> whole project? Write a test case? Anything special I should include in
> the commit comment?
>

Good stuff.

Yes, do build the whole project, just to make sure nothing unexpected has
broken.

If you can add a couple of unit tests also, that'd be appreciated.

I usually run up a simple app as well as a sanity check.

In terms of commit comment, just reference the appropriate JIRA.  For this
work, I think it is ISIS-232, so a comment that starts with this should be
fine.


Cheers
Dan




>
> Thanks.
> --
> Adam
>
> [1]
> http://mail-archives.apache.org/mod_mbox/incubator-isis-dev/201207.mbox/%3CCALJOYLEzOEnYHqx8JUK_ygoKN1-ET2Q%3DXnPrfy%3DGOiRdKJ%3DuqA%40mail.gmail.com%3E
>