loadUnchanged loads default revision, not latest revision

Problem/Motivation

As discovered by acbramley in #2700197: Transition Event $state_before is not correct when transitioning a non default revision for Workbench Moderation, $entity->original (as found in hook_entity_presave and similar) is not, in fact, the original entity. It is the current default revision. In core, under normal circumstances, that is always the same as the original entity. However, there is nothing that guarantees it is. In fact, if you install Workbench Moderation (or any other module that makes use of forward revisions), it will very often NOT be the case. That results in $entity->original being wrong in many cases.

On further investigation, I believe the issue is in EntityStorageBase::loadUnchanged(), which does a fresh load from the database(!) using ::load(), which by design looks for the default revision only. What we need instead is a loadLatestRevision() method, which will load the latest revision regardless of what it's default status is.

Proposed resolution

In Workbench Moderation, I have this routine for doing so:

  public function getLatestRevision($entity_type_id, $entity_id) {
    if ($latest_revision_id = $this->getLatestRevisionId($entity_type_id, $entity_id)) {
      return $this->entityTypeManager->getStorage($entity_type_id)->loadRevision($latest_revision_id);
    }
  }

  public function getLatestRevisionId($entity_type_id, $entity_id) {
    if ($storage = $this->entityTypeManager->getStorage($entity_type_id)) {
      $revision_ids = $storage->getQuery()
        ->allRevisions()
        ->condition($this->entityTypeManager->getDefinition($entity_type_id)->getKey('id'), $entity_id)
        ->sort($this->entityTypeManager->getDefinition($entity_type_id)->getKey('revision'), 'DESC')
        ->range(0, 1)
        ->execute();
      if ($revision_ids) {
        $revision_id = array_keys($revision_ids)[0];
        return $revision_id;
      }
    }
  }

Which I don't really like, but it's the best I could do from the outside. We could move that logic, more or less, into EntityStorageBase, and then use getLatestRevision() instead of loadUnchanged() (which maybe should be deprecated, per #2226037: Change signature of Entity::loadUnchanged() to accept an EntityInterface instead of random ID). I'm not sure what the entity cache implications are of this approach, however.

Alternatively, I'm open to suggestions for alternate implementations from the Entity API maintainers. :-) As long as in the end we end up with a useful getLatestRevision() method, which we then call from EntityStorageBase::doPresSave() and others, that would fix the bug as well as let me remove some now-redundant code from Workbench Moderation, which would be a nice bonus.

Remaining tasks

Confirm that the above is the best approach, then implement it. (I can do the latter if a maintainer can do the former.)

User interface changes

None.

API changes

New API method, maybe.