Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IndexedDBCache getInverseRelationshipsAsync #823

Merged
merged 2 commits into from Mar 6, 2021
Merged

Conversation

dgeb
Copy link
Member

@dgeb dgeb commented Mar 6, 2021

馃挜 Important: Action is needed to update pre-existing @orbit/indexeddb databases. See below. 馃挜

This PR fixes an issue with IndexedDBCache#getInverseRelationshipsAsync in which the wrong index was being used to lookup items in the object store that maintains inverse relationships (__inverseRels__). This issue may have prevented the AsyncCacheIntegrityProcessor from properly removing relationships when records were removed. Therefore, it's possible that your cache may contain extra "dead" relationships (i.e. relationships to records that have been removed).

This PR adds the missing index, relatedIdentity, to the object store __inverseRels__ when it is created. Therefore, any new IndexedDB databases should now properly track inverse relationships.

Has this bug affected my data?

This bug appears to only affect the integrity of relationships which do not have matching inverses explicitly declared.

In the following schema, inverse relationships are explicitly declared:

planet: {
  relationships: {
    moons: { kind: 'hasMany', type: 'moon', inverse: 'planet' }
  }
},
moon: {
  relationships: {
    planet: { kind: 'hasOne', type: 'planet', inverse: 'moons' }
  }
}

The integrity of planet.moons and moon.planet will have been maintained when either a moon or planet is removed. If all your relationships have explicit inverses declared like this, then your IndexedDB data should not be affected by this bug.

However, in the following scenario, the relationship is one-sided, without an inverse:

planet: {
  relationships: {
    moons: { kind: 'hasMany', type: 'moon' }
  }
},
moon: {
  relationships: {
    // None
  }
}

In this case, deleting a moon will not have resulted in the moon being cleared from any planets that contained that moon in their moons relationship. You may wish to validate your relationship data as part of the following steps.

Recreating / migrating pre-existing databases

If your application has any pre-existing IndexedDB databases "in the wild", you'll need to take steps to update them. The steps you take really depend why / how you're using this source in your app. If you're just using it as a temporary offline buffer, you may choose to just drop and recreate the database. However, if your IndexedDB source is the "source of truth" for some of your application's data, you'll want to upgrade the database and validate its data if it may have been affected (see above).

In order to perform an upgrade, take the following steps:

  1. Increase the version of your Schema.
  2. Override the migrateDB method for your cache instance. If event.newVersion equals your new schema version, modify the __inverseRels__ object store to add the relatedIdentity index.
  3. Optionally validate your data relationships (see above) and the contents of __inverseRels__ within migrateDB, pruning any references to non-existent records.
  4. Test everything carefully of course.

For example, let's say that you set the new schema version to 2. Override migrateDB as follows:

source.cache.migrateDB = (db, event) => {
  if (event.newVersion === 2) {
    const transaction = event.target.transaction;
    const objectStore = transaction.objectStore('__inverseRels__');

    // Add missing `relatedIdentity` index. This is required.
    objectStore.createIndex('relatedIdentity', 'relatedIdentity', {
      unique: false
    });
    
    // Perform data integrity checks after the `versionchange` transaction
    // has completed
    transaction.oncomplete = () => {
      // TODO: perform data integrity checks here
    };
  }
);

Please comment here or reach out on gitter if you have questions or problems.

This fixes an issue with `IndexedDBCache#getInverseRelationshipsAsync` in which
the wrong index was being used to lookup items in the object store that
maintains inverse relationships (`__inverseRels__`).

This bug could result in the inability of the `AsyncCacheIntegrityProcessor` to
properly remove relationships when records themselves are removed.

The correct index, `relatedIdentity`, also needs to be added. This will need to be
handled for pre-existing databases with an IndexedDB migration.
@dgeb dgeb added the bug label Mar 6, 2021
@dgeb dgeb merged commit 0831275 into main Mar 6, 2021
@dgeb dgeb deleted the fix-idb-inverses branch March 6, 2021 18:06
@dgeb dgeb mentioned this pull request Mar 6, 2021
@dgeb
Copy link
Member Author

dgeb commented Mar 7, 2021

#826 improves upon this PR with a try/catch to prevent a hard fail as well as error logging guidance that points devs to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant