Skip to content

Commit dbc10e1

Browse files
committed
Address review comments
1 parent 2b833a8 commit dbc10e1

7 files changed

Lines changed: 62 additions & 69 deletions

File tree

packages/core/lib/types/queueOptionsTypes.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,14 +145,13 @@ export type DeletionConfig = {
145145
* - Service B needs to subscribe to Service A's topic
146146
* - Neither can deploy first without the other's topic existing
147147
*
148-
* With `resourceAvailabilityConfig.enabled = true`, the consumer will poll for the
148+
* When `resourceAvailabilityConfig` is provided, the consumer will poll for the
149149
* topic/queue to become available instead of failing immediately.
150150
*
151151
* @example
152152
* // Development/staging - poll indefinitely
153153
* {
154154
* resourceAvailabilityConfig: {
155-
* enabled: true,
156155
* pollingIntervalMs: 5000,
157156
* }
158157
* }
@@ -161,19 +160,27 @@ export type DeletionConfig = {
161160
* // Production - poll with timeout to catch misconfigurations
162161
* {
163162
* resourceAvailabilityConfig: {
164-
* enabled: true,
165163
* timeoutMs: 5 * 60 * 1000, // 5 minutes
166164
* pollingIntervalMs: 10000,
167165
* }
168166
* }
167+
*
168+
* @example
169+
* // Temporarily disable without removing config
170+
* {
171+
* resourceAvailabilityConfig: {
172+
* enabled: false,
173+
* timeoutMs: 5 * 60 * 1000,
174+
* }
175+
* }
169176
*/
170177
export type ResourceAvailabilityConfig = {
171178
/**
172-
* If true, the consumer will poll for the topic/queue to become available
173-
* instead of failing immediately when using locatorConfig.
174-
* Default: false (fail immediately for backwards compatibility)
179+
* Controls whether polling is enabled.
180+
* Default: true (when resourceAvailabilityConfig is provided)
181+
* Set to false to temporarily disable polling without removing the config.
175182
*/
176-
enabled: boolean
183+
enabled?: boolean
177184

178185
/**
179186
* Maximum time in milliseconds to wait for the resource to become available.

packages/core/lib/utils/resourceAvailabilityUtils.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ describe('resourceAvailabilityUtils', () => {
1111
expect(isResourceAvailabilityWaitingEnabled({ enabled: true })).toBe(true)
1212
})
1313

14+
it('returns true when enabled is not specified (defaults to true)', () => {
15+
expect(isResourceAvailabilityWaitingEnabled({})).toBe(true)
16+
expect(isResourceAvailabilityWaitingEnabled({ pollingIntervalMs: 1000 })).toBe(true)
17+
expect(isResourceAvailabilityWaitingEnabled({ timeoutMs: 5000 })).toBe(true)
18+
})
19+
1420
it('returns false when enabled is false', () => {
1521
expect(isResourceAvailabilityWaitingEnabled({ enabled: false })).toBe(false)
1622
})

packages/core/lib/utils/resourceAvailabilityUtils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,11 @@ export async function waitForResource<T>(options: WaitForResourceOptions<T>): Pr
156156
}
157157

158158
/**
159-
* Helper to check if resource availability waiting is enabled
159+
* Helper to check if resource availability waiting is enabled.
160+
* Returns true when config is provided and enabled is not explicitly false.
160161
*/
161162
export function isResourceAvailabilityWaitingEnabled(
162163
config?: ResourceAvailabilityConfig,
163164
): config is ResourceAvailabilityConfig {
164-
return config?.enabled === true
165+
return config != null && config.enabled !== false
165166
}

packages/sns/lib/utils/snsInitter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export async function initSnsSqs(
151151
throw new Error(`Queue with queueUrl ${queueUrl} does not exist.`)
152152
}
153153
if (topicCheckResult?.error === 'not_found') {
154-
throw new Error(`Topic with topicArn ${locatorConfig.topicArn} does not exist.`)
154+
throw new Error(`Topic with topicArn ${subscriptionTopicArn} does not exist.`)
155155
}
156156
}
157157

@@ -288,7 +288,7 @@ export async function initSns(
288288
// Original behavior: check once and fail immediately if not found
289289
const checkResult = await getTopicAttributes(snsClient, topicArn)
290290
if (checkResult.error === 'not_found') {
291-
throw new Error(`Topic with topicArn ${locatorConfig.topicArn} does not exist.`)
291+
throw new Error(`Topic with topicArn ${topicArn} does not exist.`)
292292
}
293293
}
294294

packages/sns/test/consumers/SnsSqsPermissionConsumer.resourceAvailability.spec.ts

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,18 @@ import { setTimeout } from 'node:timers/promises'
22
import type { SNSClient } from '@aws-sdk/client-sns'
33
import type { SQSClient } from '@aws-sdk/client-sqs'
44
import type { STSClient } from '@aws-sdk/client-sts'
5-
import { ResourceAvailabilityTimeoutError } from '@message-queue-toolkit/core'
5+
import {
6+
MessageHandlerConfigBuilder,
7+
ResourceAvailabilityTimeoutError,
8+
} from '@message-queue-toolkit/core'
69
import { assertQueue, deleteQueue } from '@message-queue-toolkit/sqs'
710
import type { AwilixContainer } from 'awilix'
811
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'
9-
import type { SNSSQSConsumerDependencies } from '../../lib/sns/AbstractSnsSqsConsumer.ts'
10-
import { AbstractSnsSqsConsumer } from '../../lib/sns/AbstractSnsSqsConsumer.ts'
12+
import {
13+
AbstractSnsSqsConsumer,
14+
type SNSSQSConsumerDependencies,
15+
type SNSSQSConsumerOptions,
16+
} from '../../lib/sns/AbstractSnsSqsConsumer.ts'
1117
import { assertTopic, deleteTopic } from '../../lib/utils/snsUtils.ts'
1218
import type { Dependencies } from '../utils/testContext.ts'
1319
import { registerDependencies } from '../utils/testContext.ts'
@@ -16,42 +22,24 @@ import {
1622
type PERMISSIONS_ADD_MESSAGE_TYPE,
1723
} from './userConsumerSchemas.ts'
1824

25+
type TestConsumerOptions = Pick<
26+
SNSSQSConsumerOptions<PERMISSIONS_ADD_MESSAGE_TYPE, undefined, undefined>,
27+
'locatorConfig' | 'creationConfig' | 'resourceAvailabilityConfig'
28+
>
29+
1930
// Simple consumer for testing resource availability
2031
class TestResourceAvailabilityConsumer extends AbstractSnsSqsConsumer<
2132
PERMISSIONS_ADD_MESSAGE_TYPE,
2233
undefined,
2334
undefined
2435
> {
25-
constructor(
26-
dependencies: SNSSQSConsumerDependencies,
27-
options: {
28-
locatorConfig?: {
29-
topicArn?: string
30-
topicName?: string
31-
queueUrl?: string
32-
queueName?: string
33-
subscriptionArn?: string
34-
}
35-
creationConfig?: {
36-
queue?: { QueueName: string }
37-
topic?: { Name: string }
38-
}
39-
resourceAvailabilityConfig?: {
40-
enabled: boolean
41-
timeoutMs?: number
42-
pollingIntervalMs?: number
43-
}
44-
},
45-
) {
36+
constructor(dependencies: SNSSQSConsumerDependencies, options: TestConsumerOptions) {
4637
super(
4738
dependencies,
4839
{
49-
handlers: [
50-
{
51-
schema: PERMISSIONS_ADD_MESSAGE_SCHEMA,
52-
handler: () => Promise.resolve({ result: 'success' }),
53-
},
54-
],
40+
handlers: new MessageHandlerConfigBuilder<PERMISSIONS_ADD_MESSAGE_TYPE, undefined>()
41+
.addConfig(PERMISSIONS_ADD_MESSAGE_SCHEMA, () => Promise.resolve({ result: 'success' }))
42+
.build(),
5543
messageTypeResolver: { messageTypePath: 'messageType' },
5644
subscriptionConfig: { updateAttributesIfExists: false },
5745
...options,

packages/sqs/lib/utils/sqsInitter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ export async function initSqs(
108108
const checkResult = await getQueueAttributes(sqsClient, queueUrl, ['QueueArn'])
109109

110110
if (checkResult.error === 'not_found') {
111-
throw new Error(`Queue with queueUrl ${locatorConfig.queueUrl} does not exist.`)
111+
throw new Error(`Queue with queueUrl ${queueUrl} does not exist.`)
112112
}
113113

114114
queueArn = checkResult.result?.attributes?.QueueArn

packages/sqs/test/consumers/SqsPermissionConsumer.resourceAvailability.spec.ts

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import { setTimeout } from 'node:timers/promises'
22
import type { SQSClient } from '@aws-sdk/client-sqs'
3-
import { ResourceAvailabilityTimeoutError } from '@message-queue-toolkit/core'
3+
import {
4+
MessageHandlerConfigBuilder,
5+
ResourceAvailabilityTimeoutError,
6+
} from '@message-queue-toolkit/core'
47
import type { AwilixContainer } from 'awilix'
58
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
6-
7-
import type { SQSConsumerDependencies } from '../../lib/sqs/AbstractSqsConsumer.ts'
8-
import { AbstractSqsConsumer } from '../../lib/sqs/AbstractSqsConsumer.ts'
9+
import {
10+
AbstractSqsConsumer,
11+
type SQSConsumerDependencies,
12+
type SQSConsumerOptions,
13+
} from '../../lib/sqs/AbstractSqsConsumer.ts'
914
import { assertQueue, deleteQueue } from '../../lib/utils/sqsUtils.ts'
1015
import type { Dependencies } from '../utils/testContext.ts'
1116
import { registerDependencies } from '../utils/testContext.ts'
@@ -14,38 +19,24 @@ import {
1419
type PERMISSIONS_ADD_MESSAGE_TYPE,
1520
} from './userConsumerSchemas.ts'
1621

22+
type TestConsumerOptions = Pick<
23+
SQSConsumerOptions<PERMISSIONS_ADD_MESSAGE_TYPE, undefined, undefined>,
24+
'locatorConfig' | 'creationConfig' | 'resourceAvailabilityConfig'
25+
>
26+
1727
// Simple consumer for testing resource availability
1828
class TestResourceAvailabilityConsumer extends AbstractSqsConsumer<
1929
PERMISSIONS_ADD_MESSAGE_TYPE,
2030
undefined,
2131
undefined
2232
> {
23-
constructor(
24-
dependencies: SQSConsumerDependencies,
25-
options: {
26-
locatorConfig?: {
27-
queueUrl?: string
28-
queueName?: string
29-
}
30-
creationConfig?: {
31-
queue?: { QueueName: string }
32-
}
33-
resourceAvailabilityConfig?: {
34-
enabled: boolean
35-
timeoutMs?: number
36-
pollingIntervalMs?: number
37-
}
38-
},
39-
) {
33+
constructor(dependencies: SQSConsumerDependencies, options: TestConsumerOptions) {
4034
super(
4135
dependencies,
4236
{
43-
handlers: [
44-
{
45-
schema: PERMISSIONS_ADD_MESSAGE_SCHEMA,
46-
handler: () => Promise.resolve({ result: 'success' }),
47-
},
48-
],
37+
handlers: new MessageHandlerConfigBuilder<PERMISSIONS_ADD_MESSAGE_TYPE, undefined>()
38+
.addConfig(PERMISSIONS_ADD_MESSAGE_SCHEMA, () => Promise.resolve({ result: 'success' }))
39+
.build(),
4940
messageTypeResolver: { messageTypePath: 'messageType' },
5041
...options,
5142
},

0 commit comments

Comments
 (0)