Opened 7 years ago

Closed 7 years ago

#1961 closed defect (fixed)

Implementation of getAnnotatableParents() is flawed

Reported by: Nicklas Nordborg Owned by: Nicklas Nordborg
Priority: major Milestone: BASE 3.7
Component: core Version:
Keywords: Cc:

Description

In many classes the implementation of the Annotatable.getAnnotatableParents() is flawed. It may for example look like this (from the RawBioAssay class):

public Set<Annotatable> getAnnotatableParents()
  throws BaseException
{
  Set<Annotatable> annotatable = new HashSet<Annotatable>();
  try
  {
    if (getData().getArrayDesign() != null) annotatable.add(getArrayDesign());
    if (getData().getParentBioAssay() != null) annotatable.add(getParentBioAssay());
    if (getData().getParentExtract() != null) annotatable.add(getParentExtract());
    if (getData().getSoftware() != null) annotatable.add(getSoftware());
  }
  catch (PermissionDeniedException ex)
  {}
  return annotatable;
}

Depending on the permissions for the logged in user, the returned Set may not include all items that the user has access to. For example, if the current user doesn't have access to the array design, then the code will never try to load the parent bioassay, extract or software even if the user may have access to those items.

The code example is typical and the implementation is similar in most other classes. The implementation could for example be fixed by adding a try/catch around each line, but this creates messy code. A better approach is probably to implement some kind of utility function.

Change History (3)

comment:1 Changed 7 years ago by Nicklas Nordborg

Owner: changed from everyone to Nicklas Nordborg
Status: newassigned

comment:2 Changed 7 years ago by Nicklas Nordborg

(In [7002]) References #1961: Implementation of getAnnotatableParents() is flawed

Added utility function BasicItem.addAnnotatableParents() which should handle PermissionDeniedException much better. All getAnnotatableParents() methods have been updated to use the utility method in all cases which are sensitive to permission issues.

comment:3 Changed 7 years ago by Nicklas Nordborg

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.