Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/data-connect/data-connect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class DataConnectService {
}

getDataConnect(connectorConfig: ConnectorConfig): DataConnect {
const id = `${connectorConfig.location}-${connectorConfig.serviceId}`;
const id = `${connectorConfig.location}-${connectorConfig.serviceId}-${connectorConfig.connector ?? ''}`;
const dc = this.dataConnectInstances.get(id);
if (typeof dc !== 'undefined') {
return dc;
Expand Down
109 changes: 109 additions & 0 deletions test/unit/data-connect/data-connect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,112 @@ describe('DataConnect', () => {
});
});
});

describe('getDataConnect() caching', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you rename this section just 'getDataConnect()'?

const mockOptions = {
credential: new mocks.MockCredential(),
projectId: 'test-project',
};
let mockApp: FirebaseApp;
let getAppStub: sinon.SinonStub;

beforeEach(() => {
mockApp = mocks.appWithOptions(mockOptions);
getAppStub = sinon.stub(appIndex, 'getApp').returns(mockApp);
});

afterEach(() => {
getAppStub.restore();
return mockApp.delete();
});

describe('should cache DataConnect instances correctly', () => {
it('should return the same instance for identical connector configs', () => {
const config: ConnectorConfig = {
location: 'us-west2',
serviceId: 'my-service',
connector: 'my-connector',
};

const dc1 = getDataConnect(config);
const dc2 = getDataConnect(config);

expect(dc1).to.equal(dc2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably use .to.deep.equal when checking DataConnect instances

});

it('should return different instances for different connectors with same location and serviceId', () => {
const config1: ConnectorConfig = {
location: 'us-west2',
serviceId: 'my-service',
connector: 'connector-a',
};
const config2: ConnectorConfig = {
location: 'us-west2',
serviceId: 'my-service',
connector: 'connector-b',
};

const dc1 = getDataConnect(config1);
const dc2 = getDataConnect(config2);

expect(dc1).to.not.equal(dc2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above - .to.not.deep.equal

expect(dc1.connectorConfig.connector).to.equal('connector-a');
expect(dc2.connectorConfig.connector).to.equal('connector-b');
});

it('should return different instances for different locations', () => {
const config1: ConnectorConfig = {
location: 'us-west2',
serviceId: 'my-service',
connector: 'my-connector',
};
const config2: ConnectorConfig = {
location: 'us-east1',
serviceId: 'my-service',
connector: 'my-connector',
};

const dc1 = getDataConnect(config1);
const dc2 = getDataConnect(config2);

expect(dc1).to.not.equal(dc2);
});

it('should return different instances for different serviceIds', () => {
const config1: ConnectorConfig = {
location: 'us-west2',
serviceId: 'service-a',
connector: 'my-connector',
};
const config2: ConnectorConfig = {
location: 'us-west2',
serviceId: 'service-b',
connector: 'my-connector',
};

const dc1 = getDataConnect(config1);
const dc2 = getDataConnect(config2);

expect(dc1).to.not.equal(dc2);
});

it('should handle connector being undefined vs defined', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you make this more descriptive? something like "should consider configs with connector undefined and defined as different"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a test here to make sure the difference between empty connector: "" and no defined value for connector is picked up on by the code

const configWithConnector: ConnectorConfig = {
location: 'us-west2',
serviceId: 'my-service',
connector: 'my-connector',
};
const configWithoutConnector: ConnectorConfig = {
location: 'us-west2',
serviceId: 'my-service',
};

const dc1 = getDataConnect(configWithConnector);
const dc2 = getDataConnect(configWithoutConnector);

expect(dc1).to.not.equal(dc2);
expect(dc1.connectorConfig.connector).to.equal('my-connector');
expect(dc2.connectorConfig.connector).to.be.undefined;
});
});
});