Fix SqsSource attributes adoption from sqs source settings.

[janjaali]

Purpose

It seems that the AWS-SDK (not only the one for Java) has an issue by setting the requested attributes in a ReceiveMessageRequest against a list of QueueAttributeNames which makes no sense: https://github.com/aws/aws-sdk-java/blob/042fa6c1727e52328a9dfd1684f9d46dbb98e39b/aws-java-sdk-sqs/src/main/java/com/amazonaws/services/sqs/model/ReceiveMessageRequest.java#L1008.

Creating a ReceiveMessageRequest in SqsSource using this method lead to an validation of the provided attributes agains the QueueAttributesNames which again results in a UNKNOWN_TO_SDK_VERSION: https://github.com/aws/aws-sdk-java/blob/042fa6c1727e52328a9dfd1684f9d46dbb98e39b/aws-java-sdk-sqs/src/main/java/com/amazonaws/services/sqs/model/QueueAttributeName.java#L63

This PR will fix the attributes adoption by using the ReceiveMessageRequest::messageAttributeNames method without the validation against queue related attribute names.

References

References #1838

Changes

• Replace attributes adoption in ReceiveMessageRequest building phase from ReceiveMessageRequest::attributeNames method usage to ReceiveMessageRequest::attributeNamesWithStrings.

• Remove not used AttributeName's in SqsSourceSettings to avoid miss-usage.

• Add some dedicated tests for the attributes adotion

Background Context

Using alpakka - thx to the awesome alpakka-team - in production code to operate with (among others) SQS hinted me due to a migration to the new alpakka version that we were missing attributes, unfortunately without any warnings.

[janjaali]

I guess removing some of the not used/usable AttributeNames was not that good idea? The MiMa-check is failing on travis. Strangely executing mimaReportBinaryIssues throws no errors on my machine.

[2m]

Thanks for the PR. sqs/mimaReportBinaryIssues is not failing locally for you probably of a missing tag. We have configured MiMa to figure out the previous released version to test BC against from the git history. Try running git fetch --tags and then sqs/mimaReportBinaryIssues to see if it helped.

Since this is breaking user facing API, I'll target this for the upcoming Alpakka 2.0.

[janjaali]

Yes thx, getting the tags helped to fail MiMa. Anything else I can do for now regarding this PR?

[2m]

When the mimaReportBinaryIssues fails, it prints out the filters, that can be added to ignore those issues. Please add the suggested filters to sqs/src/main/mima-filters/1.1.0.backwards.excludes file. This will make Travis CI validation pass.

[janjaali]

I have added the required filters to the backwards-exclusion file and the build is green now :) Anything else I can help with?

[ennru]

Does the problem apply to AWS SNS in a similar way?

[ennru]

These attributes are available in sqs.model.QueueAttributeName. Can you find a way to keep these classes as deprecated in Alpakka for the time being? Eg. make them use the the enum.

[janjaali]

It would be possible to classify these queue-attributes as QueueAttributeName's (next to the already existing AttributeName's). They would not be deleted nor deprecated in that case, they 'just' wouldn't be used:

sealed abstract class QueueAttributeName(val name: String)

case object Policy extends QueueAttributeName("Policy")
case object VisibilityTimeout extends QueueAttributeName("VisibilityTimeout")
case object MaximumMessageSize extends QueueAttributeName("MaximumMessageSize")
case object MessageRetentionPeriod extends QueueAttributeName("MessageRetentionPeriod")
case object ApproximateNumberOfMessages extends QueueAttributeName("ApproximateNumberOfMessages")
case object ApproximateNumberOfMessagesNotVisible extends QueueAttributeName("ApproximateNumberOfMessagesNotVisible")
case object CreatedTimestamp extends QueueAttributeName("CreatedTimestamp")
case object LastModifiedTimestamp extends QueueAttributeName("LastModifiedTimestamp")
case object QueueArn extends QueueAttributeName("QueueArn")
case object ApproximateNumberOfMessagesDelayed extends QueueAttributeName("ApproximateNumberOfMessagesDelayed")
case object DelaySeconds extends QueueAttributeName("DelaySeconds")
case object ReceiveMessageWaitTimeSeconds extends QueueAttributeName("ReceiveMessageWaitTimeSeconds")
case object RedrivePolicy extends QueueAttributeName("RedrivePolicy")
case object FifoQueue extends QueueAttributeName("FifoQueue")
case object ContentBasedDeduplication extends QueueAttributeName("ContentBasedDeduplication")
case object KmsMasterKeyId extends QueueAttributeName("KmsMasterKeyId")
case object KmsDataKeyReusePeriodSeconds extends QueueAttributeName("KmsDataKeyReusePeriodSeconds")

[janjaali]

I was not sure if I get your point with "using the enum", but if I get it right it would look like this:

import software.amazon.awssdk.services.sqs.model
sealed abstract class QueueAttributeName(val name: model.QueueAttributeName)

case object Policy extends QueueAttributeName(model.QueueAttributeName.POLICY)
case object VisibilityTimeout extends QueueAttributeName(model.QueueAttributeName.VISIBILITY_TIMEOUT)
case object MaximumMessageSize extends QueueAttributeName(model.QueueAttributeName.MAXIMUM_MESSAGE_SIZE)
case object MessageRetentionPeriod extends QueueAttributeName(model.QueueAttributeName.MESSAGE_RETENTION_PERIOD)
case object ApproximateNumberOfMessages extends QueueAttributeName(model.QueueAttributeName.APPROXIMATE_NUMBER_OF_MESSAGES)
case object ApproximateNumberOfMessagesNotVisible extends QueueAttributeName(model.QueueAttributeName.APPROXIMATE_NUMBER_OF_MESSAGES_NOT_VISIBLE)
case object CreatedTimestamp extends QueueAttributeName(model.QueueAttributeName.CREATED_TIMESTAMP)
case object LastModifiedTimestamp extends QueueAttributeName(model.QueueAttributeName.LAST_MODIFIED_TIMESTAMP)
case object QueueArn extends QueueAttributeName(model.QueueAttributeName.QUEUE_ARN)
case object ApproximateNumberOfMessagesDelayed extends QueueAttributeName(model.QueueAttributeName.APPROXIMATE_NUMBER_OF_MESSAGES_DELAYED)
case object DelaySeconds extends QueueAttributeName(model.QueueAttributeName.DELAY_SECONDS)
case object ReceiveMessageWaitTimeSeconds extends QueueAttributeName(model.QueueAttributeName.RECEIVE_MESSAGE_WAIT_TIME_SECONDS)
case object RedrivePolicy extends QueueAttributeName(model.QueueAttributeName.REDRIVE_POLICY)
case object FifoQueue extends QueueAttributeName(model.QueueAttributeName.FIFO_QUEUE)
case object ContentBasedDeduplication extends QueueAttributeName(model.QueueAttributeName.CONTENT_BASED_DEDUPLICATION)
case object KmsMasterKeyId extends QueueAttributeName(model.QueueAttributeName.KMS_MASTER_KEY_ID)
case object KmsDataKeyReusePeriodSeconds extends QueueAttributeName(model.QueueAttributeName.KMS_DATA_KEY_REUSE_PERIOD_SECONDS)

Using the same approach for the AttributeName could be a little bit tricky due to aws/aws-sdk-java#2061. There is already an enum sqs.model.MessageSystemAttributeName modelling the message attributes but this one is currently missing the All option:

sealed abstract class AttributeName(val name: MessageSystemAttributeName)

case object All extends AttributeName(???)
case object ApproximateFirstReceiveTimestamp extends AttributeName(MessageSystemAttributeName.APPROXIMATE_FIRST_RECEIVE_TIMESTAMP)

[ennru]

What about defining them as

sealed abstract class MessageSystemAttributeName(_name: String) extends AttributeName(_name) {
  protected def this(msname: model.MessageSystemAttributeName) {
    this(msname.toString)
  }
}
sealed abstract class QueueAttributeName(qaname: model.QueueAttributeName) extends AttributeName(qaname.toString)

[janjaali]

Applied your suggestion, can you please re-review?

[janjaali]

Does the problem apply to AWS SNS in a similar way?

Not that I would be aware of. The SNS has no capabilities to send default attributes (nor the alpakka SnsPublisher): https://docs.aws.amazon.com/sns/latest/dg/sns-tutorial-publish-message-with-attributes.html.

[janjaali]

The travis-check Compile all tests (with Scala 2.11) fails with Error accessing /home/travis/.ivy2/cache/org.asciidoctor/asciidoctorj-diagram/jars/asciidoctorj-diagram-1.5.16.jar.

If I execute ++2.11.12 Test/compile locally, everything seems to be OK?

[ennru]

I don't know where that came from, some Travis hick-up. I had to remove the Travis cache and now it seems to build as it should.

[ennru]

I might be misunderstanding something. Are all the QueueAttributeName values illegal for use as attributes?

[janjaali]

Yes exactly. The only "valid" QueueAttributeName is All which can be used to retrieve all message attributes (ReceiveMessage). The QueueAttributeName's are only applicable for retrieving some queue attributes (GetQueueAttribute).

[ennru]

Ok, sorry, I didn't get that in the first place. Then it makes sense to remove all the attribute classes which can't be used anyway.

[janjaali]

Allright, I just removed the QueueAttributeNames but would stick to the MessageSystemAttributeNames if you agree, I like the idea to model them in that way to have them extendable for the future :)

[ennru]

I rebased this branch (to get passed the fatal-warnings check) and fixed the MiMa exclusions.

[ennru]

LGTM.

[ennru]

Thank you! Sorry again that I didn't get the whole story to start with.

[avdv]

Thanks @janjaali for the fix 👍, I noticed the same thing in our service. Still looking for a workaround... probably simply have to stick in All for the time being.

Upstream issue: aws/aws-sdk-java-v2#1117 (already a few months old 😮)