Skip to content

Commit 2c69d7c

Browse files
kushalshit27kushalshit27
authored
refactor: Improve efficiency and reliability of fetching enabled_clients for connections (#1334)
* refactor: Getting enabled clients for connection * feat(test): add test for fetching enabled clients while preserving connection order - test/tools/auth0/handlers/connections.tests.js: implement test to validate pool usage for enabled clients
1 parent a9098d6 commit 2c69d7c

3 files changed

Lines changed: 121 additions & 32 deletions

File tree

‎src/tools/auth0/handlers/connections.ts‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -161,19 +161,23 @@ export const getConnectionEnabledClients = async (
161161
if (!connectionId) return null;
162162

163163
try {
164-
const allClients: Management.ConnectionEnabledClient[] = [];
164+
log.debug(`Getting enabled clients for connection ${connectionId}`);
165+
166+
const enabledClients: Management.ConnectionEnabledClient[] = [];
165167
let page = await auth0Client.connections.clients.get(connectionId, { take: 100 });
166168

167-
allClients.push(...(page.data || []));
169+
enabledClients.push(...(page.data || []));
168170

169171
while (page.hasNextPage && page.hasNextPage()) {
170172
page = await page.getNextPage();
171-
allClients.push(...(page.data || []));
173+
enabledClients.push(...(page.data || []));
172174
}
173175

174-
return allClients.filter((client) => !!client?.client_id).map((client) => client.client_id);
176+
return enabledClients.filter((client) => !!client?.client_id).map((client) => client.client_id);
175177
} catch (error) {
176-
log.warn(`Unable to retrieve enabled clients for connection ${connectionId}: ${error?.message}`);
178+
log.warn(
179+
`Unable to retrieve enabled clients for connection ${connectionId}: ${error?.message}`
180+
);
177181
return null;
178182
}
179183
};
@@ -589,35 +593,45 @@ export default class ConnectionsHandler extends DefaultAPIHandler {
589593
this.existing = filteredConnections;
590594
if (this.existing === null) return [];
591595

592-
const connectionsWithEnabledClients = await Promise.all(
593-
filteredConnections.map(async (con) => {
594-
if (!con?.id) return con;
595-
const enabledClients = await getConnectionEnabledClients(this.client, con.id);
596+
const connectionTasks = filteredConnections.map((con, index) => ({ con, index }));
597+
598+
const connectionsWithEnabledClients = await this.client.pool
599+
.addEachTask({
600+
data: connectionTasks,
601+
generator: async ({ con, index }) => {
602+
if (!con?.id) {
603+
return { index, connection: con as Connection };
604+
}
596605

597-
// Cast to Asset to allow adding properties
598-
let connection: Connection = { ...con };
606+
const enabledClients = await getConnectionEnabledClients(this.client, con.id);
599607

600-
if (enabledClients && enabledClients?.length) {
601-
connection.enabled_clients = enabledClients;
602-
}
608+
let connection: Connection = { ...con };
603609

604-
if (connection.strategy === 'google-apps' && directoryProvisioningConfigs) {
605-
const dirProvConfig = directoryProvisioningConfigs.find(
606-
(congigCon) => congigCon.connection_id === con.id
607-
);
608-
if (dirProvConfig) {
609-
connection.directory_provisioning_configuration = {
610-
mapping: dirProvConfig.mapping,
611-
synchronize_automatically: dirProvConfig.synchronize_automatically,
612-
};
610+
if (enabledClients?.length) {
611+
connection.enabled_clients = enabledClients;
613612
}
614-
}
615613

616-
return connection;
614+
if (connection.strategy === 'google-apps' && directoryProvisioningConfigs) {
615+
const dirProvConfig = directoryProvisioningConfigs.find(
616+
(configCon) => configCon.connection_id === con.id
617+
);
618+
619+
if (dirProvConfig) {
620+
connection.directory_provisioning_configuration = {
621+
mapping: dirProvConfig.mapping,
622+
synchronize_automatically: dirProvConfig.synchronize_automatically,
623+
};
624+
}
625+
}
626+
627+
return { index, connection };
628+
},
617629
})
618-
);
630+
.promise();
619631

620-
this.existing = connectionsWithEnabledClients;
632+
this.existing = connectionsWithEnabledClients
633+
.sort((a, b) => a.index - b.index)
634+
.map(({ connection }) => connection);
621635

622636
// Apply `scim_configuration` to all the relevant `SCIM` connections. This method mutates `this.existing`.
623637
await this.scimHandler.applyScimConfiguration(this.existing);

‎test/tools/auth0/handlers/connections.tests.js‎

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -959,9 +959,12 @@ describe('#connections enabled clients functionality', () => {
959959

960960
// Simulate two pages via the PagedResponse format
961961
mockAuth0Client.connections.clients.get.resolves(
962-
mockPagedData({}, 'clients', [{ client_id: 'client_1' }, { client_id: 'client_2' }], [
963-
[{ client_id: 'client_3' }, { client_id: 'client_4' }],
964-
])
962+
mockPagedData(
963+
{},
964+
'clients',
965+
[{ client_id: 'client_1' }, { client_id: 'client_2' }],
966+
[[{ client_id: 'client_3' }, { client_id: 'client_4' }]]
967+
)
965968
);
966969

967970
const result = await getConnectionEnabledClients(mockAuth0Client, connectionId);
@@ -1420,7 +1423,9 @@ describe('#connections enabled clients functionality', () => {
14201423
// Mock enabled clients responses — SDK v5 .get() returns a PagedResponse, not a flat array
14211424
getEnabledClientsStub
14221425
.withArgs('con_1', { take: 100 })
1423-
.resolves(mockPagedData({}, 'clients', [{ client_id: 'client_1' }, { client_id: 'client_2' }]))
1426+
.resolves(
1427+
mockPagedData({}, 'clients', [{ client_id: 'client_1' }, { client_id: 'client_2' }])
1428+
)
14241429
.withArgs('con_2', { take: 100 })
14251430
.resolves(mockPagedData({}, 'clients', [{ client_id: 'client_3' }]));
14261431

@@ -1444,6 +1449,74 @@ describe('#connections enabled clients functionality', () => {
14441449
expect(result[1].enabled_clients).to.deep.equal(['client_3']);
14451450
});
14461451

1452+
it('should use the pool to fetch enabled clients and preserve connection order', async () => {
1453+
const addEachTaskStub = sinon.stub();
1454+
const poolExecutor = {
1455+
addSingleTask: ({ data, generator }) => ({
1456+
promise: async () => generator(data),
1457+
}),
1458+
addEachTask: addEachTaskStub,
1459+
};
1460+
1461+
addEachTaskStub.callsFake(({ data, generator }) => ({
1462+
promise: async () => {
1463+
const results = [];
1464+
1465+
for (const task of [...(data || [])].reverse()) {
1466+
results.push(await generator(task));
1467+
}
1468+
1469+
return results;
1470+
},
1471+
}));
1472+
1473+
const getEnabledClientsStub = sinon.stub();
1474+
const auth0 = {
1475+
connections: {
1476+
list: (params) =>
1477+
mockPagedData(params, 'connections', [
1478+
{ id: 'con_1', strategy: 'github', name: 'github-connection' },
1479+
{ id: 'con_2', strategy: 'google', name: 'google-connection' },
1480+
{ strategy: 'auth0', name: 'db-should-be-ignored' },
1481+
]),
1482+
clients: {
1483+
get: getEnabledClientsStub,
1484+
},
1485+
_getRestClient: () => ({}),
1486+
},
1487+
clients: {
1488+
list: (params) => mockPagedData(params, 'clients', []),
1489+
},
1490+
};
1491+
1492+
getEnabledClientsStub
1493+
.withArgs('con_1', { take: 100 })
1494+
.resolves(mockPagedData({}, 'clients', [{ client_id: 'client_1' }]))
1495+
.withArgs('con_2', { take: 100 })
1496+
.resolves(mockPagedData({}, 'clients', [{ client_id: 'client_2' }]));
1497+
1498+
const client = pageClient(auth0);
1499+
client.pool = poolExecutor;
1500+
1501+
const handler = new connections.default({ client, config });
1502+
handler.scimHandler = scimHandlerMock;
1503+
sinon.stub(handler, 'getConnectionDirectoryProvisionings').resolves(null);
1504+
1505+
const result = await handler.getType();
1506+
1507+
sinon.assert.calledOnce(addEachTaskStub);
1508+
expect(addEachTaskStub.firstCall.args[0].data).to.have.length(2);
1509+
expect(addEachTaskStub.firstCall.args[0].data.map(({ con }) => con.id)).to.deep.equal([
1510+
'con_1',
1511+
'con_2',
1512+
]);
1513+
expect(addEachTaskStub.firstCall.args[0].generator).to.be.a('function');
1514+
1515+
expect(result.map((connection) => connection.id)).to.deep.equal(['con_1', 'con_2']);
1516+
expect(result[0].enabled_clients).to.deep.equal(['client_1']);
1517+
expect(result[1].enabled_clients).to.deep.equal(['client_2']);
1518+
});
1519+
14471520
it('should handle connections without enabled clients', async () => {
14481521
const auth0 = {
14491522
connections: {

‎test/tools/auth0/handlers/databases.tests.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2242,7 +2242,9 @@ describe('#databases handler with enabled clients integration', () => {
22422242
// Mock enabled clients responses — SDK v5 .get() returns a PagedResponse, not a flat array
22432243
getEnabledClientsStub
22442244
.withArgs('con_1', { take: 100 })
2245-
.resolves(mockPagedData({}, 'clients', [{ client_id: 'client_1' }, { client_id: 'client_2' }]))
2245+
.resolves(
2246+
mockPagedData({}, 'clients', [{ client_id: 'client_1' }, { client_id: 'client_2' }])
2247+
)
22462248
.withArgs('con_2', { take: 100 })
22472249
.resolves(mockPagedData({}, 'clients', [{ client_id: 'client_3' }]));
22482250

0 commit comments

Comments
 (0)