-
Notifications
You must be signed in to change notification settings - Fork 0
Add new functionality to optionally send the data to an SQS #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,15 +7,18 @@ | |||||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.http.urlconnection.UrlConnectionHttpClient; | ||||||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.regions.Region; | ||||||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.services.dynamodb.DynamoDbClient; | ||||||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.services.sqs.SqsClient; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| public class DependencyFactory { | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| public static final String ENV_VARIABLE_TABLE = "TABLE"; | ||||||||||||||||||||||||||||||||||||||||||
| public static final String ENV_VARIABLE_SQS_ENABLED = "SQS_ENABLED"; | ||||||||||||||||||||||||||||||||||||||||||
| public static final String ENV_VARIABLE_SQS_QUEUE_URL = "SQS_QUEUE_URL"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| private DependencyFactory() {} | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * @return an instance of LambdaClient | ||||||||||||||||||||||||||||||||||||||||||
| * @return an instance of DynamoDbEnhancedClient | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
| public static DynamoDbEnhancedClient dynamoDbEnhancedClient() { | ||||||||||||||||||||||||||||||||||||||||||
| return DynamoDbEnhancedClient.builder() | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -27,8 +30,27 @@ public static DynamoDbEnhancedClient dynamoDbEnhancedClient() { | |||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * @return an instance of SqsClient | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
| public static SqsClient sqsClient() { | ||||||||||||||||||||||||||||||||||||||||||
| return SqsClient.builder() | ||||||||||||||||||||||||||||||||||||||||||
| .credentialsProvider(EnvironmentVariableCredentialsProvider.create()) | ||||||||||||||||||||||||||||||||||||||||||
| .region(Region.of(System.getenv(SdkSystemSetting.AWS_REGION.environmentVariable()))) | ||||||||||||||||||||||||||||||||||||||||||
| .httpClientBuilder(UrlConnectionHttpClient.builder()) | ||||||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| public static String tableName() { | ||||||||||||||||||||||||||||||||||||||||||
| return System.getenv(ENV_VARIABLE_TABLE); | ||||||||||||||||||||||||||||||||||||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Description: The methods tableName() and sqsQueueUrl() don't handle cases where the environment variables are not set. Consider adding null checks or providing default values for these methods. Severity: Medium
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix addresses the issue of not handling cases where environment variables are not set in the tableName() and sqsQueueUrl() methods. For tableName(), a default value "default_table_name" is returned if the environment variable is not set. For sqsQueueUrl(), an empty string is returned if the environment variable is not set. This ensures that these methods always return a non-null value, preventing potential NullPointerExceptions in the code that calls these methods.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| public static boolean isSqsEnabled() { | ||||||||||||||||||||||||||||||||||||||||||
| String sqsEnabled = System.getenv(ENV_VARIABLE_SQS_ENABLED); | ||||||||||||||||||||||||||||||||||||||||||
| return sqsEnabled != null && sqsEnabled.equalsIgnoreCase("true"); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| public static String sqsQueueUrl() { | ||||||||||||||||||||||||||||||||||||||||||
| return System.getenv(ENV_VARIABLE_SQS_QUEUE_URL); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,22 +9,32 @@ | |||||||||||||||||||||||||||||||||||||
| import com.home.amazon.serverless.model.Book; | ||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.enhanced.dynamodb.DynamoDbEnhancedClient; | ||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.enhanced.dynamodb.DynamoDbTable; | ||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.enhanced.dynamodb.Key; | ||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.enhanced.dynamodb.TableSchema; | ||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.services.sqs.SqsClient; | ||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.services.sqs.model.SendMessageRequest; | ||||||||||||||||||||||||||||||||||||||
| import software.amazon.awssdk.services.sqs.model.SendMessageResponse; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import java.util.Collections; | ||||||||||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public class PostItemHandler implements RequestHandler<APIGatewayProxyRequestEvent, APIGatewayProxyResponseEvent> { | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| private final DynamoDbEnhancedClient dbClient; | ||||||||||||||||||||||||||||||||||||||
| private final SqsClient sqsClient; | ||||||||||||||||||||||||||||||||||||||
| private final String tableName; | ||||||||||||||||||||||||||||||||||||||
| private final TableSchema<Book> bookTableSchema; | ||||||||||||||||||||||||||||||||||||||
| private final boolean sqsEnabled; | ||||||||||||||||||||||||||||||||||||||
| private final String sqsQueueUrl; | ||||||||||||||||||||||||||||||||||||||
| private final Gson gson; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public PostItemHandler() { | ||||||||||||||||||||||||||||||||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning Description: The constructor initializes multiple dependencies without proper error handling. Consider wrapping the initialization in a try-catch block to handle potential exceptions during object creation. Severity: High
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix addresses the comment by wrapping the constructor initialization in a try-catch block. This allows for proper error handling during object creation. If an exception occurs during initialization, it's caught and rethrown as a RuntimeException with a descriptive message, providing better error reporting and preventing silent failures.
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| dbClient = DependencyFactory.dynamoDbEnhancedClient(); | ||||||||||||||||||||||||||||||||||||||
| sqsClient = DependencyFactory.sqsClient(); | ||||||||||||||||||||||||||||||||||||||
| tableName = DependencyFactory.tableName(); | ||||||||||||||||||||||||||||||||||||||
| bookTableSchema = TableSchema.fromBean(Book.class); | ||||||||||||||||||||||||||||||||||||||
| sqsEnabled = DependencyFactory.isSqsEnabled(); | ||||||||||||||||||||||||||||||||||||||
| sqsQueueUrl = DependencyFactory.sqsQueueUrl(); | ||||||||||||||||||||||||||||||||||||||
| gson = new Gson(); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -38,12 +48,27 @@ public APIGatewayProxyResponseEvent handleRequest(APIGatewayProxyRequestEvent in | |||||||||||||||||||||||||||||||||||||
| item.setAuthor(queryStringParameters.get("author")); | ||||||||||||||||||||||||||||||||||||||
| item.setName(queryStringParameters.get("name")); | ||||||||||||||||||||||||||||||||||||||
| booksTable.putItem(item); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Optionally send to SQS if enabled | ||||||||||||||||||||||||||||||||||||||
| if (sqsEnabled && sqsQueueUrl != null && !sqsQueueUrl.isEmpty()) { | ||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||
| String messageBody = gson.toJson(item); | ||||||||||||||||||||||||||||||||||||||
| SendMessageRequest sendMessageRequest = SendMessageRequest.builder() | ||||||||||||||||||||||||||||||||||||||
| .queueUrl(sqsQueueUrl) | ||||||||||||||||||||||||||||||||||||||
| .messageBody(messageBody) | ||||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| SendMessageResponse sendMessageResponse = sqsClient.sendMessage(sendMessageRequest); | ||||||||||||||||||||||||||||||||||||||
| context.getLogger().log("Message sent to SQS. MessageId: " + sendMessageResponse.messageId()); | ||||||||||||||||||||||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||||||||||||||||||||||
| context.getLogger().log("Error sending message to SQS: " + e.getMessage()); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return new APIGatewayProxyResponseEvent().withStatusCode(200) | ||||||||||||||||||||||||||||||||||||||
| .withIsBase64Encoded(Boolean.FALSE) | ||||||||||||||||||||||||||||||||||||||
| .withHeaders(Collections.emptyMap()) | ||||||||||||||||||||||||||||||||||||||
| .withBody("Success"); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description: The class contains both client creation methods and environment variable access methods, potentially violating the Single Responsibility Principle. Consider separating the environment variable access methods into a separate class, such as EnvironmentConfig, to improve modularity.
Severity: Low
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix addresses the Single Responsibility Principle violation by removing the environment variable constants from the DependencyFactory class. These constants should be moved to a separate EnvironmentConfig class to improve modularity and separate concerns. The DependencyFactory class now focuses solely on creating and providing instances of AWS clients.