Skip to content

Commit 64221e6

Browse files
chore(databases-collections): align DropNamespacePlugin and RenameCollectionPlugin to emit event via ConnectionScopedRegistry (#5744)
1 parent c64ae18 commit 64221e6

File tree

15 files changed

+326
-119
lines changed

15 files changed

+326
-119
lines changed

packages/compass-app-stores/src/stores/instance-store.spec.ts

Lines changed: 112 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,35 @@ describe('InstanceStore [Store]', function () {
177177
});
178178

179179
context(`on 'collection-dropped' event`, function () {
180-
it('should remove collection from the database collections', function () {
180+
it('should not respond when the connectionId does not matches the connectionId for the instance', function () {
181+
// we only start with 2 collections;
182+
expect(
183+
connectedInstance.databases.get('foo')?.collections.length
184+
).to.equal(2);
185+
186+
// emit the event without connectionId
181187
globalAppRegistry.emit('collection-dropped', 'foo.bar');
188+
189+
// should still be 2
190+
expect(
191+
connectedInstance.databases.get('foo')?.collections.length
192+
).to.equal(2);
193+
194+
// emit the event with a different connectionId
195+
globalAppRegistry.emit('collection-dropped', 'foo.bar', {
196+
connectionId: '2',
197+
});
198+
199+
// should still be 2
200+
expect(
201+
connectedInstance.databases.get('foo')?.collections.length
202+
).to.equal(2);
203+
});
204+
205+
it('should remove collection from the database collections', function () {
206+
globalAppRegistry.emit('collection-dropped', 'foo.bar', {
207+
connectionId: '1',
208+
});
182209
expect(
183210
connectedInstance.databases.get('foo')?.collections.get('foo.bar')
184211
).not.to.exist;
@@ -190,21 +217,48 @@ describe('InstanceStore [Store]', function () {
190217
?.collections.get('foo.bar', '_id');
191218
coll?.on('change', () => {});
192219
expect((coll as any)._events.change).to.have.lengthOf(1);
193-
globalAppRegistry.emit('collection-dropped', 'foo.bar');
220+
globalAppRegistry.emit('collection-dropped', 'foo.bar', {
221+
connectionId: '1',
222+
});
194223
expect((coll as any)._events).to.not.exist;
195224
});
196225

197226
it('should remove database if last collection was removed', function () {
198-
globalAppRegistry.emit('collection-dropped', 'foo.bar');
199-
globalAppRegistry.emit('collection-dropped', 'foo.buz');
227+
globalAppRegistry.emit('collection-dropped', 'foo.bar', {
228+
connectionId: '1',
229+
});
230+
globalAppRegistry.emit('collection-dropped', 'foo.buz', {
231+
connectionId: '1',
232+
});
200233
expect(connectedInstance.databases).to.have.lengthOf(0);
201234
expect(connectedInstance.databases.get('foo')).not.to.exist;
202235
});
203236
});
204237

205238
context(`on 'database-dropped' event`, function () {
206-
it('should remove database from instance databases', function () {
239+
it('should not respond when the connectionId does not matches the connectionId for the instance', function () {
240+
// we only start with 1 database;
241+
expect(connectedInstance.databases.length).to.equal(1);
242+
243+
// emit the event without connectionId
207244
globalAppRegistry.emit('database-dropped', 'foo');
245+
246+
// should still be 1
247+
expect(connectedInstance.databases.length).to.equal(1);
248+
249+
// emit the event with a different connectionId
250+
globalAppRegistry.emit('database-dropped', 'foo', {
251+
connectionId: '2',
252+
});
253+
254+
// should still be 1
255+
expect(connectedInstance.databases.length).to.equal(1);
256+
});
257+
258+
it('should remove database from instance databases', function () {
259+
globalAppRegistry.emit('database-dropped', 'foo', {
260+
connectionId: '1',
261+
});
208262
expect(connectedInstance.databases).to.have.lengthOf(0);
209263
expect(connectedInstance.databases.get('foo')).not.to.exist;
210264
});
@@ -213,7 +267,9 @@ describe('InstanceStore [Store]', function () {
213267
const db = connectedInstance.databases.get('foo');
214268
db?.on('change', () => {});
215269
expect((db as any)._events.change).to.have.lengthOf(1);
216-
globalAppRegistry.emit('database-dropped', 'foo');
270+
globalAppRegistry.emit('database-dropped', 'foo', {
271+
connectionId: '1',
272+
});
217273
expect((db as any)._events).to.not.exist;
218274
});
219275
});
@@ -249,20 +305,29 @@ describe('InstanceStore [Store]', function () {
249305
}
250306

251307
context(`on 'collection-created' event`, function () {
252-
it('should only respond when there is a connectionId and it matches the connectionId for the instance', function () {
253-
const collectionsLength =
254-
connectedInstance.databases.get('foo')?.collections.length;
255-
expect(collectionsLength).to.equal(2); // we only start with 2 collections;
308+
it('should not respond when the connectionId does not matches the connectionId for the instance', function () {
309+
// we only start with 2 collections;
310+
expect(
311+
connectedInstance.databases.get('foo')?.collections.length
312+
).to.equal(2);
256313

257314
// emit the event without connectionId
258315
globalAppRegistry.emit('collection-created', 'foo.qux');
259-
expect(collectionsLength).to.equal(2); // should still be 2
316+
317+
// should still be 2
318+
expect(
319+
connectedInstance.databases.get('foo')?.collections.length
320+
).to.equal(2);
260321

261322
// emit the event with a different connectionId
262323
globalAppRegistry.emit('collection-created', 'foo.qux', {
263324
connectionId: '2',
264325
});
265-
expect(collectionsLength).to.equal(2); // should still be 2
326+
327+
// should still be 2
328+
expect(
329+
connectedInstance.databases.get('foo')?.collections.length
330+
).to.equal(2);
266331
});
267332

268333
it('should add collection to the databases collections', function () {
@@ -295,11 +360,42 @@ describe('InstanceStore [Store]', function () {
295360
});
296361

297362
context(`on 'collection-renamed' event`, function () {
298-
it('should update collection _id', function () {
299-
globalAppRegistry.emit('collection-renamed', {
300-
from: 'foo.bar',
301-
to: 'foo.qux',
363+
it('should not respond when the connectionId does not matches the connectionId for the instance', function () {
364+
// we only start with 2 collections;
365+
expect(
366+
connectedInstance.databases.get('foo')?.collections.length
367+
).to.equal(2);
368+
369+
// emit the event without connectionId
370+
globalAppRegistry.emit('collection-renamed', 'foo.qux');
371+
372+
// should still be 2
373+
expect(
374+
connectedInstance.databases.get('foo')?.collections.length
375+
).to.equal(2);
376+
377+
// emit the event with a different connectionId
378+
globalAppRegistry.emit('collection-renamed', 'foo.qux', {
379+
connectionId: '2',
302380
});
381+
382+
// should still be 2
383+
expect(
384+
connectedInstance.databases.get('foo')?.collections.length
385+
).to.equal(2);
386+
});
387+
388+
it('should update collection _id', function () {
389+
globalAppRegistry.emit(
390+
'collection-renamed',
391+
{
392+
from: 'foo.bar',
393+
to: 'foo.qux',
394+
},
395+
{
396+
connectionId: connectedConnectionInfoId,
397+
}
398+
);
303399
expect(
304400
connectedInstance.databases.get('foo')?.collections
305401
).to.have.lengthOf(2);

packages/compass-app-stores/src/stores/instance-store.ts

Lines changed: 52 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -238,43 +238,69 @@ export function createInstancesStore(
238238

239239
on(globalAppRegistry, 'refresh-data', refreshInstance);
240240

241-
on(globalAppRegistry, 'database-dropped', (dbName: string) => {
242-
const db = instance.databases.remove(dbName);
243-
if (db) {
244-
MongoDBInstance.removeAllListeners(db);
241+
on(
242+
globalAppRegistry,
243+
'database-dropped',
244+
(dbName: string, { connectionId }: { connectionId?: string } = {}) => {
245+
if (connectionId !== instanceConnectionId) {
246+
return;
247+
}
248+
249+
const db = instance.databases.remove(dbName);
250+
if (db) {
251+
MongoDBInstance.removeAllListeners(db);
252+
}
245253
}
246-
});
254+
);
247255

248-
on(globalAppRegistry, 'collection-dropped', (namespace: string) => {
249-
const { database } = toNS(namespace);
250-
const db = instance.databases.get(database);
251-
const coll = db?.collections.get(namespace, '_id');
256+
on(
257+
globalAppRegistry,
258+
'collection-dropped',
259+
(
260+
namespace: string,
261+
{ connectionId }: { connectionId?: string } = {}
262+
) => {
263+
if (connectionId !== instanceConnectionId) {
264+
return;
265+
}
252266

253-
if (!db || !coll) {
254-
return;
255-
}
267+
const { database } = toNS(namespace);
268+
const db = instance.databases.get(database);
269+
const coll = db?.collections.get(namespace, '_id');
270+
271+
if (!db || !coll) {
272+
return;
273+
}
256274

257-
const isLastCollection = db.collections.length === 1;
258-
259-
if (isLastCollection) {
260-
instance.databases.remove(db);
261-
MongoDBInstance.removeAllListeners(db);
262-
} else {
263-
db.collections.remove(coll);
264-
MongoDBInstance.removeAllListeners(coll);
265-
// Update db stats to account for db stats affected by collection stats
266-
void db?.fetch({ dataService, force: true }).catch(() => {
267-
// noop, we ignore stats fetching failures
268-
});
275+
const isLastCollection = db.collections.length === 1;
276+
277+
if (isLastCollection) {
278+
instance.databases.remove(db);
279+
MongoDBInstance.removeAllListeners(db);
280+
} else {
281+
db.collections.remove(coll);
282+
MongoDBInstance.removeAllListeners(coll);
283+
// Update db stats to account for db stats affected by collection stats
284+
void db?.fetch({ dataService, force: true }).catch(() => {
285+
// noop, we ignore stats fetching failures
286+
});
287+
}
269288
}
270-
});
289+
);
271290

272291
on(globalAppRegistry, 'refresh-databases', refreshDatabases);
273292

274293
on(
275294
globalAppRegistry,
276295
'collection-renamed',
277-
({ from, to }: { from: string; to: string }) => {
296+
(
297+
{ from, to }: { from: string; to: string },
298+
{ connectionId }: { connectionId?: string } = {}
299+
) => {
300+
if (connectionId !== instanceConnectionId) {
301+
return;
302+
}
303+
278304
const { database, collection } = toNS(from);
279305
instance.databases
280306
.get(database)

packages/compass-sidebar/src/components/legacy/sidebar-databases-navigation.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,13 @@ const onNamespaceAction = (
175175
const ns = toNS(namespace);
176176
switch (action) {
177177
case 'drop-database':
178-
emit('open-drop-database', connectionId, ns.database);
178+
emit('open-drop-database', ns.database, { connectionId });
179179
return;
180180
case 'rename-collection':
181-
emit('open-rename-collection', connectionId, ns);
181+
emit('open-rename-collection', ns, { connectionId });
182182
return;
183183
case 'drop-collection':
184-
emit('open-drop-collection', connectionId, ns);
184+
emit('open-drop-collection', ns, { connectionId });
185185
return;
186186
case 'create-collection':
187187
emit('open-create-collection', ns, {

packages/compass-sidebar/src/components/multiple-connections/active-connections/active-connection-navigation.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,13 +312,13 @@ const onNamespaceAction = (
312312
const ns = toNS(namespace);
313313
switch (action) {
314314
case 'drop-database':
315-
emit('open-drop-database', connectionId, ns.database);
315+
emit('open-drop-database', ns.database, { connectionId });
316316
return;
317317
case 'rename-collection':
318-
emit('open-rename-collection', connectionId, ns);
318+
emit('open-rename-collection', ns, { connectionId });
319319
return;
320320
case 'drop-collection':
321-
emit('open-drop-collection', connectionId, ns);
321+
emit('open-drop-collection', ns, { connectionId });
322322
return;
323323
case 'create-collection':
324324
emit('open-create-collection', ns, {

packages/databases-collections/src/collections-plugin.spec.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ describe('Collections [Plugin]', function () {
6464
});
6565

6666
it('renders a list of collections', function () {
67+
expect(screen.getAllByRole('gridcell')).to.have.lengthOf(2);
68+
});
69+
70+
it('initiates action to create a collection', function () {
6771
userEvent.click(
6872
screen.getByRole('button', { name: /Create collection/ })
6973
);
@@ -74,18 +78,24 @@ describe('Collections [Plugin]', function () {
7478
// connection id is the default provided by the connectionInfoProvider
7579
{ connectionId: 'TEST' }
7680
);
81+
});
7782

83+
it('initiates action to refresh collections', function () {
7884
userEvent.click(screen.getByRole('button', { name: /Refresh/ }));
7985
// eslint-disable-next-line @typescript-eslint/unbound-method
8086
expect(mongodbInstance.databases.get('foo')?.fetchCollectionsDetails).to
8187
.have.been.called;
88+
});
8289

90+
it('initiates action to drop a collection', function () {
8391
userEvent.hover(screen.getByRole('gridcell', { name: /bar/ }));
8492
userEvent.click(screen.getByRole('button', { name: /Delete/ }));
8593
expect(appRegistry.emit).to.have.been.calledWithMatch(
8694
'open-drop-collection',
87-
'TEST',
88-
{ ns: 'foo.bar' }
95+
{ ns: 'foo.bar' },
96+
// this event is supposed to emit always with a connectionId and this
97+
// connection id is the default provided by the connectionInfoProvider
98+
{ connectionId: 'TEST' }
8999
);
90100
});
91101

packages/databases-collections/src/components/rename-collection-modal/rename-collection-modal.spec.tsx

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,14 @@ describe('RenameCollectionModal [Component]', function () {
5151
pipelineStorage: pipelineStorage as any,
5252
});
5353
render(<Plugin> </Plugin>);
54-
appRegistry.emit('open-rename-collection', '12345', {
55-
database: 'foo',
56-
collection: 'bar',
57-
});
54+
appRegistry.emit(
55+
'open-rename-collection',
56+
{
57+
database: 'foo',
58+
collection: 'bar',
59+
},
60+
{ connectionId: '12345' }
61+
);
5862

5963
await waitFor(() => screen.getByText('Rename collection'));
6064
});
@@ -171,10 +175,14 @@ describe('RenameCollectionModal [Component]', function () {
171175
pipelineStorage: pipelineStorage as any,
172176
});
173177
render(<Plugin> </Plugin>);
174-
appRegistry.emit('open-rename-collection', '1234', {
175-
database: 'foo',
176-
collection: 'bar',
177-
});
178+
appRegistry.emit(
179+
'open-rename-collection',
180+
{
181+
database: 'foo',
182+
collection: 'bar',
183+
},
184+
{ connectionId: '12345' }
185+
);
178186

179187
await waitFor(() => screen.getByText('Rename collection'));
180188

0 commit comments

Comments
 (0)