In some cases it is possible that, when preparing to generate activities a fatal error can occur. This is usually not noticeable for the user as it is most likely something that is happening when processing a queue item.

In the ActivityLoggerFactory.php the code assumes the owner of the entity is available, this is however not always the case. Next to that getOwner() method does not always exist for an entity.

Comments

bramtenhove created an issue. See original summary.

bramtenhove’s picture

Status: Active » Needs review
jaapjan’s picture

Status: Needs review » Fixed
kingdutch’s picture

Assigned: Unassigned » kingdutch
Status: Fixed » Needs work

I'm moving this back to needs work. We should encourage the use of Interfaces (this is exactly what they're for) instead of checking whether the method exists.

Performance wise the difference is negligible (instanceof is even slightly faster). However, instanceof more clearly communicates intent because we're looking to use the getOwner function in the way that it's defined in that interface. When using method_exists, if someone were to implement getOwner but not return an instance of UserInterface then our code would still fall apart.

kingdutch’s picture

Assigned: kingdutch » Unassigned
Status: Needs work » Needs review

  • jaapjan committed b2ad294 on 8.x-5.x
    Issue #3045199 by bramtenhove, jaapjan: Fix possible fatal error can...

  • 11dda01 committed on 8.x-4.x
    Issue #3045199 by bramtenhove, jaapjan: Fix possible fatal error can...
jaapjan’s picture

Status: Needs review » Fixed

Kingdutch, like discussed let's move the last PR to 5.0 and to a separate issue.

  • Kingdutch committed 54d52d6 on 8.x-5.x
    Issue #3045199: Implement EntityOwnerInterface for MentionsInterface...
  • Kingdutch committed b73afc3 on 8.x-5.x
    Issue #3045199 by bramtenhove, jaapjan, Kingdutch: prefer instanceof...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.