From 5e5fa1f357afa73a6174bd2a176611e9d2f28141 Mon Sep 17 00:00:00 2001 From: Jae Hyeon Bae Date: Wed, 2 Jul 2014 15:53:39 -0700 Subject: [PATCH] A few fixes - EasyMock instead of Mockito - FileSessionCredentialsProvider fields should be volatile - getRestS3Service should create AWSCredentials not a AWSSessionCredentials with fixed credentials --- s3-extensions/pom.xml | 5 ++--- .../s3/AWSSessionCredentialsAdapter.java | 17 ++++++++--------- .../s3/FileSessionCredentialsProvider.java | 6 +++--- .../storage/s3/S3StorageDruidModule.java | 12 ++++++++++-- .../s3/TestAWSCredentialsProvider.java | 19 ++++++++++--------- 5 files changed, 33 insertions(+), 26 deletions(-) diff --git a/s3-extensions/pom.xml b/s3-extensions/pom.xml index 24807b42db2..1af0b941bdd 100644 --- a/s3-extensions/pom.xml +++ b/s3-extensions/pom.xml @@ -70,9 +70,8 @@ test - org.mockito - mockito-core - 1.9.5 + org.easymock + easymock test diff --git a/s3-extensions/src/main/java/io/druid/storage/s3/AWSSessionCredentialsAdapter.java b/s3-extensions/src/main/java/io/druid/storage/s3/AWSSessionCredentialsAdapter.java index 8cabf29c135..abdc5e5cde8 100644 --- a/s3-extensions/src/main/java/io/druid/storage/s3/AWSSessionCredentialsAdapter.java +++ b/s3-extensions/src/main/java/io/druid/storage/s3/AWSSessionCredentialsAdapter.java @@ -8,7 +8,10 @@ public class AWSSessionCredentialsAdapter extends AWSSessionCredentials { public AWSSessionCredentialsAdapter(AWSCredentialsProvider provider) { super(null, null, null); - this.provider = provider; + if(provider.getCredentials() instanceof com.amazonaws.auth.AWSSessionCredentials) + this.provider = provider; + else + throw new IllegalArgumentException("provider does not contain session credentials"); } @Override @@ -18,7 +21,7 @@ public class AWSSessionCredentialsAdapter extends AWSSessionCredentials { @Override public String getVersionPrefix() { - return "Netflix AWSSessionCredentialsAdapter, version: "; + return "AWSSessionCredentialsAdapter, version: "; } @Override @@ -32,12 +35,8 @@ public class AWSSessionCredentialsAdapter extends AWSSessionCredentials { } public String getSessionToken() { - if (provider.getCredentials() instanceof com.amazonaws.auth.AWSSessionCredentials) { - com.amazonaws.auth.AWSSessionCredentials sessionCredentials = - (com.amazonaws.auth.AWSSessionCredentials) provider.getCredentials(); - return sessionCredentials.getSessionToken(); - } else { - return ""; - } + com.amazonaws.auth.AWSSessionCredentials sessionCredentials = + (com.amazonaws.auth.AWSSessionCredentials) provider.getCredentials(); + return sessionCredentials.getSessionToken(); } } diff --git a/s3-extensions/src/main/java/io/druid/storage/s3/FileSessionCredentialsProvider.java b/s3-extensions/src/main/java/io/druid/storage/s3/FileSessionCredentialsProvider.java index c37edfb51da..35a999682ba 100644 --- a/s3-extensions/src/main/java/io/druid/storage/s3/FileSessionCredentialsProvider.java +++ b/s3-extensions/src/main/java/io/druid/storage/s3/FileSessionCredentialsProvider.java @@ -13,9 +13,9 @@ import java.util.Properties; public class FileSessionCredentialsProvider implements AWSCredentialsProvider { private final String sessionCredentials; - private String sessionToken; - private String accessKey; - private String secretKey; + private volatile String sessionToken; + private volatile String accessKey; + private volatile String secretKey; public FileSessionCredentialsProvider(String sessionCredentials) { this.sessionCredentials = sessionCredentials; diff --git a/s3-extensions/src/main/java/io/druid/storage/s3/S3StorageDruidModule.java b/s3-extensions/src/main/java/io/druid/storage/s3/S3StorageDruidModule.java index 3a66292e76a..3d2434365ae 100644 --- a/s3-extensions/src/main/java/io/druid/storage/s3/S3StorageDruidModule.java +++ b/s3-extensions/src/main/java/io/druid/storage/s3/S3StorageDruidModule.java @@ -30,6 +30,7 @@ import io.druid.guice.JsonConfigProvider; import io.druid.guice.LazySingleton; import io.druid.initialization.DruidModule; import org.jets3t.service.impl.rest.httpclient.RestS3Service; +import org.jets3t.service.security.AWSCredentials; import java.util.List; @@ -92,8 +93,15 @@ public class S3StorageDruidModule implements DruidModule @Provides @LazySingleton - public RestS3Service getRestS3Service(AWSCredentialsProvider credentialsProvider) + public RestS3Service getRestS3Service(AWSCredentialsProvider provider) { - return new RestS3Service(new AWSSessionCredentialsAdapter(credentialsProvider)); + if(provider.getCredentials() instanceof com.amazonaws.auth.AWSSessionCredentials) { + return new RestS3Service(new AWSSessionCredentialsAdapter(provider)); + } else { + return new RestS3Service(new AWSCredentials( + provider.getCredentials().getAWSAccessKeyId(), + provider.getCredentials().getAWSSecretKey() + )); + } } } diff --git a/s3-extensions/src/test/java/io/druid/storage/s3/TestAWSCredentialsProvider.java b/s3-extensions/src/test/java/io/druid/storage/s3/TestAWSCredentialsProvider.java index f677d25813c..e98e178550f 100644 --- a/s3-extensions/src/test/java/io/druid/storage/s3/TestAWSCredentialsProvider.java +++ b/s3-extensions/src/test/java/io/druid/storage/s3/TestAWSCredentialsProvider.java @@ -3,6 +3,7 @@ package io.druid.storage.s3; import com.amazonaws.auth.AWSCredentials; import com.amazonaws.auth.AWSCredentialsProvider; import com.amazonaws.auth.AWSSessionCredentials; +import org.easymock.EasyMock; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -13,17 +14,16 @@ import java.io.PrintWriter; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; public class TestAWSCredentialsProvider { @Test public void testWithFixedAWSKeys() { S3StorageDruidModule module = new S3StorageDruidModule(); - AWSCredentialsConfig config = mock(AWSCredentialsConfig.class); - doReturn("accessKeySample").when(config).getAccessKey(); - doReturn("secretKeySample").when(config).getSecretKey(); + AWSCredentialsConfig config = EasyMock.createMock(AWSCredentialsConfig.class); + EasyMock.expect(config.getAccessKey()).andReturn("accessKeySample").atLeastOnce(); + EasyMock.expect(config.getSecretKey()).andReturn("secretKeySample").atLeastOnce(); + EasyMock.replay(config); AWSCredentialsProvider provider = module.getAWSCredentialsProvider(config); AWSCredentials credentials = provider.getCredentials(); @@ -41,14 +41,15 @@ public class TestAWSCredentialsProvider { public void testWithFileSessionCredentials() throws IOException { S3StorageDruidModule module = new S3StorageDruidModule(); - AWSCredentialsConfig config = mock(AWSCredentialsConfig.class); - doReturn("").when(config).getAccessKey(); - doReturn("").when(config).getSecretKey(); + AWSCredentialsConfig config = EasyMock.createMock(AWSCredentialsConfig.class); + EasyMock.expect(config.getAccessKey()).andReturn(""); + EasyMock.expect(config.getSecretKey()).andReturn(""); File file = folder.newFile(); PrintWriter out = new PrintWriter(file.getAbsolutePath()); out.println("sessionToken=sessionTokenSample\nsecretKey=secretKeySample\naccessKey=accessKeySample"); out.close(); - doReturn(file.getAbsolutePath()).when(config).getFileSessionCredentials(); + EasyMock.expect(config.getFileSessionCredentials()).andReturn(file.getAbsolutePath()).atLeastOnce(); + EasyMock.replay(config); AWSCredentialsProvider provider = module.getAWSCredentialsProvider(config); AWSCredentials credentials = provider.getCredentials();