Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions config/install/migrate_plus.migration.ead_media_to_node.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ source:
- 'http://vocab.getty.edu/page/aat/300026539'
Prefix_date_bulk: 'Bulk: '
plugin: media_xml_data
high_water_property:
name: file_changed
alias: fc
track_changes: true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will this addition impact existing migrated items?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked with Claude to consult how drupal track_changes works:

track_changes is a property read by Drupal\migrate\Plugin\migrate\source\SourcePluginBase. When set to true:

For each source row, Migrate computes a hash of the row's data. This hash is stored in the map table's hash column.
On subsequent runs, the newly computed hash for each row is compared against the stored hash.
If they differ, the row is flagged as needing update (MigrateIdMapInterface::STATUS_NEEDS_UPDATE) even if it was already STATUS_IMPORTED, so it gets reprocessed.
If they match, the row is skipped as unchanged.

while the tradeoff or impact I think will be for a very large dataset, it can slow down migration run due to hash.

I also asked Claude in case if we don't use any mechanism to detect change:

Without any mechanism, drupal migration will simply check the map table for the row's source MediaID. If no matching row exists in the map table, it's treated as new and gets imported; while if a matching row exists and its status is STATUS_IMPORTED, the row is skipped, no matter file change in the source (in our case file_changed value).

I think it will be better we add the track_changes property for a clean incremental migration. but there's a performance tradeoff as mentioned above. I am open to your thought.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran this through Claude as well, as:

In Drupal Migrate can I use a custom hashing mechanism for the track_changes source option? I have a custom source plugin which will read files and extract contents into source fields, but I really only need to compute the content hash on the files' properties, like size and modification time. Fetching the data for every file every time to check for changes would be too operationally expensive.

The response was essentially:

  • Don't use track_changes; the hash mechanism isn't customizable
  • Short circuit the expensive work in prepareRow() with a return FALSE; when there is nothing to do

So:

  /**
   * {@inheritdoc}
   */
  protected function initializeIterator() {
    $this->filesList = $this->listMediaXmlFiles();
    return new \ArrayIterator($this->filesList);
  }
  /**
   * prepare each source row after initializeIterator() and trigger POST_RAW_SAVE
   * {@inheritdoc}
   */
  public function prepareRow(Row $row) {
    foreach ($files as $file) {
        $row->setSourceProperty('file_modified', $file->getChangedTime());
        $row->setSourceProperty('file_uri', $file->getFileUri());
        $row->rehash();
          // Compare against what's stored for this row already.
          $map_row = $this->migration->getIdMap()->getRowBySource($row->getSourceIdValues());
          if ($map_row && $map_row['hash'] === $row->getHash()) {
            // Nothing changed on disk — skip without ever reading file contents.
            return FALSE;
          }
    }
    // New or change files
    foreach ($this->parseXmlFile($file) as $property) {
        $row->setSourceProperty($property->name, $property->value);
    }
    return parent::prepareRow($row);
  }

namespaces:
ead: 'urn:isbn:1-931666-22-9'
xlink: 'http://www.w3.org/1999/xlink'
Expand Down
55 changes: 0 additions & 55 deletions src/Plugin/migrate/source/MediaXmlData.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ class MediaXmlData extends SourcePluginBase implements ContainerFactoryPluginInt
* @var array
*/
protected $parsedData = [];
protected string $eadXmlDir;
const SAVE_BASE_URI = 'private://findingaid';

/**
* {@inheritdoc}
Expand All @@ -49,15 +47,6 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
parent::__construct($configuration, $plugin_id, $plugin_definition, $migration);
$this->entityTypeManager = $entity_type_manager;
$this->fileSystem = $file_system;

//handle field uri scheme
$fieldConfig = \Drupal::entityTypeManager()
->getStorage('field_config')
->load("media.findingaid.field_media_file");

$f_uri_scheme = ($fieldConfig === null) ? 'private' : ($fieldConfig->getSetting('uri_scheme') ?? 'private');
$f_sub_dir = ($fieldConfig === null) ? 'findingaid' : ($fieldConfig->getSetting('file_directory') ?? 'findingaid');
$this->eadXmlDir = $f_uri_scheme . '://' . trim($f_sub_dir, '/');
}

/**
Expand Down Expand Up @@ -152,13 +141,6 @@ protected function parseMediaXmlFiles() {
//$file_path = $this->fileSystem->realpath($file_uri);
$file_changed = $file->getChangedTime();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$file_changed looks abandoned here. It is passed to parseXmlFile() but unused there.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_changed is still referred in parseXmlFile() to capture the value in the source data row (...// Process each item
foreach ($items as $index => $item) {
$row_data = [
'media_id' => $media_id,
'file_changed' => $file_changed,
];
....
). It is necessary to include for checksum of the source row array as a row hash in the migration map table to detect a file change.


// Determine if we should process this media
$should_process = $this->shouldProcessMedia($media, $file_changed);

if (!$should_process) {
continue;
}

// Check if it's an XML file
$mime_type = $file->getMimeType();
if (in_array($mime_type, ['application/xml', 'text/xml']) ||
Expand All @@ -175,43 +157,6 @@ protected function parseMediaXmlFiles() {
}
}

/**
* Determine if a media entity should be processed.
*
* @param \Drupal\media\Entity\Media $media: media entity
* @param int $file_changed: file changed timestamp.
*
* @return bool: TRUE if should process, FALSE otherwise.
*/
protected function shouldProcessMedia($media, $file_changed) {
// Check if media has an associated node in field_media_of
if ($media->hasField('field_media_of') && !$media->get('field_media_of')->isEmpty()) {
$node_id = $media->get('field_media_of')->target_id;
$node_storage = $this->entityTypeManager->getStorage('node');
$node = $node_storage->load($node_id);

if ($node) {
$node_changed = $node->getChangedTime();

// Process if file timestamp is greater than node timestamp
// This will trigger a reimport and override the existing node
if ($file_changed > $node_changed) {
return TRUE;
}

// File hasn't changed since node was last updated, skip
return FALSE;
}

// Referenced node doesn't exist, should process
return TRUE;
}

// No associated node (field_media_of is empty)
// Always process to create new node
return TRUE;
}

/**
* Parse an XML file and extract data based on configuration.
*
Expand Down