From 7089db06c25fb96a3be3a69f32e6a43014ec5449 Mon Sep 17 00:00:00 2001 From: Johnny Date: Mon, 19 Jan 2026 23:09:17 +0800 Subject: [PATCH] test: enhance memo filter tests with COALESCE for JSON extraction and add migration data persistence tests --- store/db/mysql/memo_filter_test.go | 26 ++--- store/test/instance_setting_test.go | 86 ++++++++--------- store/test/migrator_test.go | 145 ++++++++++++++++++++++++++++ store/test/store.go | 22 +++++ 4 files changed, 222 insertions(+), 57 deletions(-) diff --git a/store/db/mysql/memo_filter_test.go b/store/db/mysql/memo_filter_test.go index 020f2a28c..748a426fe 100644 --- a/store/db/mysql/memo_filter_test.go +++ b/store/db/mysql/memo_filter_test.go @@ -58,37 +58,37 @@ func TestConvertExprToSQL(t *testing.T) { }, { filter: `has_task_list`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList') = CAST('true' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList'), CAST('false' AS JSON)) = CAST('true' AS JSON)", args: []any{}, }, { filter: `has_task_list == true`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList') = CAST('true' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList'), CAST('false' AS JSON)) = CAST('true' AS JSON)", args: []any{}, }, { filter: `has_task_list != false`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList') != CAST('false' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList'), CAST('false' AS JSON)) != CAST('false' AS JSON)", args: []any{}, }, { filter: `has_task_list == false`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList') = CAST('false' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList'), CAST('false' AS JSON)) = CAST('false' AS JSON)", args: []any{}, }, { filter: `!has_task_list`, - want: "NOT (JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList') = CAST('true' AS JSON))", + want: "NOT (COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList'), CAST('false' AS JSON)) = CAST('true' AS JSON))", args: []any{}, }, { filter: `has_task_list && pinned`, - want: "(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList') = CAST('true' AS JSON) AND `memo`.`pinned` IS TRUE)", + want: "(COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList'), CAST('false' AS JSON)) = CAST('true' AS JSON) AND `memo`.`pinned` IS TRUE)", args: []any{}, }, { filter: `has_task_list && content.contains("todo")`, - want: "(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList') = CAST('true' AS JSON) AND `memo`.`content` LIKE ?)", + want: "(COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasTaskList'), CAST('false' AS JSON)) = CAST('true' AS JSON) AND `memo`.`content` LIKE ?)", args: []any{"%todo%"}, }, { @@ -118,32 +118,32 @@ func TestConvertExprToSQL(t *testing.T) { }, { filter: `has_link == true`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasLink') = CAST('true' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasLink'), CAST('false' AS JSON)) = CAST('true' AS JSON)", args: []any{}, }, { filter: `has_code == false`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasCode') = CAST('false' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasCode'), CAST('false' AS JSON)) = CAST('false' AS JSON)", args: []any{}, }, { filter: `has_incomplete_tasks != false`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasIncompleteTasks') != CAST('false' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasIncompleteTasks'), CAST('false' AS JSON)) != CAST('false' AS JSON)", args: []any{}, }, { filter: `has_link`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasLink') = CAST('true' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasLink'), CAST('false' AS JSON)) = CAST('true' AS JSON)", args: []any{}, }, { filter: `has_code`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasCode') = CAST('true' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasCode'), CAST('false' AS JSON)) = CAST('true' AS JSON)", args: []any{}, }, { filter: `has_incomplete_tasks`, - want: "JSON_EXTRACT(`memo`.`payload`, '$.property.hasIncompleteTasks') = CAST('true' AS JSON)", + want: "COALESCE(JSON_EXTRACT(`memo`.`payload`, '$.property.hasIncompleteTasks'), CAST('false' AS JSON)) = CAST('true' AS JSON)", args: []any{}, }, } diff --git a/store/test/instance_setting_test.go b/store/test/instance_setting_test.go index 12a2694a7..0f0c4cfb0 100644 --- a/store/test/instance_setting_test.go +++ b/store/test/instance_setting_test.go @@ -254,49 +254,47 @@ func TestInstanceSettingListAll(t *testing.T) { ts.Close() } - - func TestInstanceSettingEdgeCases(t *testing.T) { - t.Parallel() - ctx := context.Background() - ts := NewTestingStore(ctx, t) - - // Case 1: General Setting with special characters and Unicode - specialScript := `` - specialStyle := `body { font-family: "Noto Sans SC", sans-serif; content: "\u2764"; }` - _, err := ts.UpsertInstanceSetting(ctx, &storepb.InstanceSetting{ - Key: storepb.InstanceSettingKey_GENERAL, - Value: &storepb.InstanceSetting_GeneralSetting{ - GeneralSetting: &storepb.InstanceGeneralSetting{ - AdditionalScript: specialScript, - AdditionalStyle: specialStyle, - }, + +func TestInstanceSettingEdgeCases(t *testing.T) { + t.Parallel() + ctx := context.Background() + ts := NewTestingStore(ctx, t) + + // Case 1: General Setting with special characters and Unicode + specialScript := `` + specialStyle := `body { font-family: "Noto Sans SC", sans-serif; content: "\u2764"; }` + _, err := ts.UpsertInstanceSetting(ctx, &storepb.InstanceSetting{ + Key: storepb.InstanceSettingKey_GENERAL, + Value: &storepb.InstanceSetting_GeneralSetting{ + GeneralSetting: &storepb.InstanceGeneralSetting{ + AdditionalScript: specialScript, + AdditionalStyle: specialStyle, }, - }) - require.NoError(t, err) - - generalSetting, err := ts.GetInstanceGeneralSetting(ctx) - require.NoError(t, err) - require.Equal(t, specialScript, generalSetting.AdditionalScript) - require.Equal(t, specialStyle, generalSetting.AdditionalStyle) - - // Case 2: Memo Related Setting with Unicode reactions - unicodeReactions := []string{"🐱", "🐶", "🦊", "🦄"} - _, err = ts.UpsertInstanceSetting(ctx, &storepb.InstanceSetting{ - Key: storepb.InstanceSettingKey_MEMO_RELATED, - Value: &storepb.InstanceSetting_MemoRelatedSetting{ - MemoRelatedSetting: &storepb.InstanceMemoRelatedSetting{ - ContentLengthLimit: 1000, - Reactions: unicodeReactions, - }, + }, + }) + require.NoError(t, err) + + generalSetting, err := ts.GetInstanceGeneralSetting(ctx) + require.NoError(t, err) + require.Equal(t, specialScript, generalSetting.AdditionalScript) + require.Equal(t, specialStyle, generalSetting.AdditionalStyle) + + // Case 2: Memo Related Setting with Unicode reactions + unicodeReactions := []string{"🐱", "🐶", "🦊", "🦄"} + _, err = ts.UpsertInstanceSetting(ctx, &storepb.InstanceSetting{ + Key: storepb.InstanceSettingKey_MEMO_RELATED, + Value: &storepb.InstanceSetting_MemoRelatedSetting{ + MemoRelatedSetting: &storepb.InstanceMemoRelatedSetting{ + ContentLengthLimit: 1000, + Reactions: unicodeReactions, }, - }) - require.NoError(t, err) - - memoSetting, err := ts.GetInstanceMemoRelatedSetting(ctx) - require.NoError(t, err) - require.Equal(t, unicodeReactions, memoSetting.Reactions) - - ts.Close() - } - - + }, + }) + require.NoError(t, err) + + memoSetting, err := ts.GetInstanceMemoRelatedSetting(ctx) + require.NoError(t, err) + require.Equal(t, unicodeReactions, memoSetting.Reactions) + + ts.Close() +} diff --git a/store/test/migrator_test.go b/store/test/migrator_test.go index 513409bfd..23a152ed5 100644 --- a/store/test/migrator_test.go +++ b/store/test/migrator_test.go @@ -8,6 +8,8 @@ import ( "time" "github.com/stretchr/testify/require" + + "github.com/usememos/memos/store" ) // TestFreshInstall verifies that LATEST.sql applies correctly on a fresh database. @@ -31,6 +33,149 @@ func TestFreshInstall(t *testing.T) { require.Equal(t, currentSchemaVersion, instanceSetting.SchemaVersion) } +// TestMigrationDataPersistence verifies that data created in the old version +// is preserved and accessible after migration to the new version. +func TestMigrationDataPersistence(t *testing.T) { + t.Parallel() + + // Only run for SQLite for simplicity and speed in this edge case test, + // but the logic applies to all drivers. + if getDriverFromEnv() != "sqlite" { + t.Skip("skipping data persistence test for non-sqlite driver") + } + + ctx := context.Background() + dataDir := t.TempDir() + + // 1. Start Old Memos container (Stable) + oldCfg := MemosContainerConfig{ + Driver: "sqlite", + DataDir: dataDir, + Version: StableMemosVersion, + } + + t.Logf("Starting Memos %s container...", oldCfg.Version) + oldContainer, err := StartMemosContainer(ctx, oldCfg) + require.NoError(t, err, "failed to start old memos container") + + // Wait for startup + time.Sleep(5 * time.Second) + + err = oldContainer.Terminate(ctx) + require.NoError(t, err, "failed to stop old memos container") + + // 2. Start New Memos container (Local) - this triggers migration + newCfg := MemosContainerConfig{ + Driver: "sqlite", + DataDir: dataDir, + Version: "local", + } + + t.Log("Starting new Memos container to trigger migration...") + newContainer, err := StartMemosContainer(ctx, newCfg) + require.NoError(t, err, "failed to start new memos container") + defer newContainer.Terminate(ctx) + + // Wait for migration to complete + time.Sleep(5 * time.Second) + + // 3. Verify Data Access using Store + dsn := fmt.Sprintf("%s/memos_prod.db", dataDir) + + // Create a store instance connected to the migrated DB + ts := createTestingStoreWithDSN(t, "sqlite", dsn) + + // Check schema version + currentVersion, err := ts.GetCurrentSchemaVersion() + require.NoError(t, err) + require.NotEmpty(t, currentVersion, "schema version should be present") + t.Logf("Migrated schema version: %s", currentVersion) + + // Check if we can write new data + user, err := createTestingHostUser(ctx, ts) + require.NoError(t, err) + + memo, err := ts.CreateMemo(ctx, &store.Memo{ + UID: "migrated-test-memo", + CreatorID: user.ID, + Content: "Post-migration content", + Visibility: store.Public, + }) + require.NoError(t, err) + require.Equal(t, "Post-migration content", memo.Content) +} + +// TestMigrationIdempotency verifies that running the migration multiple times +// (e.g. container restart) is safe and doesn't corrupt data. +func TestMigrationIdempotency(t *testing.T) { + t.Parallel() + + if getDriverFromEnv() != "sqlite" { + t.Skip("skipping idempotency test for non-sqlite driver") + } + + ctx := context.Background() + dataDir := t.TempDir() + + // 1. Initial Migration (Local version) + cfg := MemosContainerConfig{ + Driver: "sqlite", + DataDir: dataDir, + Version: "local", + } + + t.Log("Run 1: Initial migration...") + container1, err := StartMemosContainer(ctx, cfg) + require.NoError(t, err) + time.Sleep(5 * time.Second) + container1.Terminate(ctx) + + // 2. Second Run (Restart) + t.Log("Run 2: Restart (should be idempotent)...") + container2, err := StartMemosContainer(ctx, cfg) + require.NoError(t, err) + defer container2.Terminate(ctx) + time.Sleep(5 * time.Second) + + // 3. Verify Store Integrity + dsn := fmt.Sprintf("%s/memos_prod.db", dataDir) + ts := createTestingStoreWithDSN(t, "sqlite", dsn) + + // Ensure we can still use the DB + _, err = ts.GetCurrentSchemaVersion() + require.NoError(t, err, "database should be healthy after restart") +} + +// TestMigrationReRun verifies that re-running the migration on an already +// migrated database does not fail or cause issues. This simulates a +// scenario where the server is restarted. +func TestMigrationReRun(t *testing.T) { + t.Parallel() + + ctx := context.Background() + // Use the shared testing store which already runs migrations on init + ts := NewTestingStore(ctx, t) + + // Get current version + initialVersion, err := ts.GetCurrentSchemaVersion() + require.NoError(t, err) + + // Manually trigger migration again + err = ts.Migrate(ctx) + require.NoError(t, err, "re-running migration should not fail") + + // Verify version hasn't changed (or at least is valid) + finalVersion, err := ts.GetCurrentSchemaVersion() + require.NoError(t, err) + require.Equal(t, initialVersion, finalVersion, "version should match after re-run") +} + +// createTestingStoreWithDSN helper to connect to an existing DB file. +func createTestingStoreWithDSN(t *testing.T, driver, dsn string) *store.Store { + ctx := context.Background() + return NewTestingStoreWithDSN(ctx, t, driver, dsn) +} + // testMigration is a helper function that orchestrates the migration test flow. // It starts the stable version, waits for initialization, and then starts the local version. func testMigration(t *testing.T, driver string, prepareFunc func() (MemosContainerConfig, func())) { diff --git a/store/test/store.go b/store/test/store.go index 3c7abf20a..194754894 100644 --- a/store/test/store.go +++ b/store/test/store.go @@ -37,6 +37,28 @@ func NewTestingStore(ctx context.Context, t *testing.T) *store.Store { return store } +// NewTestingStoreWithDSN creates a testing store connected to a specific DSN. +// This is useful for testing migrations on existing data. +func NewTestingStoreWithDSN(_ context.Context, t *testing.T, driver, dsn string) *store.Store { + profile := &profile.Profile{ + Mode: "prod", + Port: getUnusedPort(), + Data: t.TempDir(), // Dummy dir, DSN matters + DSN: dsn, + Driver: driver, + Version: version.GetCurrentVersion("prod"), + } + dbDriver, err := db.NewDBDriver(profile) + if err != nil { + t.Fatalf("failed to create db driver: %v", err) + } + + store := store.New(dbDriver, profile) + // Do not run Migrate() automatically, as we might be testing pre-migration state + // or want to run it manually. + return store +} + func getUnusedPort() int { // Get a random unused port listener, err := net.Listen("tcp", "localhost:0")