From d8dd500a9f46d00ea5b59de5dd31fd22b92c8a76 Mon Sep 17 00:00:00 2001 From: Alexander Czigler <3116043+alexanderczigler@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:24:24 +0100 Subject: [PATCH 1/6] test(subscriber): verify that createSubscription is called --- packages/pubsub/src/lib/subscriber.spec.ts | 28 +++++++++++++++++----- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/pubsub/src/lib/subscriber.spec.ts b/packages/pubsub/src/lib/subscriber.spec.ts index 1c52b066..5ea6410f 100644 --- a/packages/pubsub/src/lib/subscriber.spec.ts +++ b/packages/pubsub/src/lib/subscriber.spec.ts @@ -1,6 +1,6 @@ import { PubSub, type Subscription, type Topic } from '@google-cloud/pubsub' import type { Schema } from 'avsc' -import { type MockedObject, afterAll, describe, expect, it, vi } from 'vitest' +import { type MockedObject, describe, expect, it, vi } from 'vitest' import { createSubscriber } from './subscriber' type ExampleMessage = { @@ -8,11 +8,6 @@ type ExampleMessage = { message: string } -const message = { - messageType: 'type of message', - message: 'message data', -} satisfies ExampleMessage - type ExamplePubsubChannels = { example: ExampleMessage } @@ -98,4 +93,25 @@ describe('subscriber', () => { expect(topicMock.subscription).toHaveBeenCalled() expect(topicMock.createSubscription.mock.calls.length).toBe(0) }) + + it('created a subscription if it does not exist', async () => { + const topicMock = new PubSub().topic(topicName) as MockedObject + const subscriptionMock = topicMock.subscription( + subscriptionName + ) as MockedObject + + subscriptionMock.exists.mockImplementation(() => [false]) + + const subscriber = createSubscriber({ + projectId: 'test', + }) + + topicMock.createSubscription.mockClear() + + await subscriber.topic('example').subscribe('example-subscription', { + onMessage: () => Promise.resolve(), + }) + + expect(topicMock.createSubscription).toHaveBeenCalled() + }) }) From 89f5259f8836b4fcf43b33dce7cfea62f0bae2b6 Mon Sep 17 00:00:00 2001 From: Alexander Czigler <3116043+alexanderczigler@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:24:52 +0100 Subject: [PATCH 2/6] fix(subscriber): var name --- packages/pubsub/src/lib/subscriber.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pubsub/src/lib/subscriber.ts b/packages/pubsub/src/lib/subscriber.ts index 0b112e4a..134c45db 100644 --- a/packages/pubsub/src/lib/subscriber.ts +++ b/packages/pubsub/src/lib/subscriber.ts @@ -7,7 +7,7 @@ import { type Topic, } from '@google-cloud/pubsub' -const makeSureSubacriptionExists = async ( +const makeSureSubscriptionExists = async ( topic: Topic, name: string, options?: PubSubOptions @@ -72,7 +72,7 @@ export const createSubscriber = >( return { initiate: async (subscriptionName, options) => { - await makeSureSubacriptionExists(_topic, subscriptionName, options) + await makeSureSubscriptionExists(_topic, subscriptionName, options) }, subscribe: async (subscriptionName, callbacks, options) => { if (!_topic) { From f27205d670dee5990d6501cb1a0bc319f9aa3ce1 Mon Sep 17 00:00:00 2001 From: Alexander Czigler <3116043+alexanderczigler@users.noreply.github.com> Date: Thu, 7 Nov 2024 14:58:04 +0100 Subject: [PATCH 3/6] fix(subscriber): set topic so that initiate also works --- packages/pubsub/src/lib/subscriber.spec.ts | 73 ++++++++++++++-------- packages/pubsub/src/lib/subscriber.ts | 5 +- 2 files changed, 47 insertions(+), 31 deletions(-) diff --git a/packages/pubsub/src/lib/subscriber.spec.ts b/packages/pubsub/src/lib/subscriber.spec.ts index 5ea6410f..111a33eb 100644 --- a/packages/pubsub/src/lib/subscriber.spec.ts +++ b/packages/pubsub/src/lib/subscriber.spec.ts @@ -1,7 +1,8 @@ import { PubSub, type Subscription, type Topic } from '@google-cloud/pubsub' import type { Schema } from 'avsc' -import { type MockedObject, describe, expect, it, vi } from 'vitest' +import { type MockedObject, beforeAll, describe, expect, it, vi } from 'vitest' import { createSubscriber } from './subscriber' +import { beforeEach } from 'node:test' type ExampleMessage = { messageType: string @@ -72,46 +73,64 @@ describe('subscriber', () => { const topicName = 'example' const subscriptionName = 'example-subscription' - it('uses an existing subscription if it exists', async () => { - const topicMock = new PubSub().topic(topicName) as MockedObject - const subscriptionMock = topicMock.subscription( + let topicMock: MockedObject + let subscriptionMock: MockedObject + + beforeAll(() => { + topicMock = new PubSub().topic(topicName) as MockedObject + subscriptionMock = topicMock.subscription( subscriptionName ) as MockedObject + }) - subscriptionMock.exists.mockImplementation(() => [true]) + beforeEach(() => {}) - const subscriber = createSubscriber({ - projectId: 'test', - }) + describe('subscribe', () => { + it('uses an existing subscription if it exists', async () => { + subscriptionMock.exists.mockImplementationOnce(() => [true]) - topicMock.createSubscription.mockClear() + const subscriber = createSubscriber({ + projectId: 'test', + }) - await subscriber.topic('example').subscribe('existing-subscription', { - onMessage: () => Promise.resolve(), - }) + topicMock.createSubscription.mockClear() - expect(topicMock.subscription).toHaveBeenCalled() - expect(topicMock.createSubscription.mock.calls.length).toBe(0) + await subscriber.topic('example').subscribe('existing-subscription', { + onMessage: () => Promise.resolve(), + }) + + expect(topicMock.subscription).toHaveBeenCalled() + expect(topicMock.createSubscription.mock.calls.length).toBe(0) + }) }) - it('created a subscription if it does not exist', async () => { - const topicMock = new PubSub().topic(topicName) as MockedObject - const subscriptionMock = topicMock.subscription( - subscriptionName - ) as MockedObject + describe('initiate', () => { + it('does not create a subscription if it exists', async () => { + subscriptionMock.exists.mockImplementationOnce(() => [true]) - subscriptionMock.exists.mockImplementation(() => [false]) + const subscriber = createSubscriber({ + projectId: 'test', + }) - const subscriber = createSubscriber({ - projectId: 'test', - }) + topicMock.createSubscription.mockClear() - topicMock.createSubscription.mockClear() + await subscriber.topic('example').initiate('existing-subscription') - await subscriber.topic('example').subscribe('example-subscription', { - onMessage: () => Promise.resolve(), + expect(topicMock.createSubscription.mock.calls.length).toBe(0) }) - expect(topicMock.createSubscription).toHaveBeenCalled() + it('creates a subscription if it does not exist', async () => { + subscriptionMock.exists.mockImplementationOnce(() => [false]) + + const subscriber = createSubscriber({ + projectId: 'test', + }) + + topicMock.createSubscription.mockClear() + + await subscriber.topic('example').initiate('example-subscription') + + expect(topicMock.createSubscription).toHaveBeenCalled() + }) }) }) diff --git a/packages/pubsub/src/lib/subscriber.ts b/packages/pubsub/src/lib/subscriber.ts index 134c45db..9255a03e 100644 --- a/packages/pubsub/src/lib/subscriber.ts +++ b/packages/pubsub/src/lib/subscriber.ts @@ -68,16 +68,13 @@ export const createSubscriber = >( const typedClient: SubscriptionClient = { topic: (name) => { - let _topic: Topic + const _topic: Topic = client.topic(name as string) return { initiate: async (subscriptionName, options) => { await makeSureSubscriptionExists(_topic, subscriptionName, options) }, subscribe: async (subscriptionName, callbacks, options) => { - if (!_topic) { - _topic = client.topic(name as string) - } const subscription = _topic.subscription(subscriptionName) subscription.on('message', async (msg) => { const data = JSON.parse(msg.data.toString('utf8')) From c6b0350fd851cbb45876c89a591b7daea115ef1a Mon Sep 17 00:00:00 2001 From: Alexander Czigler <3116043+alexanderczigler@users.noreply.github.com> Date: Thu, 7 Nov 2024 14:59:41 +0100 Subject: [PATCH 4/6] refactor(formatting): linting --- packages/pubsub/src/lib/subscriber.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pubsub/src/lib/subscriber.spec.ts b/packages/pubsub/src/lib/subscriber.spec.ts index 111a33eb..66e4d3e9 100644 --- a/packages/pubsub/src/lib/subscriber.spec.ts +++ b/packages/pubsub/src/lib/subscriber.spec.ts @@ -1,8 +1,8 @@ +import { beforeEach } from 'node:test' import { PubSub, type Subscription, type Topic } from '@google-cloud/pubsub' import type { Schema } from 'avsc' import { type MockedObject, beforeAll, describe, expect, it, vi } from 'vitest' import { createSubscriber } from './subscriber' -import { beforeEach } from 'node:test' type ExampleMessage = { messageType: string From a39b6d0cb5af93343809630d9be6f52ab55c19f8 Mon Sep 17 00:00:00 2001 From: Alexander Czigler <3116043+alexanderczigler@users.noreply.github.com> Date: Thu, 7 Nov 2024 15:01:03 +0100 Subject: [PATCH 5/6] chore: create changeset --- .changeset/quiet-papayas-greet.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/quiet-papayas-greet.md diff --git a/.changeset/quiet-papayas-greet.md b/.changeset/quiet-papayas-greet.md new file mode 100644 index 00000000..c0e00ffd --- /dev/null +++ b/.changeset/quiet-papayas-greet.md @@ -0,0 +1,5 @@ +--- +"@sebspark/pubsub": patch +--- + +Fixed bug that broke topic.initiate, leading to subscription creation failing. From 7fc19cb2585be56cd4f040dfe77c29562e6fba94 Mon Sep 17 00:00:00 2001 From: Alexander Czigler <3116043+alexanderczigler@users.noreply.github.com> Date: Thu, 7 Nov 2024 15:14:41 +0100 Subject: [PATCH 6/6] test(subscriber): remove interfering mock implementation --- packages/pubsub/src/lib/subscriber.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/pubsub/src/lib/subscriber.spec.ts b/packages/pubsub/src/lib/subscriber.spec.ts index 66e4d3e9..153b6f1b 100644 --- a/packages/pubsub/src/lib/subscriber.spec.ts +++ b/packages/pubsub/src/lib/subscriber.spec.ts @@ -87,8 +87,6 @@ describe('subscriber', () => { describe('subscribe', () => { it('uses an existing subscription if it exists', async () => { - subscriptionMock.exists.mockImplementationOnce(() => [true]) - const subscriber = createSubscriber({ projectId: 'test', })