Review resolution

2 files changed, 45 insertions, 44 deletions

diff --git a/src/main/java/org/traccar/config/Keys.java b/src/main/java/org/traccar/config/Keys.java
index 937510f10..d2e5bbd99 100644
--- a/src/main/java/org/traccar/config/Keys.java
+++ b/src/main/java/org/traccar/config/Keys.java
@@ -690,31 +690,23 @@ public final class Keys {
     /**
      * AWS Access Key with SNS permission.
      */
-    public static final ConfigKey<String> AWS_ACCESS_KEY = new ConfigKey<>(
-            "aws.access",
+    public static final ConfigKey<String> SMS_AWS_ACCESS = new ConfigKey<>(
+            "sms.aws.access",
             Collections.singletonList(KeyType.GLOBAL));
 
     /**
      * AWS Secret Access Key with SNS permission.
      */
-    public static final ConfigKey<String> AWS_SECRET_KEY = new ConfigKey<>(
-            "aws.secret",
-            Collections.singletonList(KeyType.GLOBAL));
-
-    /**
-     * SNS Enable switch.
-     * Use 'enable' to send message with SNS or 'disable' to stop using SNS
-     */
-    public static final ConfigKey<String> AWS_SNS_ENABLED = new ConfigKey<>(
-            "aws.sns.enabled",
+    public static final ConfigKey<String> SMS_AWS_SECRET = new ConfigKey<>(
+            "sms.aws.secret",
             Collections.singletonList(KeyType.GLOBAL));
 
     /**
      * AWS Region for SNS service.
      * Make sure to use regions that are supported for messaging.
      */
-    public static final ConfigKey<String> AWS_REGION = new ConfigKey<>(
-            "aws.region",
+    public static final ConfigKey<String> SMS_AWS_REGION = new ConfigKey<>(
+            "sms.aws.region",
             Collections.singletonList(KeyType.GLOBAL));
 
     /**
diff --git a/src/main/java/org/traccar/sms/SnsSmsClient.java b/src/main/java/org/traccar/sms/SnsSmsClient.java
index 8ac234605..0c2be0bd2 100644
--- a/src/main/java/org/traccar/sms/SnsSmsClient.java
+++ b/src/main/java/org/traccar/sms/SnsSmsClient.java
@@ -1,3 +1,18 @@
+/*
+ * Copyright 2021 Anton Tananaev (anton@traccar.org)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.traccar.sms;
 
 import com.amazonaws.auth.AWSStaticCredentialsProvider;
@@ -6,7 +21,7 @@ import com.amazonaws.services.sns.AmazonSNS;
 import com.amazonaws.services.sns.AmazonSNSClientBuilder;
 import com.amazonaws.services.sns.model.MessageAttributeValue;
 import com.amazonaws.services.sns.model.PublishRequest;
-import com.amazonaws.services.sns.model.PublishResult;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.traccar.Context;
@@ -16,54 +31,48 @@ import org.traccar.notification.MessageException;
 import java.util.HashMap;
 import java.util.Map;
 
-
 public class SnsSmsClient implements SmsManager {
     private static final Logger LOGGER = LoggerFactory.getLogger(SnsSmsClient.class);
 
-    private final String accessKey;
-    private final String secretKey;
-    private final String region;
-
-    private final String snsStatus;
     private final AmazonSNS snsClient;
 
     public SnsSmsClient() {
-        accessKey = Context.getConfig().getString(Keys.AWS_ACCESS_KEY);
-        secretKey = Context.getConfig().getString(Keys.AWS_SECRET_KEY);
-        snsStatus = Context.getConfig().getString(Keys.AWS_SNS_ENABLED);
-        region = Context.getConfig().getString(Keys.AWS_REGION);
-        snsClient = awsSNSClient(accessKey, secretKey, region);
-
-        if (!snsStatus.equals("true") || accessKey == null || secretKey == null || region == null) {
-            LOGGER.error("SNS Not Configured Properly. Please provide valid config.");
+        if (Context.getConfig().hasKey(Keys.SMS_AWS_REGION)
+                && Context.getConfig().hasKey(Keys.SMS_AWS_ACCESS)
+                && Context.getConfig().hasKey(Keys.SMS_AWS_SECRET)) {
+            snsClient = awsSNSClient();
+        } else {
+            throw new RuntimeException("SNS Not Configured Properly. Please provide valid config.");
         }
     }
 
-    public AmazonSNS awsSNSClient(String accessKey, String secretKey, String region) {
-        BasicAWSCredentials awsCreds = new BasicAWSCredentials(accessKey, secretKey);
-        return AmazonSNSClientBuilder.standard().withRegion(region)
-                .withCredentials(new AWSStaticCredentialsProvider(awsCreds)).build();
+    public AmazonSNS awsSNSClient() {
+        BasicAWSCredentials awsCredentials = new BasicAWSCredentials(Context.getConfig().getString(Keys.SMS_AWS_ACCESS),
+                Context.getConfig().getString(Keys.SMS_AWS_SECRET));
+        return AmazonSNSClientBuilder.standard().withRegion(Context.getConfig().getString(Keys.SMS_AWS_REGION))
+                .withCredentials(new AWSStaticCredentialsProvider(awsCredentials)).build();
     }
 
-    public void sendSNSMessage(String message, String destAddress) {
+    @Override
+    public void sendMessageSync(String destAddress, String message, boolean command)
+            throws InterruptedException, MessageException {
         Map<String, MessageAttributeValue> smsAttributes = new HashMap<>();
         smsAttributes.put("AWS.SNS.SMS.SenderID",
                 new MessageAttributeValue().withStringValue("SNS").withDataType("String"));
         smsAttributes.put("AWS.SNS.SMS.SMSType",
                 new MessageAttributeValue().withStringValue("Transactional").withDataType("String"));
-
-        PublishResult result = this.snsClient.publish(new PublishRequest().withMessage(message)
+        snsClient.publish(new PublishRequest().withMessage(message)
                 .withPhoneNumber(destAddress).withMessageAttributes(smsAttributes));
     }
 
-    @java.lang.Override
-    public void sendMessageSync(String destAddress, String message, boolean command)
-            throws InterruptedException, MessageException {
-        sendSNSMessage(message, destAddress);
-    }
-
-    @java.lang.Override
+    @Override
     public void sendMessageAsync(String destAddress, String message, boolean command) {
-        sendSNSMessage(message, destAddress);
+        try {
+            sendMessageSync(destAddress, message, command);
+        } catch (InterruptedException interruptedException) {
+            LOGGER.warn("SMS send failed", interruptedException.getMessage());
+        } catch (MessageException messageException) {
+            LOGGER.warn("SMS send failed", messageException.getMessage());
+        }
     }
 }