Provide context object as argument to `up` and `down`

[JamesMGreene]

I would like to see a context object (probably the whole migration object, honestly) provided as an argument to the invocation of up and down.

This can be beneficial for several reasons:

• To avoid copy/paste errors

  • The first Migration I wrote had a copy/paste error in the down function that made it asymmetrical with the up function: a query field value that was not updated.
  • This is easily solved by centralizing important shared values into a new data (or some other name) property to be added onto the Migration object [alongside the existing name/version/up/down properties], which would then be included as an argument (or sub-argument) to the actual invocations of the up and down functions.
  • Example:

  Migrations.add({
    name: "My Migration",
    version: 1,
    data: {
      mainDocId: "control"
    },
    up: function(migration) {
      SomeCollection.insert({ _id: migration.data.mainDocId, someValue: "important" });
    },
    down: function(migration) {
      SomeCollection.remove({ _id: migration.data.mainDocId });
    }
  });

• For logging purposes. Example:

  Migrations.add({
  name: "My Migration",
  version: 1,
  up: function(migration) {
    ExternalLoggingService.log("Running migration `up` to version: " + migration.version);
    // ...
  },
  down: function(migration) {
    ExternalLoggingService.log("Running migration `down` from version: " + migration.version);
    // ...
  }
  });

• For auditing purposes. Example:

  Migrations.add({
  name: "My Migration",
  version: 1,
  up: function(migration) {
    SomeCollection.insert({
      someValue: "important",
      created: {
        by: "Migrations[" + migration.version + "]",
        at: Date.now()
      }
    });
  },
  down: function(migration) {
    ExternalLoggingService.log("Running migration `down` from version: " + migration.version);
  }
  });

• For manual error handling purposes IF AND ONLY IF you know your up AND downare 100% symmetrically recoverable (although this is also a per-Migration option I'd like to see implemented separately in this package... but I'll open a separate issue for that). Example:

  Migrations.add({
  name: "My Migration",
  version: 1,
  up: function(migration, isRecovery) {
    try {
      SomeCollection.insert({ _id: "featureA", someValue: "important" });
    }
    catch (err) {
      if (!isRecovery) {  // avoid infinite loop
        try {
          migration.down(migration, true);
          throw new Error("Migrations[" + migration.version + "] up failed but was " +
            "successfully recovered.\n" + err);
        }
        catch (recoveryErr) {
          throw new Error("Migrations[" + migration.version + "] up failed AND failed to " +
            "recover!\n" + err + "\n" + recoveryErr);
        }
      }
    }
  },
  down: function(migration, isRecovery) {
    try {
      SomeCollection.remove({ _id: "featureA" });
    }
    catch (err) {
      if (!isRecovery) {  // avoid infinite loop
        try {
          migration.up(migration, true);
          throw new Error("Migrations[" + migration.version + "] down failed but was " +
            "successfully recovered.\n" + err);
        }
        catch (recoveryErr) {
          throw new Error("Migrations[" + migration.version + "] down failed AND failed to " +
            "recover!\n" + err + "\n" + recoveryErr);
        }
      }
    }
  }
  });

[zol]

Hey @JamesMGreene , thanks for your suggestion. In the interest of keeping this package as light as possible I'm going to refrain from making this change as it's reasonably trivial to work around it.

[JamesMGreene]

Just so we're on the same page, the change I'm proposing would just be:

Old

migration[direction].call();

New

migration[direction].call(null, migration);

OR:

migration[direction].call(migration, migration);

Not exactly heavy. :wink:

[zol]

Yeah ok, on second thoughts that's reasonable. I'd accept a PR, I think we can leave the parameter undocumented though.

[JamesMGreene]

@zol: PR #35 submitted.