enabled dir creation in setup.sh and dir/file creation in pubsub.cpp (#204)

xlcheng1 · HarshGandhi-AWS · web-flow · commit e9691f753ec3 · 2021-12-15T10:57:08.000-08:00
* enabled dir creation in setup.sh and dir/file creation in pubsub.cpp

* Addressed Marco's comments on the PR.

* Formatting error

Co-authored-by: Harsh Gandhi &lt;63674702+HarshGandhi-AWS@users.noreply.github.com&gt;
Co-authored-by: Harsh Gandhi &lt;harsgand@amazon.com&gt;
diff --git a/docs/PERMISSIONS.md b/docs/PERMISSIONS.md
@@ -34,6 +34,7 @@ Directory Storing Root Certificate Authority | 700 | **Yes**
 Directory Storing CSR File  | 700 | **Yes**
 Directory Storing Log File  | 745 | **Yes**
 Directory Storing Config Files | 745 | **Recommended**
+Directory Storing PubSub File  | 745 | **Yes**
 
 *Note: It is worth noting here that files are directories storing these files created by AWS IoT Device Client will have the above mentioned permissions set by default*
 
diff --git a/setup.sh b/setup.sh
@@ -25,6 +25,11 @@ fi
 ### Config Defaults ###
 CONF_OUTPUT_PATH=${OUTPUT_DIR}aws-iot-device-client.conf
 HANDLER_DIR=${OUTPUT_DIR}jobs
+PUBSUB_DIR=${OUTPUT_DIR}pubsub/
+PUB_FILE=${PUBSUB_DIR}publish-file.txt
+SUB_FILE=${PUBSUB_DIR}subscribe-file.txt
+PUB_FILE_PROVIDED="n"
+SUB_FILE_PROVIDED="n"
 DD_INTERVAL=300
 LOG_TYPE="STDOUT"
 LOG_LEVEL="DEBUG"
@@ -180,12 +185,20 @@ if [ "$BUILD_CONFIG" = "y" ]; then
       PUBSUB_ENABLED="true"
       printf ${PMPT} "Specify a topic for the feature to publish to:"
       read -r PUB_TOPIC
-      printf ${PMPT} "Specify the path of a file for the feature to publish (Leaving this blank will publish 'Hello World!'):"
-      read -r PUB_FILE
+      printf ${PMPT} "Specify the path of a file for the feature to publish (if no path is provided, will default to ${PUB_FILE}):"
+      read -r PUB_FILE_TMP
+      if [ "$PUB_FILE_TMP"]; then
+        PUB_FILE=$PUB_FILE_TMP
+        PUB_FILE_PROVIDED="y"
+      fi
       printf ${PMPT} "Specify a topic for the feature to subscribe to:"
       read -r SUB_TOPIC
-      printf ${PMPT} "Specify the path of a file for the feature to write to (Optional):"
-      read -r SUB_FILE
+      printf ${PMPT} "Specify the path of a file for the feature to write to (if no path is provided, will default to ${SUB_FILE}):"
+      read -r SUB_FILE_TMP
+      if [ "$SUB_FILE_TMP"]; then
+        SUB_FILE=$SUB_FILE_TMP
+        SUB_FILE_PROVIDED="y"
+      fi
     else
       PUBSUB_ENABLED="false"
     fi
@@ -284,6 +297,12 @@ if [ "$BUILD_CONFIG" = "y" ]; then
   done
 fi
 
+if [ "$PUB_FILE_PROVIDED" = "n" ] || [ "$SUB_FILE_PROVIDED" = "n" ]; then
+  printf ${GREEN} "Creating default pubsub directory..."
+  mkdir -p ${PUBSUB_DIR}
+  chmod 745 ${PUBSUB_DIR}
+fi
+
 printf ${PMPT} "Do you want to copy the sample job handlers to the specified handler directory (${HANDLER_DIR})? y/n"
 read -r COPY_HANDLERS
 
diff --git a/source/config/Config.cpp b/source/config/Config.cpp
@@ -1068,7 +1068,7 @@ bool PlainConfig::PubSub::LoadFromJson(const Crt::JsonView &json)
     {
         if (!json.GetString(jsonKey).empty())
         {
-            publishFile = FileUtils::ExtractExpandedPath(json.GetString(jsonKey).c_str());
+            publishFile = json.GetString(jsonKey).c_str();
         }
         else
         {
@@ -1144,20 +1144,7 @@ bool PlainConfig::PubSub::Validate() const
             DeviceClient::DC_FATAL_ERROR);
         return false;
     }
-    if (publishFile.has_value() && !publishFile->empty())
-    {
-        if (FileUtils::IsValidFilePath(publishFile->c_str()))
-        {
-            if (!FileUtils::ValidateFilePermissions(publishFile.value(), Permissions::PUB_SUB_FILES, true))
-            {
-                return false;
-            }
-        }
-        else
-        {
-            return false;
-        }
-    }
+
     if (!subscribeTopic.has_value() || subscribeTopic->empty())
     {
         LOGM_ERROR(
@@ -1166,20 +1153,7 @@ bool PlainConfig::PubSub::Validate() const
             DeviceClient::DC_FATAL_ERROR);
         return false;
     }
-    if (subscribeFile.has_value() && !subscribeFile->empty())
-    {
-        if (FileUtils::IsValidFilePath(subscribeFile->c_str()))
-        {
-            if (!FileUtils::ValidateFilePermissions(subscribeFile.value(), Permissions::PUB_SUB_FILES, true))
-            {
-                return false;
-            }
-        }
-        else
-        {
-            return false;
-        }
-    }
+
     return true;
 }
 
diff --git a/source/config/Config.h b/source/config/Config.h
@@ -40,6 +40,7 @@ namespace Aws
                 static constexpr int CSR_DIR = 700;
                 static constexpr int CONFIG_DIR = 745;
                 static constexpr int LOG_DIR = 745;
+                static constexpr int PUBSUB_DIR = 745;
 
                 /** Files **/
                 static constexpr int PRIVATE_KEY = 600;
diff --git a/source/samples/pubsub/PubSubFeature.cpp b/source/samples/pubsub/PubSubFeature.cpp
@@ -3,11 +3,13 @@
 
 #include "PubSubFeature.h"
 #include "../../logging/LoggerFactory.h"
+#include "../../util/FileUtils.h"
 
 #include <aws/common/byte_buf.h>
 #include <aws/crt/Api.h>
 #include <aws/iotdevicecommon/IotDevice.h>
 #include <iostream>
+#include <sys/stat.h>
 
 #include <utility>
 
@@ -22,6 +24,8 @@ using namespace Aws::Iot::DeviceClient::Util;
 using namespace Aws::Iot::DeviceClient::Logging;
 
 constexpr char PubSubFeature::TAG[];
+constexpr char PubSubFeature::DEFAULT_PUBLISH_FILE[];
+constexpr char PubSubFeature::DEFAULT_SUBSCRIBE_FILE[];
 
 #define MAX_IOT_CORE_MQTT_MESSAGE_SIZE_BYTES 128000
 
@@ -30,6 +34,61 @@ string PubSubFeature::getName()
     return "Pub Sub Sample";
 }
 
+bool PubSubFeature::createPubSub(const PlainConfig &config, std::string filePath)
+{
+    std::string pubSubFileDir = FileUtils::ExtractParentDirectory(filePath);
+    LOGM_INFO(TAG, "Creating Pub/Sub file: %s", filePath.c_str());
+    if (!FileUtils::DirectoryExists(pubSubFileDir))
+    {
+        // Create an empty directory with the expected permissions.
+        if (!FileUtils::CreateDirectoryWithPermissions(pubSubFileDir.c_str(), S_IRWXU | S_IRGRP | S_IROTH | S_IXOTH))
+        {
+            return false;
+        }
+    }
+    else
+    {
+        // Verify the directory permissions.
+        auto rcvDirPermissions = FileUtils::GetFilePermissions(pubSubFileDir);
+        if (Permissions::PUBSUB_DIR != rcvDirPermissions)
+        {
+            LOGM_ERROR(
+                TAG,
+                "Incorrect directory permissions for pubsub file: %s expected: %d received: %d",
+                Sanitize(pubSubFileDir).c_str(),
+                Permissions::PUBSUB_DIR,
+                rcvDirPermissions);
+            return false;
+        }
+    }
+
+    if (!FileUtils::FileExists(filePath))
+    {
+        // Create an empty file with the expected permissions.
+        if (!FileUtils::CreateEmptyFileWithPermissions(filePath, S_IRUSR | S_IWUSR))
+        {
+            return false;
+        }
+    }
+    else
+    {
+        // Verify the file permissions.
+        auto rcvFilePermissions = FileUtils::GetFilePermissions(filePath);
+        if (Permissions::PUB_SUB_FILES != rcvFilePermissions)
+        {
+            LOGM_ERROR(
+                TAG,
+                "Incorrect file permissions for pubsub file: %s expected: %d received: %d",
+                Sanitize(filePath).c_str(),
+                Permissions::PUB_SUB_FILES,
+                rcvFilePermissions);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 int PubSubFeature::init(
     shared_ptr<SharedCrtResourceManager> manager,
     shared_ptr<ClientBaseNotifier> notifier,
@@ -40,8 +99,28 @@ int PubSubFeature::init(
     thingName = *config.thingName;
     pubTopic = config.pubSub.publishTopic.value();
     subTopic = config.pubSub.subscribeTopic.value();
-    pubFile = config.pubSub.publishFile.has_value() ? config.pubSub.publishFile.value() : "";
-    subFile = config.pubSub.subscribeFile.has_value() ? config.pubSub.subscribeFile.value() : "";
+
+    if (config.pubSub.publishFile.has_value() && !config.pubSub.publishFile->empty())
+    {
+        pubFile = config.pubSub.publishFile->c_str();
+    }
+    pubFile = FileUtils::ExtractExpandedPath(pubFile);
+
+    if (!createPubSub(config, pubFile))
+    {
+        LOG_ERROR(TAG, "Failed to create publish directory or file");
+    }
+
+    if (config.pubSub.subscribeFile.has_value() && !config.pubSub.subscribeFile->empty())
+    {
+        subFile = config.pubSub.subscribeFile->c_str();
+    }
+    subFile = FileUtils::ExtractExpandedPath(subFile);
+
+    if (!createPubSub(config, subFile))
+    {
+        LOG_ERROR(TAG, "Failed to create subscribe directory or file");
+    }
 
     return AWS_OP_SUCCESS;
 }
@@ -52,7 +131,7 @@ int PubSubFeature::getPublishFileData(aws_byte_buf *buf)
     if (publishFileSize > MAX_IOT_CORE_MQTT_MESSAGE_SIZE_BYTES)
     {
         LOGM_ERROR(
-            TAG, "Publish file too large: %zu > %i bytes", publishFileSize, MAX_IOT_CORE_MQTT_MESSAGE_SIZE_BYTES);
+            TAG, "Publish file too large: %zu > %d bytes", publishFileSize, MAX_IOT_CORE_MQTT_MESSAGE_SIZE_BYTES);
         return AWS_OP_ERR;
     }
     if (publishFileSize == 0)
@@ -84,7 +163,7 @@ void PubSubFeature::publishFileData()
         return;
     }
     auto onPublishComplete = [payload, this](Mqtt::MqttConnection &, uint16_t packetId, int errorCode) mutable {
-        LOGM_DEBUG(TAG, "PublishCompAck: PacketId:(%u), ErrorCode:%i", getName().c_str(), errorCode);
+        LOGM_DEBUG(TAG, "PublishCompAck: PacketId:(%s), ErrorCode:%d", getName().c_str(), errorCode);
         aws_byte_buf_clean_up_secure(&payload);
     };
     resourceManager->getConnection()->Publish(
@@ -97,11 +176,11 @@ int PubSubFeature::start()
 
     auto onSubAck =
         [&](MqttConnection &connection, uint16_t packetId, const String &topic, QOS qos, int errorCode) -> void {
-        LOGM_DEBUG(TAG, "SubAck: PacketId:(%u), ErrorCode:%i", getName().c_str(), errorCode);
+        LOGM_DEBUG(TAG, "SubAck: PacketId:(%s), ErrorCode:%d", getName().c_str(), errorCode);
     };
     auto onRecvData = [&](MqttConnection &connection, const String &topic, const ByteBuf &payload) -> void {
         LOGM_DEBUG(TAG, "Message received on subscribe topic, size: %zu bytes", payload.len);
-        if (string((char *)payload.buffer) == PUBLISH_TRIGGER_PAYLOAD)
+        if (string((char *)payload.buffer, payload.len) == PUBLISH_TRIGGER_PAYLOAD)
         {
             publishFileData();
         }
@@ -127,7 +206,7 @@ int PubSubFeature::start()
 int PubSubFeature::stop()
 {
     auto onUnsubscribe = [&](MqttConnection &connection, uint16_t packetId, int errorCode) -> void {
-        LOGM_DEBUG(TAG, "Unsubscribing: PacketId:%u, ErrorCode:%i", packetId, errorCode);
+        LOGM_DEBUG(TAG, "Unsubscribing: PacketId:%u, ErrorCode:%d", packetId, errorCode);
     };
 
     resourceManager->getConnection()->Unsubscribe(subTopic.c_str(), onUnsubscribe);
diff --git a/source/samples/pubsub/PubSubFeature.h b/source/samples/pubsub/PubSubFeature.h
@@ -29,6 +29,10 @@ namespace Aws
                 class PubSubFeature : public Feature
                 {
                   public:
+                    static constexpr char DEFAULT_PUBLISH_FILE[] = "~/.aws-iot-device-client/pubsub/publish-file.txt";
+                    static constexpr char DEFAULT_SUBSCRIBE_FILE[] =
+                        "~/.aws-iot-device-client/pubsub/subscribe-file.txt";
+                    bool createPubSub(const PlainConfig &config, std::string absFilePath);
                     /**
                      * \brief Initializes the PubSub feature with all the required setup information, event
                      * handlers, and the SharedCrtResourceManager
@@ -73,16 +77,15 @@ namespace Aws
                     /**
                      * \brief Location of file containing data to publish
                      */
-                    std::string pubFile;
+                    std::string pubFile = DEFAULT_PUBLISH_FILE;
                     /**
                      * \brief Topic to subscribe to
                      */
                     std::string subTopic;
                     /**
                      * \brief Topic to write subscription payloads to
                      */
-                    std::string subFile;
-
+                    std::string subFile = DEFAULT_SUBSCRIBE_FILE;
                     /**
                      * \brief Default payload if no publish file was provided
                      */
diff --git a/source/util/FileUtils.cpp b/source/util/FileUtils.cpp
@@ -289,6 +289,11 @@ bool FileUtils::CreateDirectoryWithPermissions(const char *dirPath, mode_t permi
             else
             {
                 // Desired and actual permissions match.
+                LOGM_INFO(
+                    TAG,
+                    "Successfully create directory %s with required permissions %d",
+                    Sanitize(expandedPath).c_str(),
+                    desiredPermissions);
                 return true;
             }
         }

Original file line number	Diff line number	Diff line change
`@@ -289,6 +289,11 @@ bool FileUtils::CreateDirectoryWithPermissions(const char *dirPath, mode_t permi`
`289`	`289`	`else`
`290`	`290`	`{`
`291`	`291`	`// Desired and actual permissions match.`
	`292`	`+ LOGM_INFO(`
	`293`	`+ TAG,`
	`294`	`+ "Successfully create directory %s with required permissions %d",`
	`295`	`+ Sanitize(expandedPath).c_str(),`
	`296`	`+ desiredPermissions);`
`292`	`297`	`return true;`
`293`	`298`	`}`
`294`	`299`	`}`