Avoid redundant updates if changed attributes are a strict subset of existing attributes

packages/core/index.ts

@@ -31,7 +31,7 @@ export {
   MessageInvalidFormatError,
 } from './lib/errors/Errors'
 
-export { shallowEqual } from './lib/utils/matchUtils'
+export { isShallowSubset, objectMatches } from './lib/utils/matchUtils'
 
 export {
   HandlerContainer,

packages/core/lib/utils/matchUtils.ts

@@ -1,36 +1,41 @@
 import { deepEqual } from 'fast-equals'
 
-export function shallowEqual(
-  object1?: Record<string, unknown>,
-  object2?: Record<string, unknown>,
+/**
+ * Returns true if `maybeSubset` does not contain any fields in addition to the fields that `maybeSuperset` contain, and all of the overlapping fields are equal on a shallow level.
+ */
+export function isShallowSubset(
+  maybeSubset?: Record<string, unknown>,
+  maybeSuperset?: Record<string, unknown>,
 ): boolean {
-  if ((object1 && !object2) || (!object1 && object2)) {
-    return false
-  }
-  if (!object1 && !object2) {
+  if (!maybeSubset) {
     return true
   }
 
+  const keysSubset = Object.keys(maybeSubset)
   // eslint-disable-next-line  @typescript-eslint/no-non-null-assertion
-  const keys1 = Object.keys(object1!)
-  // eslint-disable-next-line  @typescript-eslint/no-non-null-assertion
-  const keys2 = Object.keys(object2!)
+  const keysSuperset = Object.keys(maybeSuperset!)
 
-  if (keys1.length !== keys2.length) {
+  if (keysSubset.length === 0) {
+    return true
+  }
+  if (keysSubset.length > keysSuperset.length) {
     return false
   }
 
   // eslint-disable-next-line prefer-const
-  for (let key of keys1) {
+  for (let key of keysSubset) {
     // eslint-disable-next-line  @typescript-eslint/no-non-null-assertion
-    if (object1![key] !== object2![key]) {
+    if (maybeSubset[key] !== maybeSuperset![key]) {
       return false
     }
   }
 
   return true
 }
 
+/**
+ * Returns true if `validatedObject` contains all of the fields included on `matcher`, and their values are deeply equal
+ */
 export function objectMatches(
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   matcher: Record<string, any>,

packages/core/test/utils/matchUtils.spec.ts

@@ -0,0 +1,49 @@
+import { describe } from 'vitest'
+
+import { isShallowSubset } from '../../lib/utils/matchUtils'
+
+describe('matchUtils', () => {
+  describe('isShallowSubset', () => {
+    it('returns true for exact match', () => {
+      const set1 = { a: 'a', b: 1 }
+      const set2 = { a: 'a', b: 1 }
+
+      expect(isShallowSubset(set1, set2)).toBe(true)
+    })
+
+    it('returns true for same fields with different values', () => {
+      const set1 = { a: 'a', b: 1 }
+      const set2 = { a: 'a', b: 2 }
+
+      expect(isShallowSubset(set1, set2)).toBe(false)
+    })
+
+    it('returns true for a subset', () => {
+      const set1 = { a: 'a' }
+      const set2 = { a: 'a', b: 2 }
+
+      expect(isShallowSubset(set1, set2)).toBe(true)
+    })
+
+    it('returns false for a superset', () => {
+      const set1 = { a: 'a', b: 2 }
+      const set2 = { a: 'a' }
+
+      expect(isShallowSubset(set1, set2)).toBe(false)
+    })
+
+    it('returns true for empty set', () => {
+      const set1 = {}
+      const set2 = { a: 'a', b: 2 }
+
+      expect(isShallowSubset(set1, set2)).toBe(true)
+    })
+
+    it('returns true for undefined', () => {
+      const set1 = undefined
+      const set2 = { a: 'a', b: 2 }
+
+      expect(isShallowSubset(set1, set2)).toBe(true)
+    })
+  })
+})

packages/sqs/lib/utils/sqsUtils.ts

@@ -8,7 +8,7 @@ import {
 } from '@aws-sdk/client-sqs'
 import type { CreateQueueCommandInput, SQSClient, QueueAttributeName } from '@aws-sdk/client-sqs'
 import type { Either } from '@lokalise/node-core'
-import { shallowEqual, waitAndRetry } from '@message-queue-toolkit/core'
+import { isShallowSubset, waitAndRetry } from '@message-queue-toolkit/core'
 
 import type { ExtraSQSCreationParams } from '../sqs/AbstractSqsConsumer'
 import type { SQSQueueLocatorType } from '../sqs/AbstractSqsService'
@@ -107,8 +107,8 @@ export async function assertQueue(
         throw new Error('Queue ARN was not set')
       }
 
-      if (shallowEqual(existingAttributes.result, queueConfig.Attributes)) {
-      } else {
+      // Only perform update if there are new or changed values in the queue config
+      if (!isShallowSubset(queueConfig.Attributes, existingAttributes.result.attributes)) {
         await updateQueueAttributes(sqsClient, queueUrl, queueConfig.Attributes)
       }
 

packages/sqs/test/consumers/SqsPermissionsConsumerMultiSchema.spec.ts

@@ -80,17 +80,67 @@ describe('SqsPermissionsConsumerMultiSchema', () => {
         logMessages: true,
       })
 
+      const sqsSpy = vi.spyOn(sqsClient, 'send')
+
       await newConsumer.init()
       expect(newConsumer.queueUrl).toBe(
         'http://sqs.eu-west-1.localstack:4566/000000000000/existingQueue',
       )
 
+      const updateCall = sqsSpy.mock.calls.find((entry) => {
+        return entry[0].constructor.name === 'SetQueueAttributesCommand'
+      })
+      expect(updateCall).toBeDefined()
+
       const attributes = await getQueueAttributes(sqsClient, {
         queueUrl: newConsumer.queueUrl,
       })
 
       expect(attributes.result?.attributes!.KmsMasterKeyId).toBe('othervalue')
     })
+
+    it('does not update existing queue when attributes did not change', async () => {
+      await assertQueue(sqsClient, {
+        QueueName: 'existingQueue',
+        Attributes: {
+          KmsMasterKeyId: 'somevalue',
+        },
+      })
+
+      const newConsumer = new SqsPermissionConsumerMultiSchema(diContainer.cradle, {
+        creationConfig: {
+          queue: {
+            QueueName: 'existingQueue',
+            Attributes: {
+              KmsMasterKeyId: 'somevalue',
+            },
+          },
+          updateAttributesIfExists: true,
+        },
+        deletionConfig: {
+          deleteIfExists: false,
+        },
+        logMessages: true,
+      })
+
+      const sqsSpy = vi.spyOn(sqsClient, 'send')
+
+      await newConsumer.init()
+      expect(newConsumer.queueUrl).toBe(
+        'http://sqs.eu-west-1.localstack:4566/000000000000/existingQueue',
+      )
+
+      const updateCall = sqsSpy.mock.calls.find((entry) => {
+        return entry[0].constructor.name === 'SetQueueAttributesCommand'
+      })
+      expect(updateCall).toBeUndefined()
+
+      const attributes = await getQueueAttributes(sqsClient, {
+        queueUrl: newConsumer.queueUrl,
+      })
+
+      expect(attributes.result?.attributes!.KmsMasterKeyId).toBe('somevalue')
+    })
   })
 
   describe('logging', () => {