[BUG]: Cannot find target migration ${target} in applied migrations

[brewencoded]

Describe the bug
I was trying to use the cli to execute a migration from version 0.0.3 to 0.0.1. I received the error above. After reviewing the code at https://github.com/sensedeep/onetable-cli/blob/main/src/cli.js#L338 I determined this was a bug in the logic within the if logic.

To Reproduce
Steps to reproduce the behavior:

I create a vitest and mocked out everything I could to isolate the issue. Jest is not friendly with "type": "module" projects.

// @ts-nocheck
import { Table } from "dynamodb-onetable";
import { CLI } from "./test.js";
import File from "./paks/js-file/index.js";

import { expect, it, vi } from "vitest";
import { DynamoDB } from "aws-sdk";
import { DocumentClient } from "aws-sdk/clients/dynamodb.js";

vi.mock("fs", () => {
  return {
    default: {
      existsSync: () => true,
    },
  };
});

vi.mock("aws-sdk", () => {
  return {
    default: {
      DynamoDB: {
        DocumentClient: vi.fn().mockReturnValue({
          invoke: vi.fn().mockReturnValue({
            promise: vi.fn().mockResolvedValue({
              StatusCode: 200,
              Payload: JSON.stringify({
                body: {},
              }),
            }),
          }),
        })
      }
    },
  };
});

vi.mock("onetable-migrate", () => {
  return {
      Migrate: vi.fn().mockReturnValue({
        init: vi.fn(),
        getPastMigrations: vi.fn().mockResolvedValue([
          {
            version: "0.0.3",
            status: "success",
            path: "memory",
          },
          {
            version: "0.0.2",
            status: "success",
            path: "memory",
          },
          {
            version: "0.0.1",
            status: "success",
            path: "memory",
          },
        ]), // all migrations have been successful to this point
        getCurrentVersion: vi.fn().mockResolvedValue("0.0.3"), // we are on 0.0.3
        getOutstandingVersions: vi.fn().mockResolvedValue()
      })
  };
});

it("should not throw when we try to move down multiple versions", async () => {
  process.argv = ["", "", "--config", "migrate.json", "0.0.1"];// target is 0.0.1

  vi.spyOn(File, "readJson").mockResolvedValue({
    arn: "",
    onetable: { name: "testtable" },
  });

  const cli = new CLI();
  await cli.init();
  await cli.command(); // The bug happens in CLI.run()
  // Cannot find target migration 0.0.1 in applied migrations
  // This happens because of !pastVersions.find((p) => p == target)
  // p == target is testing a string to an object. This will never be true.
  // The fix should be to change that to p.version === target
});

Expected behavior
I was expecting that the cli would execute 0.0.3 down and then 0.0.2 down migrations.

Environment (please complete the following information):

• Mac Ventura
• Node 20

[mobsense]

Can I ask a few questions:

Are you getting the error:

                    error(`Cannot find target migration ${target} in applied migrations`)

This test is meant to find your target version 0.0.1 in the list of past versions. Do you have a 0.0.1 in the past versions?

[brewencoded]

@mobsense I am getting that error. A quick clarification as well, the above test I took cli.js and removed all the main logic so I could test this properly. That is why I'm importing from "./test.js". This test mocks the response from getPastMigrations which is used:

// onetable-cli/blob/main/src/cli.js#L288
let pastMigrations = await this.migrate.getPastMigrations()
let pastVersions = pastMigrations.filter(m => Semver.valid(m.version))

This is used to populate pastVersions. I made sure the test resolves 0.0.1, 0.0.2, and 0.0.3, it still fails due to the line:

// onetable-cli/blob/main/src/cli.js#L338
if (target != '0.0.0' && !pastVersions.find(p => p == target)) {

Because target is a string and pastVersions is a Array<object> so p == target is effectively string == object.
I see the same behavior when I try to use 0.0.1 from 0.0.2:

• running onetable down works
• running onetable 0.0.1 throws Cannot find target migration 0.0.1 in applied migrations
• This happens even though running onetable list shows:

 Date                                Version   Description
 18:43:00 Nov 6, 2024                 0.0.1   Test migration 0.0.1

[mobsense]

Ah, thank you.

It should be:

                if (target != '0.0.0' && pastVersions.find(p => p == target).length == 0) {

Let me know if that works for you.

[brewencoded]

Looks like what will fix it is:

cmd = "down";
// pastVersions is `Record<string, any>[]` so we can get `p.version == target`
if (target != "0.0.0" && !pastVersions.find((p) => p.version == target).length === 0) {
  error(`Cannot find target migration ${target} in applied migrations`);
}
// We also need to use `v.version` here
versions = pastVersions
  .reverse()
  .filter((v) => Semver.compare(v.version, target) > 0);

We know pastVersions is is an array of objects because of this line:

let pastVersions = pastMigrations.filter(m => Semver.valid(m.version))

Could also fix this by doing:

let pastVersions = pastMigrations.map(m => m.version).filter(m => Semver.valid(m));
// still need to fix the if logic
if (target != "0.0.0" && !pastVersions.find((p) => p == target).length === 0) {
  error(`Cannot find target migration ${target} in applied migrations`);
}

[dev-embedthis]

Thanks, will test that.