Problem/Motivation

I have a migration where certain rows are skipped. The migration is run with the --sync option so that rows which are removed from the source, get removed from the destination.

When a row which was skipped on import, gets removed from the source, the "drush migrate:import --sync" command throws a warning:
[warning] array_flip(): Can only flip STRING and INTEGER values! EntityStorageBase.php:312

Steps to reproduce

Install attached demo module.
Move the users.csv file to your /tmp/ directory.
Run: drush migrate:import --sync user_rollback_skipped_demo
Remove the last line from /tmp/users.csv (the one that got skipped).
Run again: drush migrate:import --sync user_rollback_skipped_demo

CommentFileSizeAuthor
#17 3260646-17.patch840 bytesuniquename
#7 interdiff_5-7.txt1.31 KBheddn
#7 3260646-7.patch1016 bytesheddn
#5 3260646-rollback-skipped-fix-5.patch814 bytespcate
#2 3260646-rollback-skipped-fix.patch878 byteserwin de vylder
migrate_tools_rollback_skipped_demo.zip1.8 KBerwin de vylder

Issue fork migrate_tools-3260646

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

    About issue forks

    Comments

    Erwin De Vylder created an issue. See original summary.

    erwin de vylder’s picture

    erwin de vylder commented
    StatusFileSize
    new 3260646-rollback-skipped-fix.patch878 bytes

    Patch attached. Hope this works.

    ras-ben’s picture

    ras-ben commented
    Status: Active » Reviewed & tested by the community

    Patch works for me :)

    heddn’s picture

    heddn
    Primary language English
    Location Illinois
    commented
    Status: Reviewed & tested by the community » Needs work 
    +++ b/src/EventSubscriber/MigrationImportSync.php
@@ -69,7 +69,7 @@ class MigrationImportSync implements EventSubscriberInterface {
+          if ($destination_ids) $destination->rollback($destination_ids);

    this isn't proper code style.

    pcate’s picture

    pcate commented
    Version: 8.x-5.1 » 6.0.x-dev
    Status: Needs work » Needs review
    StatusFileSize
    new 3260646-rollback-skipped-fix-5.patch814 bytes

    Updated patch to fix code formatting.

    solideogloria’s picture

    solideogloria commented
    Status: Needs review » Reviewed & tested by the community

    Confirmed the issue and the fix works for me.

    heddn’s picture

    heddn
    Primary language English
    Location Illinois
    commented
    Status: Reviewed & tested by the community » Needs review
    StatusFileSize
    new 3260646-7.patch1016 bytes
    new interdiff_5-7.txt1.31 KB

    I think this is a better fix as we don't want to trigger the various events or even deletes if we don't pull back destination IDs. Can someone confirm?

    solideogloria’s picture

    solideogloria commented
    Status: Needs review » Needs work

    #7 doesn't fix the original issue, as I get the array_flip(): Can only flip string and integer values warnings again.

    So it either needs improvement somehow, or #5 should be used.

    solideogloria’s picture

    solideogloria commented
    Title: Rolling back a skipped row (using --sync) results in: [warning] array_flip(): Can only flip STRING and INTEGER values! EntityStorageBase.php:312 » Rolling back a skipped row (using --sync) results in: [warning] array_flip(): Can only flip string and integer values! EntityStorageBase.php:312
    heddn’s picture

    heddn
    Primary language English
    Location Illinois
    commented

    Is the reason #7 didn't work because the return value isn't truly null?

    solideogloria’s picture

    solideogloria commented

    Yes. It returns an empty array in this case.

    Also, it should be noted that those functions say the return type is array, but the doc comments say it can return null.

      /**
   * Looks up the destination identifier currently being iterated.
   *
   * @return array
   *   The destination identifier values of the record, or NULL on failure.
   */
  public function currentDestination();

  /**
   * Looks up the source identifier(s) currently being iterated.
   *
   * @return array
   *   The source identifier values of the record, or NULL on failure.
   */
  public function currentSource();

    This in itself is a conflict:

       * @return array
   *   The source identifier values of the record, or NULL on failure.

    It should either have array|null in the comments and ?array as the return type hint, or it should remove the comment about it returning NULL if it never will.

    • heddn committed 0813c0f on 6.0.x 
      Issue #3260646 by Erwin De Vylder, heddn, PCate, solideogloria: Rolling...
    heddn’s picture

    heddn
    Primary language English
    Location Illinois
    commented
    Status: Needs work » Fixed

    This should fix it.

    Status: Fixed » Closed (fixed)

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

    juagarc4’s picture

    juagarc4 commented

    This issue seems not to be fixed.
    I still have the same warning and I installed the last dev version and I can't see the changes of the patch in the code.
    And the patch doesn't apply anymore.

    juagarc4’s picture

    juagarc4 commented

    Sorry I have already seen, that it was replaced by a new change afterwards.

    uniquename’s picture

    uniquename commented
    StatusFileSize
    new 3260646-17.patch840 bytes

    It seems that the changes got removed in https://www.drupal.org/project/migrate_tools/issues/3397550

    Find a patch attached that applies to 6.0.4

    solideogloria’s picture

    solideogloria commented

    @uniquename Please open a new issue. This issue is closed and might not get any visibility from the maintainer.

    uniquename’s picture

    uniquename commented

    Here is a follow up ticket. https://www.drupal.org/project/migrate_tools/issues/3432321