diff --git a/homeassistant/components/recorder/const.py b/homeassistant/components/recorder/const.py index c91845e8436..b7ee984558c 100644 --- a/homeassistant/components/recorder/const.py +++ b/homeassistant/components/recorder/const.py @@ -50,6 +50,11 @@ STATES_META_SCHEMA_VERSION = 38 LAST_REPORTED_SCHEMA_VERSION = 43 LEGACY_STATES_EVENT_ID_INDEX_SCHEMA_VERSION = 28 +LEGACY_STATES_EVENT_FOREIGN_KEYS_FIXED_SCHEMA_VERSION = 43 +# https://github.com/home-assistant/core/pull/120779 +# fixed the foreign keys in the states table but it did +# not bump the schema version which means only databases +# created with schema 44 and later do not need the rebuild. INTEGRATION_PLATFORM_COMPILE_STATISTICS = "compile_statistics" INTEGRATION_PLATFORM_LIST_STATISTIC_IDS = "list_statistic_ids" diff --git a/homeassistant/components/recorder/migration.py b/homeassistant/components/recorder/migration.py index c6cdd6d317f..3aa12f2b1f9 100644 --- a/homeassistant/components/recorder/migration.py +++ b/homeassistant/components/recorder/migration.py @@ -52,6 +52,7 @@ from .auto_repairs.statistics.schema import ( from .const import ( CONTEXT_ID_AS_BINARY_SCHEMA_VERSION, EVENT_TYPE_IDS_SCHEMA_VERSION, + LEGACY_STATES_EVENT_FOREIGN_KEYS_FIXED_SCHEMA_VERSION, LEGACY_STATES_EVENT_ID_INDEX_SCHEMA_VERSION, STATES_META_SCHEMA_VERSION, SupportedDialect, @@ -2490,9 +2491,10 @@ class BaseMigration(ABC): if self.initial_schema_version > self.max_initial_schema_version: _LOGGER.debug( "Data migration '%s' not needed, database created with version %s " - "after migrator was added", + "after migrator was added in version %s", self.migration_id, self.initial_schema_version, + self.max_initial_schema_version, ) return False if self.start_schema_version < self.required_schema_version: @@ -2868,7 +2870,14 @@ class EventIDPostMigration(BaseRunTimeMigration): """Migration to remove old event_id index from states.""" migration_id = "event_id_post_migration" - max_initial_schema_version = LEGACY_STATES_EVENT_ID_INDEX_SCHEMA_VERSION - 1 + # Note we don't subtract 1 from the max_initial_schema_version + # in this case because we need to run this migration on databases + # version >= 43 because the schema was not bumped when the table + # rebuild was added in + # https://github.com/home-assistant/core/pull/120779 + # which means its only safe to assume version 44 and later + # do not need the table rebuild + max_initial_schema_version = LEGACY_STATES_EVENT_FOREIGN_KEYS_FIXED_SCHEMA_VERSION task = MigrationTask migration_version = 2 diff --git a/tests/components/recorder/test_migration_from_schema_32.py b/tests/components/recorder/test_migration_from_schema_32.py index 0a5f5d4da73..012e227c11a 100644 --- a/tests/components/recorder/test_migration_from_schema_32.py +++ b/tests/components/recorder/test_migration_from_schema_32.py @@ -225,6 +225,7 @@ async def test_migrate_events_context_ids( patch.object(recorder, "db_schema", old_db_schema), patch.object(migration, "SCHEMA_VERSION", old_db_schema.SCHEMA_VERSION), patch.object(migration.EventsContextIDMigration, "migrate_data"), + patch.object(migration.EventIDPostMigration, "migrate_data"), patch(CREATE_ENGINE_TARGET, new=_create_engine_test), ): async with ( @@ -282,6 +283,7 @@ async def test_migrate_events_context_ids( patch( "sqlalchemy.schema.Index.create", autospec=True, wraps=Index.create ) as wrapped_idx_create, + patch.object(migration.EventIDPostMigration, "migrate_data"), ): async with async_test_recorder( hass, wait_recorder=False, wait_recorder_setup=False @@ -588,6 +590,7 @@ async def test_migrate_states_context_ids( patch.object(recorder, "db_schema", old_db_schema), patch.object(migration, "SCHEMA_VERSION", old_db_schema.SCHEMA_VERSION), patch.object(migration.StatesContextIDMigration, "migrate_data"), + patch.object(migration.EventIDPostMigration, "migrate_data"), patch(CREATE_ENGINE_TARGET, new=_create_engine_test), ): async with ( @@ -640,6 +643,7 @@ async def test_migrate_states_context_ids( patch( "sqlalchemy.schema.Index.create", autospec=True, wraps=Index.create ) as wrapped_idx_create, + patch.object(migration.EventIDPostMigration, "migrate_data"), ): async with async_test_recorder( hass, wait_recorder=False, wait_recorder_setup=False @@ -1127,6 +1131,7 @@ async def test_post_migrate_entity_ids( patch.object(migration, "SCHEMA_VERSION", old_db_schema.SCHEMA_VERSION), patch.object(migration.EntityIDMigration, "migrate_data"), patch.object(migration.EntityIDPostMigration, "migrate_data"), + patch.object(migration.EventIDPostMigration, "migrate_data"), patch(CREATE_ENGINE_TARGET, new=_create_engine_test), ): async with ( @@ -1158,9 +1163,12 @@ async def test_post_migrate_entity_ids( return {state.state: state.entity_id for state in states} # Run again with new schema, let migration run - with patch( - "sqlalchemy.schema.Index.create", autospec=True, wraps=Index.create - ) as wrapped_idx_create: + with ( + patch( + "sqlalchemy.schema.Index.create", autospec=True, wraps=Index.create + ) as wrapped_idx_create, + patch.object(migration.EventIDPostMigration, "migrate_data"), + ): async with ( async_test_home_assistant() as hass, async_test_recorder(hass) as instance, @@ -1169,7 +1177,6 @@ async def test_post_migrate_entity_ids( await hass.async_block_till_done() await async_wait_recording_done(hass) - await async_wait_recording_done(hass) states_by_state = await instance.async_add_executor_job( _fetch_migrated_states diff --git a/tests/components/recorder/test_migration_run_time_migrations_remember.py b/tests/components/recorder/test_migration_run_time_migrations_remember.py index 43a1b028348..350126b4c72 100644 --- a/tests/components/recorder/test_migration_run_time_migrations_remember.py +++ b/tests/components/recorder/test_migration_run_time_migrations_remember.py @@ -115,7 +115,7 @@ def _create_engine_test( "event_context_id_as_binary": (0, 1), "event_type_id_migration": (2, 1), "entity_id_migration": (2, 1), - "event_id_post_migration": (0, 0), + "event_id_post_migration": (1, 1), "entity_id_post_migration": (0, 1), }, [ @@ -131,7 +131,7 @@ def _create_engine_test( "event_context_id_as_binary": (0, 0), "event_type_id_migration": (2, 1), "entity_id_migration": (2, 1), - "event_id_post_migration": (0, 0), + "event_id_post_migration": (1, 1), "entity_id_post_migration": (0, 1), }, ["ix_states_entity_id_last_updated_ts"], @@ -143,13 +143,43 @@ def _create_engine_test( "event_context_id_as_binary": (0, 0), "event_type_id_migration": (0, 0), "entity_id_migration": (2, 1), - "event_id_post_migration": (0, 0), + "event_id_post_migration": (1, 1), "entity_id_post_migration": (0, 1), }, ["ix_states_entity_id_last_updated_ts"], ), ( 38, + { + "state_context_id_as_binary": (0, 0), + "event_context_id_as_binary": (0, 0), + "event_type_id_migration": (0, 0), + "entity_id_migration": (0, 0), + "event_id_post_migration": (1, 1), + "entity_id_post_migration": (0, 0), + }, + [], + ), + ( + 43, + { + "state_context_id_as_binary": (0, 0), + "event_context_id_as_binary": (0, 0), + "event_type_id_migration": (0, 0), + "entity_id_migration": (0, 0), + # Schema was not bumped when the SQLite + # table rebuild was implemented so we need + # run event_id_post_migration up until + # schema 44 since its the first one we can + # be sure has the foreign key constraint was removed + # via https://github.com/home-assistant/core/pull/120779 + "event_id_post_migration": (1, 1), + "entity_id_post_migration": (0, 0), + }, + [], + ), + ( + 44, { "state_context_id_as_binary": (0, 0), "event_context_id_as_binary": (0, 0), @@ -266,8 +296,14 @@ async def test_data_migrator_logic( # the expected number of times. for migrator, mock in migrator_mocks.items(): needs_migrate_calls, migrate_data_calls = expected_migrator_calls[migrator] - assert len(mock["needs_migrate"].mock_calls) == needs_migrate_calls - assert len(mock["migrate_data"].mock_calls) == migrate_data_calls + assert len(mock["needs_migrate"].mock_calls) == needs_migrate_calls, ( + f"Expected {migrator} needs_migrate to be called {needs_migrate_calls} times," + f" got {len(mock['needs_migrate'].mock_calls)}" + ) + assert len(mock["migrate_data"].mock_calls) == migrate_data_calls, ( + f"Expected {migrator} migrate_data to be called {migrate_data_calls} times, " + f"got {len(mock['migrate_data'].mock_calls)}" + ) @pytest.mark.parametrize("enable_migrate_state_context_ids", [True])