path_alias record does not update on entity save.

Created on 10 February 2025, 3 months ago

Problem/Motivation

Continuation from πŸ› Update domain_id when field_domain_source is changed Fixed , the issue persists.

When saving content and re-assigning the Domain value, the record in the path_alias table does not update until saving it again. Clearing cache also does not have an effect, until you save it again. This value needs to be in sync, or there is a chance you could show the wrong content as a response.

πŸ› Bug report
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States kevinquillen

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @kevinquillen
  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen
  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen
  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen

    I think the issue may lie here:

          if ($node instanceof NodeInterface) {
            $langcode = \Drupal::languageManager()
              ->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();
            $langcode = $entity->get('langcode')->getString() ?? $langcode;
            $translation_languages = $node->getTranslationLanguages();
            if (in_array($langcode, array_keys($translation_languages))) {
              $domain_id = $helper->getDomainIdFromEntity($node);
            }
          }
    

    The path was already updated before this, but the presave hook fires again, and this code is hit. It gets the value from the original node, which sets it back to the old value. On the next save, without making any changes, the correct value is inserted.

  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen

    It almost seems like both the node and the path entity are updated and the latter overwrites the former. If I comment out the code in #4, it updates correctly on every save.

  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen

    As far as I can tell, since this happens in the presave hook that by the time the path alias logic runs the node it is attached to has not had its changes committed yet. So loading it from either the route or with the entityTypeManager is going to give you the previous Domain Access field value no matter what, until you save it again.

    This is a real issue because its hard to track this kind of bug. Ultimately, it means changing the Domain Access value on an entity will lag behind one save and causing pathing issues as content resolves to potentially the wrong domain, causing a 404 on another domain (where it should be).

    How can both cases effectively be handled without overwriting?

  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen

    So far removing the logic in presave that updates if the entity is a PathAlias resolves what I am seeing.

    I think that the case here is when the user is either updating a node or updating a Path entity directly. But theres a few things I don't quite get:

        // Get domains configuration.
        $options = \Drupal::entityTypeManager()
          ->getStorage('domain')
          ->loadOptionsList();
    
        // Get domain_id from get request param.
        $helper = \Drupal::service('domain_unique_path_alias.helper');
        $domain_id = $helper->getDomainIdByRequest();
    
        // Get domain_id from field_domain_source field.
        if (empty($domain_id) || !in_array($domain_id, $options)) {
          // Get field_domain_source data if a new translation.
          $node = \Drupal::routeMatch()->getParameter('node');
    

    If we are updating from the path form from admin/config/search/path/edit/{id} it seems like we should be getting the entity not from the route (because any content entity could have Path enabled) but try to extrapolate it from the path value, which is here:

          if (!$node instanceof NodeInterface) {
            $path = explode("/", $entity->getPath());
            if (
              isset($path[1], $path[2])
              && $path[1] === 'node'
              && is_numeric($path[2])
            ) {
              $node = Node::load($path[2]);
            }
          }
    

    Which is likely fine as it is, but I think to prevent the race condition this may have to move to a submit hook on the path edit form.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 148s
    #476071
  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen

    I've added an initial change here that tries to do two things:

    • - Eliminate the race condition in hook_entity_presave(). I tried using hook_entity_update, but I saw the same behavior(s) reported. With this change I cannot replicate the behavior.
    • - Update the path entity from the Node level on save.
    • - Move the other path entity update logic to a form submit on the path entity form.

    It's not exactly perfect because the path entity form has no awareness of the domain id (perhaps it could be a hidden value) so on submit I have to call entity save, which is likely happening more than once and could introduce new problems. This part would need more work, I believe - especially to expand support beyond nodes to any content entity with a path.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 177s
    #476089
  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen

    I've updated the change to incorporate what was there previously, except without loading the node from the route parameter. Using the helper to get the domain ID seems to work 100% of the time, but grabbing the node from the request or otherwise loads the unsaved node with the previous value.

    This now appears to work reliably on create/update of a node.

    However, I now notice that node edit links are incorrect (like tasks). I will make a new issue for that.

  • πŸ‡ΊπŸ‡ΈUnited States kevinquillen

    The additional saving of path alias in presave is also problematic if you are editing content for multiple Domains from one domain. At this level, I am not sure why the PathAlias handling is in a presave hook when it is explicitly being set otherwise.

  • Pipeline finished with Failed
    26 days ago
    Total: 145s
    #479348
Production build 0.71.5 2024