Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

INTEXT-6: Initial push for AWS core and SES adapter #5

Closed
wants to merge 3 commits into from

2 participants

@amolnayak311
Collaborator

Add support for AWS SES Adapters from Spring Integration.

The umbrella JIRA for the task is https://jira.springsource.org/browse/INTEXT-4
and the sub task for adding SES support is https://jira.springsource.org/browse/INTEXT-6

Find more information about Amazon Web Services (AWS) product here

spring-integration-aws/pom.xml
((109 lines not shown))
+ <artifactId>commons-io</artifactId>
+ <version>2.0.1</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.mail</groupId>
+ <artifactId>mail</artifactId>
+ <version>1.4.4</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework.integration</groupId>
+ <artifactId>spring-integration-core</artifactId>
+ <version>2.2.0.BUILD-SNAPSHOT</version>
@ghillert Owner

You can probably rely on 2.2.RC1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spring-integration-aws/pom.xml
((168 lines not shown))
+ <version>2.2.0.BUILD-SNAPSHOT</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.hamcrest</groupId>
+ <artifactId>hamcrest-core</artifactId>
+ <version>1.1</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit-dep</artifactId>
+ <version>4.8.2</version>
+ <scope>test</scope>
@ghillert Owner

Update version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spring-integration-aws/pom.xml
((189 lines not shown))
+ <version>1.9.0</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>1.9.0</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework</groupId>
+ <artifactId>spring-test</artifactId>
+ <version>3.1.0.RELEASE</version>
+ <scope>test</scope>
@ghillert Owner

Update Spring version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...onfig/xml/AbstractAWSInboundChannelAdapterParser.java
((38 lines not shown))
+ *
+ * @since 1.0
+ *
+ */
+public abstract class AbstractAWSInboundChannelAdapterParser extends
+ AbstractPollingInboundChannelAdapterParser {
+
+ /**
+ * Registers the {@link AmazonWSCredentials} bean with the current ApplicationContext
+ * @param element
+ * @param parserContext
+ * @return
+ */
+ protected String registerAmazonWSCredentials(Element element,ParserContext parserContext) {
+ String accessKey = element.getAttribute(ACCESS_KEY);
+ String secretKey = element.getAttribute(SECRET_KEY);
@ghillert Owner

I think, we need to refactor authentication. E.g. other adapters: Twitter has the twitterTemplate, XMPP has the xmpp-connection namespace element...

Maybe we should just directly expose the AwsCredentials interface

Otherwise users may have to pass the same information multiple times across adapter

@amolnayak311 Collaborator

Agreed, will expose the setter for the credentials but will use the o.s.i.aws.core.AmazonWSCredentials instead of c.a.a.AWSCredentials. I am not exposing any AWS SDK class as we also plan to have alternate implementations for say S3 operations in which case we would need some more generic datastructure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ngframework/integration/aws/core/CommonConstants.java
((10 lines not shown))
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.integration.aws.core;
+
+/**
+ * Common constants that would be used in the Project
+ * @author Amol Nayak
+ *
+ * @since 1.0
+ *
+ */
+public interface CommonConstants {
@ghillert Owner

I am not a fan if storing constants in Interfaces. We should use an Enum instead. Also, it looks like the constants are using only by the XML Parser classes - should it be better to move the constants to the respective package - eg:

  • org.springframework.integration.aws.config.xml or
  • org.springframework.integration.aws.config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@amolnayak311
Collaborator

Pushed review changes

...amework/integration/aws/core/AmazonWSCommonUtils.java
((58 lines not shown))
+ } catch (NoSuchAlgorithmException e) {
+ logger.error("Caught Exception while generating a MessageDigest instance", e);
+ } catch (FileNotFoundException e) {
+ logger.error("File " + file.getName() + " not found",e);
+ } catch(IOException e) {
+ logger.error("IO Exception occurred while reading file " + file.getName(), e);
+ }
+ return null;
+ }
+
+ /**
+ * Compute the MD5 hash of the provided String
+ * @param the String whose MD5 sun is to be computed
+ * @return
+ */
+ public static byte[] getContentsMD5AsBytes(String contents) {
@ghillert Owner
ghillert added a note

Not sure if we need to re-invent the wheel here:

We could use a DigestInputStream or Google's Guava libraries. See for further details: http://stackoverflow.com/questions/304268/getting-a-files-md5-checksum-in-java

Looks like the method is not currently used anyway (?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...egration/aws/ses/core/AmazonSESSimpleMailMessage.java
((17 lines not shown))
+
+import java.util.List;
+
+
+/**
+ * The Class representing the Amazon SES mail message.
+ * use {@link AmazonSESMailSender} for sending mail messages
+ *
+ * @see AmazonSESMailSender
+ *
+ * @author Amol Nayak
+ *
+ * @since 1.0
+ *
+ */
+public class AmazonSESSimpleMailMessage {
@ghillert Owner
ghillert added a note

Shouldn't we reuse Spring's support here? E.g. SimpleMailMessage or MimeMailMessage? Or at least be based on it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ork/integration/aws/ses/core/AmazonSESMailSender.java
((15 lines not shown))
+ */
+package org.springframework.integration.aws.ses.core;
+
+import javax.mail.internet.MimeMessage;
+
+/**
+ * The common interface for sending the mail using Amazon Simple Email Service (SES)
+ * for more information on Amazon SES visit
+ * http://aws.amazon.com/ses/
+ *
+ * @author Amol Nayak
+ *
+ * @since 1.0
+ *
+ */
+public interface AmazonSESMailSender {
@ghillert Owner
ghillert added a note

Not sure whether it may make sense to provide some basic reuse of Spring's MailSender interface

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghillert
Owner

Please rebase against master to get the latest changes.

spring-integration-aws/pom.xml
((109 lines not shown))
+ <artifactId>commons-io</artifactId>
+ <version>2.0.1</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.mail</groupId>
+ <artifactId>mail</artifactId>
+ <version>1.4.4</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework.integration</groupId>
+ <artifactId>spring-integration-core</artifactId>
+ <version>2.2.0.RC1</version>
@ghillert Owner
ghillert added a note
  • change to 2.2.0.RELEASE
  • move to property
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spring-integration-aws/pom.xml
((146 lines not shown))
+ <version>4.1.1</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpcore</artifactId>
+ <version>4.1.1</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <!-- No compile time dependency, just using the MimeMessageHelper in tests -->
+ <dependency>
+ <groupId>org.springframework</groupId>
+ <artifactId>spring-context-support</artifactId>
+ <version>3.1.0.RELEASE</version>
@ghillert Owner
ghillert added a note
  • Change to 3.1.3.RELEASE
  • Consider using property
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
spring-integration-aws/pom.xml
((153 lines not shown))
+ <version>4.1.1</version>
+ <scope>compile</scope>
+ </dependency>
+
+ <!-- No compile time dependency, just using the MimeMessageHelper in tests -->
+ <dependency>
+ <groupId>org.springframework</groupId>
+ <artifactId>spring-context-support</artifactId>
+ <version>3.1.0.RELEASE</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.springframework.integration</groupId>
+ <artifactId>spring-integration-test</artifactId>
+ <version>2.2.0.BUILD-SNAPSHOT</version>
@ghillert Owner
ghillert added a note
  • change to 2.2.0.RELEASE
  • move to property
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ion/aws/ses/core/AmazonSESMailSenderImplAWSTests.java
((53 lines not shown))
+
+ @BeforeClass
+ public static final void setupSender() throws Exception {
+ PropertiesAWSCredentials credentials =
+ new PropertiesAWSCredentials("classpath:awscredentials.properties");
+ credentials.afterPropertiesSet();
+ sender = new AmazonSESMailSenderImpl(credentials);
+ }
+
+ /**
+ * Send a mail using {@link AmazonSESSimpleMailMessage}
+ */
+ @Test
+ public void sendAmazonSESSimpleMailMessage() {
+ AmazonSESSimpleMailMessage message = new AmazonSESSimpleMailMessage();
+ message.setFrom("amolnayak311@gmail.com");
@ghillert Owner
ghillert added a note

We should probably not use hard-coded email addresses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ghillert
Owner

We should probably move to a gradle-based build. But I can make that change once the PR is merged.

@ghillert
Owner

Once the code is checked out, several tests are failing. For integration tests (Using SES directly) that require configuration, we should probably use Spring's Profile support for tests (disabled by default). All basic tests should pass without the need to make configuration changes. Mock as much as possible.

@ghillert
Owner

Add a file /src/main/resources/META-INF/spring.tooling

This will provide good defaults in Eclipse/STS when adding Spring Integration AWS namespace support, e.g.:

http\://www.springframework.org/schema/integration/aws@name=integration aws Namespace
http\://www.springframework.org/schema/integration/aws@prefix=int-aws
http\://www.springframework.org/schema/integration/twitter@icon=org/springframework/integration/twitter/config/spring-integration-aws.gif

Not sure about the best prefix, yet - However, since this is the AWS module, I would probably like to go with int-aws.

This would also have implications for the naming of the SES adapters. Thus instead, of:

<aws-ses:outbound-channel-adapter

I would prefer:

<int-aws:ses-outbound-channel-adapter

That way, we only have to deal with one namespace when adding support for additional AWS services.

...aws/ses/config/xml/spring-integration-aws-ses-1.0.xsd
((1 lines not shown))
+<?xml version="1.0" encoding="UTF-8"?>
+<xsd:schema xmlns="http://www.springframework.org/schema/integration/aws/ses"
+ xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:beans="http://www.springframework.org/schema/beans"
+ xmlns:tool="http://www.springframework.org/schema/tool"
+ xmlns:integration="http://www.springframework.org/schema/integration"
+ targetNamespace="http://www.springframework.org/schema/integration/aws/ses"
+ elementFormDefault="qualified"
+ attributeFormDefault="unqualified">
+
+ <xsd:import namespace="http://www.springframework.org/schema/beans"/>
+ <xsd:import namespace="http://www.springframework.org/schema/tool"/>
+ <xsd:import namespace="http://www.springframework.org/schema/integration"
+ schemaLocation="http://www.springframework.org/schema/integration/spring-integration-2.2.xsd"/>
+
+
@ghillert Owner
ghillert added a note

If possible comment all Elements and Attributes. Currently no descriptions are provided in the STS for the *SES outbound-channel-adapter**. Particular for new users, having some documentation for the STS code completion would be nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...integration/aws/ses/core/AmazonSESMailSenderImpl.java
((29 lines not shown))
+import com.amazonaws.services.simpleemail.model.Content;
+import com.amazonaws.services.simpleemail.model.Destination;
+import com.amazonaws.services.simpleemail.model.Message;
+import com.amazonaws.services.simpleemail.model.RawMessage;
+import com.amazonaws.services.simpleemail.model.SendEmailRequest;
+import com.amazonaws.services.simpleemail.model.SendRawEmailRequest;
+
+/**
+ * The implementation class for sending mail using the Amazon SES service
+ *
+ * @author Amol Nayak
+ *
+ * @since 1.0
+ *
+ */
+public class AmazonSESMailSenderImpl implements AmazonSESMailSender {
@ghillert Owner
ghillert added a note

Call it rather DefaultAmazonSESMailSender

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@amolnayak311
Collaborator

Let me work on the changes, should be done by this weekend.

@ghillert
Owner

I wonder if we should name/prefix things (classes, methods) AWS instead of AmazonWS. That seems to be more consistent with the Amazon SDK naming strategy. Also, JetS3t uses the same naming strategy. Technically, I would rather use Aws but I think this would be too confusing in this context.

Therefore, use AWSCredentials instead of AmazonWSCredentials.

...amework/integration/aws/core/BasicAWSCredentials.java
((12 lines not shown))
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.integration.aws.core;
+
+/**
+ * The basic implementation class holding the Access key and the secret
+ * key for the AWS account .
+ *
+ * @author Amol Nayak
+ *
+ * @since 1.0
+ *
+ */
+public class BasicAWSCredentials implements AmazonWSCredentials {
@ghillert Owner
ghillert added a note

I wonder if the implementation should just wrap Amazon's BasicAWSCredentials

@amolnayak311 Collaborator

Didn't want to do that as that would mandate a dependency on AWS Client libraries. In case of S3 which is coming up, if we provide alternate implementations like JetS3, we would not like to have AWS Client jars in classpath too just for one class. Let me know if you have any other view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...work/integration/aws/ses/AmazonSESMessageHandler.java
((36 lines not shown))
+ *
+ * @since 1.0
+ *
+ */
+public class AmazonSESMessageHandler extends AbstractMessageHandler {
+
+
+ /**.
+ * The mail sender implementation of Amazon SES
+ */
+ private AmazonSESMailSender mailSender;
+
+ /**
+ * The AWS Access key of the user account
+ */
+ private final String accessKey;
@ghillert Owner
ghillert added a note

I don't think we should care about the accessKey in the MessageHandler. The accessKey is an implementation detail of the specified mailSender implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...work/integration/aws/ses/AmazonSESMessageHandler.java
((118 lines not shown))
+ if(value != null) {
+ if(value instanceof String)
+ body = (String)value;
+ else
+ throw new AmazonSESMailSendException(accessKey,
+ "Body is expected to be String, found "
+ + value.getClass().getCanonicalName(),
+ null, null);
+ }
+
+
+
+ // construct the mail message
+ AmazonSESSimpleMailMessage mailMessage =
+ AmazonSESMailMessageBuilder
+ .newBuilder(accessKey)
@ghillert Owner
ghillert added a note

Don't think we need to be aware of the accessKey here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...amework/integration/aws/ses/AmazonSESMailHeaders.java
((19 lines not shown))
+ * All the pre defined Amazon SES Mail headers
+ *
+ * @author Amol Nayak
+ *
+ * @since 1.0
+ *
+ */
+public interface AmazonSESMailHeaders {
+
+ public static final String FROM_EMAIL_ID = "fromEmailId";
+ public static final String TO_EMAIL_ID = "toEmailId";
+ public static final String BCC_EMAIL_ID = "bccEmailId";
+ public static final String CC_EMAIL_ID = "ccEmailId";
+ public static final String REPLYTO_EMAIL_ID = "ccEmailId";
+ public static final String HTML_FORMAT = "htmlFormat";
+ public static final String SUBJECT = "subject";
@ghillert Owner
ghillert added a note

Check whitespace vs tabs here. May want to use whitespace only to separate FROM_EMAIL_ID = "fromEmailId"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@amolnayak311
Collaborator

Made the following changes

  • Use SimpleMailMessage instead of AmazonSESSimpleMailMessage
  • DefaultAmazonSESMailSender now extends JavaMailSenderImpl overriding the doSend
  • All maven artifact versions updated and moved to properties.
  • Namespace changes made to allow just one namespace for all the AWS adapters
  • The AmazonSESMessageHandler now extends MailSendingMessageHandler from spring-integration-mail. This was done in order to prevent duplication of code. This is now possible as the AmazonSESMailSender is a JavaMailSender.
  • Added the spring.tooling file
  • Added comments to attributes and elements in the spring-integration-aws's namespace xsd.
  • Removed the extra classes/interfaces not used anymore after the recommended changes.

Changes not made

  • I'm not too sure how we are planning to use spring profiles to test the AWS tests, we can talk over skype about this.The AWS tests are excluded from maven's surefire plugin so that they do not execute by default. In order to execute them from maven, We can have maven build profiles which doesn't skip those tests.Since we are anyways doing away with maven in favor of gradle, I didnt bother to make the change.
@ghillert ghillert referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghillert ghillert closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.