Skip to content

Commit

Permalink
fix: always fill the topic and sub names when creating from a PubSub …
Browse files Browse the repository at this point in the history
…object (#1816)
  • Loading branch information
feywind committed Sep 8, 2023
1 parent dedfdea commit ddf8b8a
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 10 deletions.
19 changes: 19 additions & 0 deletions src/pubsub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,17 @@ export class PubSub {
return;
}
subscription.metadata = resp!;

// If this is the first call we've made, the projectId might be empty still.
if (subscription.name?.includes(PROJECT_ID_PLACEHOLDER)) {
if (subscription.metadata && subscription.metadata.name) {
subscription.name = Subscription.formatName_(
this.projectId,
subscription.metadata.name
);
}
}

callback!(null, subscription, resp!);
}
);
Expand Down Expand Up @@ -655,6 +666,14 @@ export class PubSub {
return;
}
topic.metadata = resp!;

// If this is the first call we've made, the projectId might be empty still.
if (topic.name?.includes(PROJECT_ID_PLACEHOLDER)) {
if (topic.metadata && topic.metadata.name) {
topic.name = Topic.formatName_(this.projectId, topic.metadata.name);
}
}

callback!(null, topic, resp!);
}
);
Expand Down
92 changes: 82 additions & 10 deletions test/pubsub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const PKG = require('../../package.json');
const sandbox = sinon.createSandbox();

const fakeCreds = {} as gax.grpc.ChannelCredentials;
sandbox.stub(gax.grpc.credentials, 'createInsecure').returns(fakeCreds);

const subscriptionCached = subby.Subscription;

Expand All @@ -49,6 +48,11 @@ function Subscription(
return new overrideFn(pubsub, name, options);
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
(Subscription as any).formatName_ = (): string => {
return 'formatted';
};

let promisified = false;
const fakeUtil = Object.assign({}, util, {
promisifySome(
Expand Down Expand Up @@ -92,6 +96,10 @@ class FakeTopic {
constructor(...args: Array<{}>) {
this.calledWith_ = args;
}

static formatName_(): string {
return 'foo';
}
}

let extended = false;
Expand Down Expand Up @@ -187,6 +195,11 @@ describe('PubSub', () => {
googleAuthOverride = null;
pubsub = new PubSub(OPTIONS);
pubsub.projectId = PROJECT_ID;
sandbox.stub(gax.grpc.credentials, 'createInsecure').returns(fakeCreds);
});

afterEach(() => {
sandbox.restore();
});

describe('instantiation', () => {
Expand Down Expand Up @@ -554,13 +567,15 @@ describe('PubSub', () => {

it('should return Subscription & resp to the callback', done => {
const subscription = {};
pubsub.subscription = () => {
sandbox.stub(pubsub, 'subscription').callsFake(() => {
return subscription as subby.Subscription;
};
});

pubsub.request = (config, callback: Function) => {
callback(null, apiResponse);
};
sandbox
.stub(pubsub, 'request')
.callsFake((config, callback: Function) => {
callback(null, apiResponse);
});

function callback(
err?: Error | null,
Expand All @@ -575,6 +590,31 @@ describe('PubSub', () => {

pubsub.createSubscription?.(TOPIC_NAME, SUB_NAME, callback);
});

it('should fill the subscription object name if projectId was empty', async () => {
const subscription = {};
pubsub.projectId = undefined;
sandbox.stub(pubsub, 'subscription').callsFake(() => {
// Simulate the project ID not being resolved.
const sub = subscription as subby.Subscription;
sub.name = '{{projectId}}/foo/bar';
return sub;
});

sandbox
.stub(pubsub, 'request')
.callsFake((config, callback: Function) => {
callback(null, apiResponse);
});

const [sub, resp] = await pubsub.createSubscription!(
TOPIC_NAME,
SUB_NAME
)!;
assert.strictEqual(sub, subscription);
assert.strictEqual(sub.name.includes('{{'), false);
assert.strictEqual(resp, apiResponse);
});
});
});

Expand Down Expand Up @@ -625,12 +665,17 @@ describe('PubSub', () => {
});

describe('success', () => {
const apiResponse = {};
const apiResponse = {
name: 'new-topic',
};
let requestStub: sinon.SinonStub<unknown[], unknown>;

beforeEach(() => {
pubsub.request = (config, callback: Function) => {
callback(null, apiResponse);
};
requestStub = sandbox
.stub(pubsub, 'request')
.callsFake((config, callback: Function) => {
callback(null, apiResponse);
});
});

it('should return a Topic object', done => {
Expand All @@ -656,6 +701,33 @@ describe('PubSub', () => {
done();
});
});

it('should fill the topic object name if projectId was empty', async () => {
const topicName = 'new-topic';
const topicInstance = {};

sandbox.stub(pubsub, 'topic').callsFake(name => {
assert.strictEqual(name, topicName);

// Simulate the project ID not being resolved.
const topic = topicInstance as Topic;
topic.name = 'projects/{{projectId}}/topics/new-topic';
return topic;
});

requestStub.restore();
sandbox
.stub(pubsub, 'request')
.callsFake((config, callback: Function) => {
pubsub.projectId = 'projectId';
callback(null, apiResponse);
});

const [topic, resp] = await pubsub.createTopic!(topicName)!;
assert.strictEqual(topic, topicInstance);
assert.strictEqual(topic.name.includes('{{'), false);
assert.strictEqual(resp, apiResponse);
});
});
});

Expand Down

0 comments on commit ddf8b8a

Please sign in to comment.