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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/97-potential-unwanted-sync-during-development #7597

Merged
66 changes: 45 additions & 21 deletions app/api/sync/specs/syncWorker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,19 @@ describe('syncWorker', () => {
name: 'host1',
dbName: 'host1',
indexName: 'host1',
featureFlags: {
sync: true,
},
...(await testingUploadPaths()),
});

tenants.add({
name: 'host2',
dbName: 'host2',
indexName: 'host2',
featureFlags: {
sync: true,
},
...(await testingUploadPaths()),
});

Expand Down Expand Up @@ -488,26 +494,6 @@ describe('syncWorker', () => {
});
});

describe('when active is false', () => {
it('should not sync anything', async () => {
await applyFixtures();
await runAllTenants();
const changedFixtures = _.cloneDeep(host1Fixtures);
//@ts-ignore
changedFixtures.settings[0].sync[0].config.templates = {};
//@ts-ignore
changedFixtures.settings[0].sync[0].active = false;
await db.setupFixturesAndContext({ ...changedFixtures }, undefined, 'host1');

await runAllTenants();

await tenants.run(async () => {
const syncedTemplates = await templates.get();
expect(syncedTemplates).toHaveLength(1);
}, 'target1');
}, 10000);
});

it('should sync collections in correct preference order', async () => {
const originalBatchLimit = syncWorker.UPDATE_LOG_TARGET_COUNT;
syncWorker.UPDATE_LOG_TARGET_COUNT = 1;
Expand Down Expand Up @@ -567,12 +553,13 @@ describe('syncWorker', () => {

await applyFixtures();
syncWorker.UPDATE_LOG_TARGET_COUNT = originalBatchLimit;
});
}, 10000);

it('should throw an error, when trying to sync a collection that is not in the order list', async () => {
const fixtures = _.cloneDeep(orderedHostFixtures);
//@ts-ignore
fixtures.settings[0].sync[0].config.pages = [];

await applyFixtures(fixtures, {});

await expect(runAllTenants).rejects.toThrow(
Expand All @@ -581,4 +568,41 @@ describe('syncWorker', () => {

await applyFixtures();
});

describe('when active is false', () => {
it('should not sync anything', async () => {
await applyFixtures();
await runAllTenants();
const changedFixtures = _.cloneDeep(host1Fixtures);
//@ts-ignore
changedFixtures.settings[0].sync[0].config.templates = {};
tenants.add({ ...tenants.tenants.host1, featureFlags: { sync: false } });

await db.setupFixturesAndContext({ ...changedFixtures }, undefined, 'host1');

await runAllTenants();

await tenants.run(async () => {
const syncedTemplates = await templates.get();
expect(syncedTemplates).toHaveLength(1);
}, 'target1');
}, 10000);
});

it('should only sync on targeted tenants', async () => {
await applyFixtures();
const _fixtures = { ...host1Fixtures };

//@ts-ignore
_fixtures.settings[0].sync[0].active = false;

await db.setupFixturesAndContext(_fixtures, undefined, 'host1');

await runAllTenants();

await tenants.run(async () => {
const syncedTemplates = await templates.get();
expect(syncedTemplates).toHaveLength(0);
}, 'target1');
}, 10000);
});
12 changes: 6 additions & 6 deletions app/api/sync/syncWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ class InvalidSyncConfig extends Error {

interface SyncConfig {
url: string;
active?: boolean;
username: string;
active?: boolean;
password: string;
name: string;
config: {
Expand All @@ -54,25 +54,25 @@ export const syncWorker = {
UPDATE_LOG_TARGET_COUNT: 50,

async runAllTenants() {
return Object.keys(tenants.tenants).reduce(async (previous, tenantName) => {
return tenants.getTenantsForFeatureFlag('sync').reduce(async (previous, tenant) => {
await previous;
return tenants.run(async () => {
permissionsContext.setCommandContext();
const { sync } = await settings.get({}, 'sync');
if (sync) {
await this.syncronize(sync);
}
}, tenantName);
}, tenant.name);
}, Promise.resolve());
},

async syncronize(syncSettings: SettingsSyncSchema[]) {
await syncSettings.reduce(async (previousSync, config) => {
await previousSync;
const syncConfig = validateConfig(config);
if (syncConfig.active) {
await this.syncronizeConfig(syncConfig);
}
if (!syncConfig?.active) return;

await this.syncronizeConfig(syncConfig);
}, Promise.resolve());
},

Expand Down
20 changes: 20 additions & 0 deletions app/api/tenants/specs/tenantsContext.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,24 @@ describe('tenantsContext', () => {
expect(tenants.tenants['tenant two'].dbName).toBe('tenant_two');
});
});

it('should only return tenants enabled for given feature flag', () => {
tenants.add({
name: 'test-tenant',
dbName: 'test-tenant-db',
featureFlags: { s3Storage: true },
});

tenants.add({
name: 'test-tenant-2',
dbName: 'test-tenant-db',
featureFlags: { s3Storage: false },
});

const result = tenants.getTenantsForFeatureFlag('s3Storage');

expect(result).toHaveLength(1);
expect(result[0].name).toBe('test-tenant');
expect(result[0].featureFlags?.s3Storage).toBeTruthy();
});
});
9 changes: 8 additions & 1 deletion app/api/tenants/tenantContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { handleError } from 'api/utils';
import { appContext } from 'api/utils/AppContext';
import { TenantDocument, TenantsModel, DBTenant, tenantsModel } from './tenantsModel';

type TenantFeatureFlags = keyof NonNullable<Required<Tenant>['featureFlags']>;

type Tenant = {
name: string;
dbName: string;
Expand All @@ -13,6 +15,7 @@ type Tenant = {
activityLogs: string;
featureFlags?: {
s3Storage?: boolean;
sync?: boolean;
};
globalMatomo?: { id: string; url: string };
ciMatomoActive?: boolean;
Expand Down Expand Up @@ -86,8 +89,12 @@ class Tenants {
add(tenant: DBTenant) {
this.tenants[tenant.name] = { ...this.defaultTenant, ...tenant };
}

getTenantsForFeatureFlag(featureFlag: TenantFeatureFlags) {
return Object.values(this.tenants).filter(tenant => tenant?.featureFlags?.[featureFlag]);
}
}

const tenants = new Tenants(config.defaultTenant);
export { tenants };
export type { Tenant };
export type { Tenant, TenantFeatureFlags };
1 change: 1 addition & 0 deletions app/api/tenants/tenantsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const mongoSchema = new mongoose.Schema({
activityLogs: String,
featureFlags: {
s3Storage: Boolean,
sync: Boolean,
},
globalMatomo: { id: String, url: String },
ciMatomoActive: Boolean,
Expand Down
2 changes: 1 addition & 1 deletion app/shared/types/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,9 @@ const settingsSyncSchema = {
required: ['url', 'username', 'password', 'name', 'config'],
properties: {
url: { type: 'string' },
active: { type: 'boolean' },
username: { type: 'string' },
password: { type: 'string' },
active: { type: 'boolean' },
name: { type: 'string' },
config: {
type: 'object',
Expand Down
2 changes: 1 addition & 1 deletion app/shared/types/settingsType.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ export type SettingsSyncRelationtypesSchema = string[];

export interface SettingsSyncSchema {
url: string;
active?: boolean;
username: string;
password: string;
active?: boolean;
name: string;
config: {
templates?: {
Expand Down
Loading