diff --git a/addons/api/addon/handlers/indexed-db-handler.js b/addons/api/addon/handlers/indexed-db-handler.js index e9e527169..52c725a64 100644 --- a/addons/api/addon/handlers/indexed-db-handler.js +++ b/addons/api/addon/handlers/indexed-db-handler.js @@ -64,6 +64,11 @@ export default class IndexedDbHandler { if (storeToken) { const tokenObj = await indexedDb.token.get(tokenKey); listToken = tokenObj?.token; + } else { + // This is a temporary fix of clearing the DB (specifically for auth-methods) + // since we are not storing the token we do not get back a list of removed_ids + // from the API call and we do not want deleted items from showing in list view. + await indexedDb[type].clear(); } do { @@ -210,13 +215,6 @@ export default class IndexedDbHandler { await indexedDb[type].bulkDelete(payload.removed_ids); } - // This is a temporary fix of clearing the DB (specifically for auth-methods) - // since we are not storing the token we do not get back a list of removed_ids - // from the API call and we do not want deleted items from showing in list view. - if (!storeToken) { - await indexedDb[type].clear(); - } - const normalizedPayload = serializer.normalizeResponse( store, schema, diff --git a/addons/api/addon/handlers/sqlite-handler.js b/addons/api/addon/handlers/sqlite-handler.js index 12ec7e95b..01a361699 100644 --- a/addons/api/addon/handlers/sqlite-handler.js +++ b/addons/api/addon/handlers/sqlite-handler.js @@ -37,7 +37,7 @@ export default class SqliteHandler { // Go through normal flow if we don't yet support the model // or if we don't have a sqlite db instance - if (!supportedModels.includes(type) || !this.sqlite) { + if (!supportedModels.includes(type) || !this.sqlite.worker) { return next(context.request); } @@ -60,6 +60,11 @@ export default class SqliteHandler { }), }); listToken = tokenObj?.token; + } else { + // This is a temporary fix of clearing the DB (specifically for auth-methods) + // since we are not storing the token we do not get back a list of removed_ids + // from the API call and we do not want deleted items from showing in list view. + await this.sqlite.deleteResource(type); } do { @@ -76,8 +81,15 @@ export default class SqliteHandler { await writeToDbPromise; } } catch (err) { - console.error('Error retrieving data:', err); - throw err; + payload = await this.retryQueryFailure({ + err, + type, + adapter, + store, + schema, + remainingQuery, + }); + totalInsert += payload.items?.length ?? 0; } listToken = payload.list_token; @@ -149,6 +161,45 @@ export default class SqliteHandler { } } + async retryQueryFailure({ + err, + type, + adapter, + store, + schema, + remainingQuery, + }) { + // If we get an invalid list token, we'll delete the token and + // clear the DB and try again without a token. + if ( + err?.errors[0].status === 400 && + err?.errors[0].code === 'invalid list token' + ) { + await this.sqlite.deleteResource(type); + // Delete all tokens of the same resource type so we don't keep clearing the DB. + // Even if other tokens may still be valid, we might have cleared data that they + // depended on so we should clear all tokens of the same type when clearing the DB. + const { sql, parameters } = generateSQLExpressions('token', { + filters: { id: [{ contains: type }] }, + }); + const tokenRows = await this.sqlite.fetchResource({ + sql, + parameters, + }); + const tokenIds = tokenRows.map((row) => row.id); + + if (tokenIds.length > 0) { + await this.sqlite.deleteResource('token', tokenIds); + } + + return await adapter.query(store, schema, { + ...remainingQuery, + }); + } else { + throw err; + } + } + async writeToDb( payload, storeToken, diff --git a/addons/api/addon/services/sqlite.js b/addons/api/addon/services/sqlite.js index 937a649ae..41cfd9dbe 100644 --- a/addons/api/addon/services/sqlite.js +++ b/addons/api/addon/services/sqlite.js @@ -65,6 +65,15 @@ export const modelMapping = { scope_id: 'scope.scope_id', created_time: 'created_time', }, + 'auth-method': { + id: 'id', + type: 'type', + name: 'name', + description: 'description', + is_primary: 'is_primary', + scope_id: 'scope.scope_id', + created_time: 'created_time', + }, }; // A list of tables that we support searching using FTS5 in SQLite. @@ -75,6 +84,7 @@ export const searchTables = new Set([ 'user', 'credential-store', 'scope', + 'auth-method', ]); export default class SqliteDbService extends Service { diff --git a/addons/api/addon/workers/sqlite-worker.js b/addons/api/addon/workers/sqlite-worker.js index 424c96bb0..fd7ad8b3a 100644 --- a/addons/api/addon/workers/sqlite-worker.js +++ b/addons/api/addon/workers/sqlite-worker.js @@ -94,12 +94,13 @@ const methods = { db.exec(CREATE_TABLES(SCHEMA_VERSION)); }, deleteDatabase: () => { - const name = db.dbFilename(); - if (db) { - db.close(); - db = null; + if (!db) { + throw new Error('No database was initialized'); } + const name = db.dbFilename(); + db.close(); + db = null; poolUtil.unlink(`/${name}`); }, downloadDatabase: () => { @@ -156,12 +157,8 @@ const methods = { }); }, deleteResource: ({ resource, ids }) => { - if (ids.length === 0) { - return []; - } - // Check if we have too many parameters for the deletion and chunk if so - if (ids.length > MAX_HOST_PARAMETERS) { + if (ids?.length > MAX_HOST_PARAMETERS) { return db.transaction(() => { const results = []; for (let i = 0; i < ids.length; i += MAX_HOST_PARAMETERS) { diff --git a/addons/api/addon/workers/utils/schema.js b/addons/api/addon/workers/utils/schema.js index 4c6f943dc..d5ff082c1 100644 --- a/addons/api/addon/workers/utils/schema.js +++ b/addons/api/addon/workers/utils/schema.js @@ -230,6 +230,43 @@ CREATE TRIGGER IF NOT EXISTS scope_ad AFTER DELETE ON scope BEGIN VALUES('delete', old.rowid, old.id, old.type, old.name, old.description, old.scope_id, old.created_time); END;`; +const createAuthMethodTables = ` +CREATE TABLE IF NOT EXISTS auth_method ( + id TEXT NOT NULL PRIMARY KEY, + type TEXT NOT NULL, + name TEXT, + description TEXT, + is_primary INT, + scope_id TEXT NOT NULL, + created_time TEXT NOT NULL, + data TEXT NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_auth_method_scope_id_created_time ON auth_method(scope_id, created_time DESC); + +CREATE VIRTUAL TABLE IF NOT EXISTS auth_method_fts USING fts5( + id, + type, + name, + description, + is_primary, + scope_id, + created_time, + content='', +); + +CREATE TRIGGER IF NOT EXISTS auth_method_ai AFTER INSERT ON auth_method BEGIN + INSERT INTO auth_method_fts( + id, type, name, description, is_primary, scope_id, created_time + ) VALUES ( + new.id, new.type, new.name, new.description, new.is_primary, new.scope_id, new.created_time + ); +END; + +CREATE TRIGGER IF NOT EXISTS auth_method_ad AFTER DELETE ON auth_method BEGIN + INSERT INTO auth_method_fts(auth_method_fts, rowid, id, type, name, description, is_primary, scope_id, created_time) + VALUES('delete', old.rowid, old.id, old.type, old.name, old.description, old.is_primary, old.scope_id, old.created_time); +END;`; + export const CREATE_TABLES = (version) => ` BEGIN; @@ -246,6 +283,7 @@ ${createRoleTables} ${createUserTables} ${createCredentialStoreTables} ${createScopeTables} +${createAuthMethodTables} CREATE TABLE IF NOT EXISTS "group" ( id TEXT NOT NULL PRIMARY KEY, @@ -276,5 +314,9 @@ export const INSERT_STATEMENTS = (resource, items, modelMapping) => { return `REPLACE INTO "${underscore(resource)}" (${columns.join(', ')}, data) VALUES ${items.map(() => placeholders).join(', ')};`; }; -export const DELETE_STATEMENT = (resource, ids) => - `DELETE FROM "${underscore(resource)}" WHERE id IN (${ids.map(() => '?').join(',')})`; +export const DELETE_STATEMENT = (resource, ids) => { + const baseDeleteStatement = `DELETE FROM "${underscore(resource)}"`; + return ids?.length > 0 + ? `${baseDeleteStatement} WHERE id IN (${ids.map(() => '?').join(',')})` + : baseDeleteStatement; +}; diff --git a/addons/api/tests/unit/handlers/sqlite-handler-test.js b/addons/api/tests/unit/handlers/sqlite-handler-test.js index 3b3f17dd0..456daccc3 100644 --- a/addons/api/tests/unit/handlers/sqlite-handler-test.js +++ b/addons/api/tests/unit/handlers/sqlite-handler-test.js @@ -73,17 +73,15 @@ module('Unit | Handler | sqlite-handler', function (hooks) { sqliteHandler, store, applicationAdapter, - sqliteHandlerSpy, - applicationAdapterSpy; + sqliteHandlerSpy; hooks.beforeEach(async function setupSqliteHandler() { store = this.owner.lookup('service:store'); manager = new RequestManager(); sqliteHandler = new SqliteHandler(store); applicationAdapter = store.adapterFor('application'); - // spy on all handler and application adapter methods + // spy on handler methods sqliteHandlerSpy = sinon.spy(sqliteHandler); - applicationAdapterSpy = sinon.spy(applicationAdapter); store.requestManager = manager; manager.use([sqliteHandler]); @@ -94,6 +92,10 @@ module('Unit | Handler | sqlite-handler', function (hooks) { }); }); + hooks.afterEach(async function cleanUpSqliteHandler() { + sinon.restore(); + }); + module('fetching and batch loading', function (hooks) { // `testBatchLimit` and `serverResultPageLimit` are intentionally difference // so that multiple calls to the server are required to fetch a single batch, @@ -137,6 +139,8 @@ module('Unit | Handler | sqlite-handler', function (hooks) { }); test('supports batching requests and writes to SQLite', async function (assert) { + const applicationAdapterSpy = sinon.spy(applicationAdapter); + await store.query('target', {}); assert.strictEqual( @@ -197,6 +201,36 @@ module('Unit | Handler | sqlite-handler', function (hooks) { ); assert.strictEqual(callCounts.writeToDb, expectedWriteToSqliteCallCount); }); + + test('it stores and retrieves tokens when storeToken is true (default)', async function (assert) { + const sqliteSpy = sinon.spy(sqliteHandler.sqlite); + + await store.query('target', {}); + + assert.strictEqual( + sqliteSpy.insertResource.withArgs('token').callCount, + targets.length / testBatchLimit, + ); + assert.strictEqual( + sqliteSpy.insertResource.withArgs('target').callCount, + targets.length / testBatchLimit, + ); + }); + + test('it clears database and does not store tokens when storeToken is false', async function (assert) { + const sqliteSpy = sinon.spy(sqliteHandler.sqlite); + + await store.query('target', {}, { storeToken: false }); + + // Targets are all deleted one time + assert.ok(sqliteSpy.deleteResource.calledOnceWithExactly('target')); + + assert.ok(sqliteSpy.insertResource.withArgs('token').notCalled); + assert.strictEqual( + sqliteSpy.insertResource.withArgs('target').callCount, + targets.length / testBatchLimit, + ); + }); }); module('option: pushToStore', function (hooks) { @@ -435,4 +469,58 @@ module('Unit | Handler | sqlite-handler', function (hooks) { assert.strictEqual(results[0].name, 'specific-target'); }); + + test('it retries query when invalid list token error occurs', async function (assert) { + let targets = this.server.createList('target', 5, { scope }); + + const sqliteSpy = sinon.spy(sqliteHandler.sqlite); + + // Mock the first query to fail with invalid list token error but return data as normal otherwise + sinon + .stub(applicationAdapter, 'query') + .callThrough() + .onFirstCall() + .throws(() => { + const error = new Error('Invalid list token'); + error.errors = [ + { + status: 400, + code: 'invalid list token', + }, + ]; + return error; + }); + + this.server.get( + 'targets', + createPaginatedResponseHandler.call(this, targets, { + pageSize: 5, + }), + ); + + // Insert tokens to simulate existing state + await sqliteHandler.sqlite.insertResource('token', [ + ['target-123', 'invalid key'], + ['target-321', 'valid key'], + ['session-123', 'session key'], + ]); + + const results = await store.query('target', {}); + + assert.ok(sqliteHandlerSpy.retryQueryFailure.calledOnce); + + assert.ok(sqliteSpy.deleteResource.calledWithExactly('target')); + assert.ok( + sqliteSpy.deleteResource.calledWithExactly('token', [ + 'target-123', + 'target-321', + ]), + ); + + // Verify the query eventually succeeded + assert.strictEqual(results.length, 5); + + // Verify query was called twice (initial fail + retry) + assert.ok(applicationAdapter.query.calledTwice); + }); }); diff --git a/ui/admin/app/routes/scopes/scope/auth-methods/index.js b/ui/admin/app/routes/scopes/scope/auth-methods/index.js index a28f6061c..dd52715ee 100644 --- a/ui/admin/app/routes/scopes/scope/auth-methods/index.js +++ b/ui/admin/app/routes/scopes/scope/auth-methods/index.js @@ -94,7 +94,7 @@ export default class ScopesScopeAuthMethodsIndexRoute extends Route { }); if (primary.length === 1) { primary.forEach((val) => { - filters.is_primary.push({ equals: val }); + filters.is_primary.push({ equals: val === 'true' }); }); } diff --git a/ui/admin/tests/acceptance/accounts/change-password-test.js b/ui/admin/tests/acceptance/accounts/change-password-test.js index 039469cab..ad0fcf938 100644 --- a/ui/admin/tests/acceptance/accounts/change-password-test.js +++ b/ui/admin/tests/acceptance/accounts/change-password-test.js @@ -21,7 +21,6 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | accounts | change password', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); - setupIndexedDb(hooks); setupSqlite(hooks); setupIndexedDb(hooks); diff --git a/ui/admin/tests/acceptance/auth-methods/create-test.js b/ui/admin/tests/acceptance/auth-methods/create-test.js index 992478d07..ab8270a87 100644 --- a/ui/admin/tests/acceptance/auth-methods/create-test.js +++ b/ui/admin/tests/acceptance/auth-methods/create-test.js @@ -18,8 +18,8 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | auth-methods | create', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); - setupIndexedDb(hooks); setupSqlite(hooks); + setupIndexedDb(hooks); let getAuthMethodsCount; let featuresService; diff --git a/ui/admin/tests/acceptance/auth-methods/delete-test.js b/ui/admin/tests/acceptance/auth-methods/delete-test.js index ac1a24625..e8634c9ee 100644 --- a/ui/admin/tests/acceptance/auth-methods/delete-test.js +++ b/ui/admin/tests/acceptance/auth-methods/delete-test.js @@ -8,6 +8,7 @@ import { visit, click, currentURL } from '@ember/test-helpers'; import { setupApplicationTest } from 'admin/tests/helpers'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import { setupIndexedDb } from 'api/test-support/helpers/indexed-db'; +import { setupSqlite } from 'api/test-support/helpers/sqlite'; import { Response } from 'miragejs'; import { authenticateSession } from 'ember-simple-auth/test-support'; import { TYPE_AUTH_METHOD_LDAP } from 'api/models/auth-method'; @@ -18,6 +19,7 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | auth-methods | delete', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); + setupSqlite(hooks); setupIndexedDb(hooks); let featuresService; diff --git a/ui/admin/tests/acceptance/auth-methods/list-test.js b/ui/admin/tests/acceptance/auth-methods/list-test.js index 9fd089495..8b5010342 100644 --- a/ui/admin/tests/acceptance/auth-methods/list-test.js +++ b/ui/admin/tests/acceptance/auth-methods/list-test.js @@ -22,8 +22,8 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | auth-methods | list', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); - setupIndexedDb(hooks); setupSqlite(hooks); + setupIndexedDb(hooks); const instances = { scopes: { diff --git a/ui/admin/tests/acceptance/auth-methods/read-test.js b/ui/admin/tests/acceptance/auth-methods/read-test.js index 615a78cfd..37b1fcae8 100644 --- a/ui/admin/tests/acceptance/auth-methods/read-test.js +++ b/ui/admin/tests/acceptance/auth-methods/read-test.js @@ -22,8 +22,8 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | auth-methods | read', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); - setupIndexedDb(hooks); setupSqlite(hooks); + setupIndexedDb(hooks); const instances = { scopes: { diff --git a/ui/admin/tests/acceptance/auth-methods/update-test.js b/ui/admin/tests/acceptance/auth-methods/update-test.js index 7de8456ae..0086a0740 100644 --- a/ui/admin/tests/acceptance/auth-methods/update-test.js +++ b/ui/admin/tests/acceptance/auth-methods/update-test.js @@ -8,6 +8,7 @@ import { visit, click, fillIn, select, findAll } from '@ember/test-helpers'; import { setupApplicationTest } from 'admin/tests/helpers'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import { setupIndexedDb } from 'api/test-support/helpers/indexed-db'; +import { setupSqlite } from 'api/test-support/helpers/sqlite'; import { Response } from 'miragejs'; import { authenticateSession } from 'ember-simple-auth/test-support'; import { @@ -21,6 +22,7 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | auth-methods | update', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); + setupSqlite(hooks); setupIndexedDb(hooks); let featuresService; diff --git a/ui/admin/tests/acceptance/managed-groups/create-test.js b/ui/admin/tests/acceptance/managed-groups/create-test.js index d80efc7bf..cca9f5f9c 100644 --- a/ui/admin/tests/acceptance/managed-groups/create-test.js +++ b/ui/admin/tests/acceptance/managed-groups/create-test.js @@ -8,6 +8,7 @@ import { visit, currentURL, click, fillIn } from '@ember/test-helpers'; import { setupApplicationTest } from 'admin/tests/helpers'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import { setupIndexedDb } from 'api/test-support/helpers/indexed-db'; +import { setupSqlite } from 'api/test-support/helpers/sqlite'; import { authenticateSession } from 'ember-simple-auth/test-support'; import { Response } from 'miragejs'; import { @@ -21,6 +22,7 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | managed-groups | create', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); + setupSqlite(hooks); setupIndexedDb(hooks); let getManagedGroupCount; diff --git a/ui/admin/tests/acceptance/managed-groups/list-test.js b/ui/admin/tests/acceptance/managed-groups/list-test.js index bad5d04ae..aad611b4a 100644 --- a/ui/admin/tests/acceptance/managed-groups/list-test.js +++ b/ui/admin/tests/acceptance/managed-groups/list-test.js @@ -8,6 +8,7 @@ import { visit, click } from '@ember/test-helpers'; import { setupApplicationTest } from 'admin/tests/helpers'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import { setupIndexedDb } from 'api/test-support/helpers/indexed-db'; +import { setupSqlite } from 'api/test-support/helpers/sqlite'; import { authenticateSession } from 'ember-simple-auth/test-support'; import { TYPE_AUTH_METHOD_OIDC, @@ -20,6 +21,7 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | managed-groups | list', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); + setupSqlite(hooks); setupIndexedDb(hooks); let featuresService; diff --git a/ui/admin/tests/acceptance/users/accounts-test.js b/ui/admin/tests/acceptance/users/accounts-test.js index f638b2ba0..18f151668 100644 --- a/ui/admin/tests/acceptance/users/accounts-test.js +++ b/ui/admin/tests/acceptance/users/accounts-test.js @@ -7,6 +7,7 @@ import { module, test } from 'qunit'; import { visit, currentURL, click } from '@ember/test-helpers'; import { setupApplicationTest } from 'admin/tests/helpers'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; +import { setupSqlite } from 'api/test-support/helpers/sqlite'; import { Response } from 'miragejs'; import { authenticateSession } from 'ember-simple-auth/test-support'; import { @@ -21,6 +22,7 @@ import { setRunOptions } from 'ember-a11y-testing/test-support'; module('Acceptance | users | accounts', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); + setupSqlite(hooks); let accountsCount; let features; diff --git a/ui/admin/tests/unit/controllers/scopes/scope/auth-methods/index-test.js b/ui/admin/tests/unit/controllers/scopes/scope/auth-methods/index-test.js index cb8cd4955..b2de56b05 100644 --- a/ui/admin/tests/unit/controllers/scopes/scope/auth-methods/index-test.js +++ b/ui/admin/tests/unit/controllers/scopes/scope/auth-methods/index-test.js @@ -9,6 +9,7 @@ import { waitUntil } from '@ember/test-helpers'; import { visit } from '@ember/test-helpers'; import setupMirage from 'ember-cli-mirage/test-support/setup-mirage'; import { setupIndexedDb } from 'api/test-support/helpers/indexed-db'; +import { setupSqlite } from 'api/test-support/helpers/sqlite'; import { setupIntl } from 'ember-intl/test-support'; import { authenticateSession } from 'ember-simple-auth/test-support'; import { @@ -20,6 +21,7 @@ import { module('Unit | Controller | scopes/scope/auth-methods/index', function (hooks) { setupTest(hooks); setupMirage(hooks); + setupSqlite(hooks); setupIndexedDb(hooks); setupIntl(hooks, 'en-us');