Upgrade Path to resolve raw output

I've made another update.
- a bit cleaner,
- very fast,
- covers all multiline text fields and db revisions
- does one database table per pass (not tested on that)
- converts settings to json instead of manipulating strings
Shouldn't do any harm, at least.

<?php

/**
 * @file
 * Install file for Video Embed WYSIWYG.
 */
const VIDEO_EMBED_WYSIWYG_UPDATE_REGEX = '\{"preview_thumbnail"[^}]*"video_url"[^}]*"settings":\{[^}]*\}[^}]*\}';

/**
 * Add title_format setting to video embed settings.
 */
function video_embed_wysiwyg_update_10001(&$sandbox) {
  $database = Drupal::database();
  if (!isset($sandbox['embed_node_tables'])) {
    foreach (\Drupal::service('entity_field.manager')->getFieldMap() as $entity_type => $fields) {
      foreach ($fields as $field_name => $field_info) {
        if ($field_info['type'] == 'text_long' or $field_info['type'] == 'text_with_summary') {
          $sandbox['embed_node_tables'][$entity_type.'__'.$field_name] = 0;
          $sandbox['embed_node_tables'][$entity_type.'_revision__'.$field_name] = 0;
          $sandbox['finished'] = 0;
        }
      }
    }
  }
  $table_name = key($sandbox['embed_node_tables']);
  // Do one table each pass.
  video_embed_wysiwyg_update_text($table_name, $sandbox['embed_node_tables']);
}

/**
 * Updates up to 999 fields. Very fast.
 *
 * @param string $table_name
 * @param int $last_updated_id
 *   updated by reference
 */
function video_embed_wysiwyg_update_text($table_name, array &$tables) {
  $last_updated_id = &$tables[$table_name];
  $col = substr($table_name, strpos($table_name, '__') + 2) . '_value';
  $cols = ['entity_id', $col];
  if (str_contains($table_name, '_revision__')) {
    $cols[] = 'vid';
  }
  $field_values = $database->select($table_name, 't')
    ->fields('t', )
    ->where($col . ' REGEXP :pattern', [':pattern' => VIDEO_EMBED_WYSIWYG_UPDATE_REGEX])
    ->condition('t.entity_id', $last_updated_id, '>')
    ->orderby('entity_id', 'DESC')
    ->execute()->fetchAllAssoc('entity_id');

  $count = 0;
  while ($res = array_pop($field_values)) {
    $count++;
    $id = $res->entity_id;
    $orig_text = $res->{$col};
    $replace_text = $orig_text;
    if (preg_match_all('/'.$regex_pattern.'/', $orig_text, $matches)) {
      foreach ($matches[0] as $match) {
        $embed_data = json_decode($match, TRUE);
        $embed_data['settings'] += ['title_fallback' => TRUE, 'title_format' => '@provider | @title'];
        $replace_text = str_replace($match, json_encode($embed_data, JSON_UNESCAPED_SLASHES), $replace_text);
      }
    }
    if ($replace_text and strlen($replace_text) > strlen($orig_text)) {
      $q= $database->update($table_name)
        ->fields([$col => $replace_text]);
      if (str_contains($table_name, '_revision__')) {
        $q->condition('vid', $last_updated_id = $res->vid);
      }
      else {
        $q->condition('entity_id', $last_updated_id = $res->entity_id);
      }
      $q->execute();
    }
    $tables[$table_name] = $last_updated_id;
    if ($count > 999) break;
  }
  if (empty($field_values)) {
    unset($tables[$table_name]);
    $sandbox['finished'] = empty($tables);
  }
}

Shouldn't the rendering code also be fixed to avoid rendering raw content? 🤔

can you say a bit more about that?

Can't we just use some default value when the new configuration is missing?

@mably That approach also seems valid, and would require less upgrade code, but its also cleaner in the long term to update the old data so that all the data managed by the module is consistent. So its up to the maintenance team to decide.

I've just hit this and only incidentally found this issue. I'm going to dive into testing it, but thought I'd update the Title to something that would have jumped out to me as relating to my issue.

MR!84 is looking good to me.

While the reworking at #3 may be less code, MR!84 is very readable, doesn't introduce extra constants or functions outside of the update hook. It is also working in it's current state. If this code was going to be used repeatedly the proposed optimisation would be worth exploring, but with it being a one-shot update, I don't see the need.

MR!84, as it stands now, gets my RTBC.

Ok, let's merge this and see how it goes.

This patch doesn't seem to work with translations. We installed and ran the DB updates, and whilst it did correctly restore all the videos, it converted all fields with videos into the site's default language despite those fields being translated before running the DB update.

Here's an updated version of the MR84 patch - specifically, an extra condition at line 141 of .install to take into account the langcode.

@jwmorris previous patch has already been merged.

Could you provide an MR against the 3.x branch with your fix please?

@jwmorris I just created MR 86 to include the diff from your patch.

Can you confirm that it fixes the problem ?

@matslats could you have a look at MR 86 too, please?

Wondering if the langcode field always exists...