Fix: remove failed queue attributes from cache (#1491) (#1510)

* fix: remove failed queue attributes from cache



* add SqsTemplateTests test cases



* update shouldCacheSuccessfulQueueAttributesWithAttributeNames test case comment



* update shouldRemoveFailedQueueAttributesFromCache test case comment



---------


(cherry picked from commit fe9f347)

Signed-off-by: LeeHyungGeol <[email protected]>
Co-authored-by: 이형걸 <[email protected]>

Author: tomazfernandes

spring-cloud-aws-sqs/src/main/java/io/awspring/cloud/sqs/operations/SqsTemplate.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2023 the original author or authors.
+ * Copyright 2013-2025 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -73,6 +73,7 @@
  *
  * @author Tomaz Fernandes
  * @author Zhong Xi Lu
+ * @author Hyunggeol Lee
  *
  * @since 3.0
  */
@@ -470,8 +471,18 @@ private boolean isSkipAttribute(MessageSystemAttributeName name) {
 	}
 
 	private CompletableFuture<QueueAttributes> getQueueAttributes(String endpointName) {
-		return this.queueAttributesCache.computeIfAbsent(endpointName,
-				newName -> doGetQueueAttributes(endpointName, newName));
+		CompletableFuture<QueueAttributes> future = this.queueAttributesCache.computeIfAbsent(endpointName,
+			newName -> doGetQueueAttributes(endpointName, newName));
+
+		// Remove failed futures from cache
+		future.whenComplete((result, throwable) -> {
+			if (throwable != null) {
+				this.queueAttributesCache.remove(endpointName);
+				logger.debug("Removed failed queue attributes from cache for: {}", endpointName);
+			}
+		});
+
+		return future;
 	}
 
 	private CompletableFuture<QueueAttributes> doGetQueueAttributes(String endpointName, String newName) {

spring-cloud-aws-sqs/src/test/java/io/awspring/cloud/sqs/operations/SqsTemplateObservationTest.java

@@ -15,6 +15,7 @@
  */
 package io.awspring.cloud.sqs.operations;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
@@ -40,6 +41,7 @@
  * Tests for {@link SqsTemplate} observation support.
  *
  * @author Tomaz Fernandes
+ * @author Hyunggeol Lee
  */
 class SqsTemplateObservationTest {
 
@@ -217,4 +219,42 @@ protected KeyValues getCustomLowCardinalityKeyValues(SqsTemplateObservation.Cont
 				.hasLowCardinalityKeyValue("messaging.destination.kind", "queue")
 				.hasHighCardinalityKeyValue("messaging.message.id", messageId.toString());
 	}
+
+	@Test
+	void shouldCreateSeparateObservationsForRetryAfterCacheFailure() {
+		// Given - First attempt will fail
+		String payload = "test-payload";
+		RuntimeException firstException = new RuntimeException("First attempt - queue not found");
+
+		given(mockSqsAsyncClient.getQueueUrl(any(GetQueueUrlRequest.class)))
+			.willReturn(CompletableFuture.failedFuture(firstException));
+
+		// When - First attempt (failure)
+		try {
+			sqsTemplate.send(queueName, payload);
+		}
+		catch (Exception e) {
+			// Expected
+		}
+
+		// Then - Verify first observation has error
+		TestObservationRegistryAssert.then(observationRegistry).hasNumberOfObservationsEqualTo(1)
+			.hasSingleObservationThat().hasError();
+
+		// Given - Second attempt will succeed (cache was cleared)
+		UUID messageId = UUID.randomUUID();
+		given(mockSqsAsyncClient.getQueueUrl(any(GetQueueUrlRequest.class))).willReturn(
+			CompletableFuture.completedFuture(GetQueueUrlResponse.builder().queueUrl(queueName).build()));
+
+		given(mockSqsAsyncClient.sendMessage(any(SendMessageRequest.class))).willReturn(CompletableFuture
+			.completedFuture(SendMessageResponse.builder().messageId(messageId.toString()).build()));
+
+		// When - Second attempt (success)
+		SendResult<String> result = sqsTemplate.send(queueName, payload);
+
+		// Then - Two separate observations should be created
+		TestObservationRegistryAssert.then(observationRegistry).hasNumberOfObservationsEqualTo(2);
+
+		assertThat(result).isNotNull();
+	}
 }

spring-cloud-aws-sqs/src/test/java/io/awspring/cloud/sqs/operations/SqsTemplateTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2023 the original author or authors.
+ * Copyright 2013-2025 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -50,6 +50,7 @@
 
 /**
  * @author Tomaz Fernandes
+ * @author Hyunggeol Lee
  */
 @SuppressWarnings("unchecked")
 class SqsTemplateTests {
@@ -1218,4 +1219,104 @@ void shouldPropagateTracingAsMessageSystemAttribute() {
 				value -> assertThat(value.stringValue()).isEqualTo("abc"));
 	}
 
+	@Test
+	void shouldRemoveFailedQueueAttributesFromCache() {
+		// Given - First attempt will fail
+		String queue = "test-queue";
+		String payload = "test-payload";
+
+		CompletableFuture<GetQueueUrlResponse> failedFuture = new CompletableFuture<>();
+		failedFuture.completeExceptionally(new RuntimeException("Queue attributes resolution failed"));
+
+		given(mockClient.getQueueUrl(any(GetQueueUrlRequest.class))).willReturn(failedFuture);
+
+		SqsOperations template = SqsTemplate.newTemplate(mockClient);
+
+		// When - First attempt should fail
+		assertThatThrownBy(() -> template.send(queue, payload)).isInstanceOf(MessagingOperationFailedException.class);
+
+		// Then - Setup successful response for retry
+		GetQueueUrlResponse urlResponse = GetQueueUrlResponse.builder().queueUrl(queue).build();
+		given(mockClient.getQueueUrl(any(GetQueueUrlRequest.class)))
+			.willReturn(CompletableFuture.completedFuture(urlResponse));
+
+		mockQueueAttributes(mockClient, Map.of());
+
+		SendMessageResponse sendResponse = SendMessageResponse.builder().messageId(UUID.randomUUID().toString())
+			.build();
+		given(mockClient.sendMessage(any(SendMessageRequest.class)))
+			.willReturn(CompletableFuture.completedFuture(sendResponse));
+
+		// When - Retry should work (not use cached failure)
+		SendResult<String> result = template.send(queue, payload);
+
+		// Then - Verify getQueueUrl was called twice (failure was not cached)
+		then(mockClient).should(times(2)).getQueueUrl(any(GetQueueUrlRequest.class));
+		assertThat(result.endpoint()).isEqualTo(queue);
+		assertThat(result.message().getPayload()).isEqualTo(payload);
+	}
+
+	@Test
+	void shouldCacheSuccessfulQueueAttributes() {
+		// Given - Setup successful responses
+		String queue = "test-queue";
+		String payload1 = "test-payload-1";
+		String payload2 = "test-payload-2";
+
+		GetQueueUrlResponse urlResponse = GetQueueUrlResponse.builder().queueUrl(queue).build();
+		given(mockClient.getQueueUrl(any(GetQueueUrlRequest.class)))
+			.willReturn(CompletableFuture.completedFuture(urlResponse));
+
+		SendMessageResponse sendResponse = SendMessageResponse.builder().messageId(UUID.randomUUID().toString())
+			.build();
+		given(mockClient.sendMessage(any(SendMessageRequest.class)))
+			.willReturn(CompletableFuture.completedFuture(sendResponse));
+
+		SqsOperations template = SqsTemplate.newTemplate(mockClient);
+
+		// When - Send twice to same queue
+		SendResult<String> result1 = template.send(queue, payload1);
+		SendResult<String> result2 = template.send(queue, payload2);
+
+		// Then - Queue URL should be cached (only called once)
+		then(mockClient).should(times(1)).getQueueUrl(any(GetQueueUrlRequest.class));
+		then(mockClient).should(times(2)).sendMessage(any(SendMessageRequest.class));
+	}
+
+	@Test
+	void shouldCacheSuccessfulQueueAttributesWithAttributeNames() {
+		// Given - Template with queueAttributeNames configured
+		String queue = "test-queue";
+		String payload1 = "test-payload-1";
+		String payload2 = "test-payload-2";
+
+		GetQueueUrlResponse urlResponse = GetQueueUrlResponse.builder().queueUrl(queue).build();
+		given(mockClient.getQueueUrl(any(GetQueueUrlRequest.class)))
+			.willReturn(CompletableFuture.completedFuture(urlResponse));
+
+		GetQueueAttributesResponse attributesResponse = GetQueueAttributesResponse.builder()
+			.attributes(Map.of(QueueAttributeName.QUEUE_ARN, "test-arn")).build();
+		given(mockClient.getQueueAttributes(any(Consumer.class)))
+			.willReturn(CompletableFuture.completedFuture(attributesResponse));
+
+		SendMessageResponse sendResponse = SendMessageResponse.builder().messageId(UUID.randomUUID().toString())
+			.build();
+		given(mockClient.sendMessage(any(SendMessageRequest.class)))
+			.willReturn(CompletableFuture.completedFuture(sendResponse));
+
+		// Create template with queueAttributeNames configured
+		SqsOperations template = SqsTemplate.builder().sqsAsyncClient(mockClient)
+			.configure(
+				options -> options.queueAttributeNames(Collections.singletonList(QueueAttributeName.QUEUE_ARN)))
+			.buildSyncTemplate();
+
+		// When - Send twice to same queue
+		template.send(queue, payload1);
+		template.send(queue, payload2);
+
+		// Then - Queue attributes should be cached (only called once each)
+		then(mockClient).should(times(1)).getQueueUrl(any(GetQueueUrlRequest.class));
+		then(mockClient).should(times(1)).getQueueAttributes(any(Consumer.class));
+		then(mockClient).should(times(2)).sendMessage(any(SendMessageRequest.class));
+	}
 }