Refactor StartMonitors (#73)

* first commit

* resolve linter warnings

* move function to another class and create a unit test for OpenConfigFile function

* add unit test and move open config file to configfileparser class

* amend unit test

* amend unit test

* add unit test and centralize logger settings

* shorten line

* un-do linter issue

* Change SAL Annotation

* amend unit test

* fix annotation

* add utility file

* remove un-necessary line of code

Co-authored-by: Charity Kathure <ckathure@microsoft.com>

LogMonitor/LogMonitorTests/ConfigFileParserTests.cpp

@@ -65,6 +65,12 @@ namespace LogMonitorTests
             return str;
         }
 
+        ///
+        /// Add the path of the created directories, to be removed during
+        /// cleanup.
+        ///
+        std::vector<std::wstring> directoriesToDeleteAtCleanup;
+
     public:
 
         ///
@@ -1523,5 +1529,46 @@ namespace LogMonitorTests
                 Assert::IsTrue(output.find(L"WARNING") != std::wstring::npos);
             }
         }
+
+        ///
+        /// Check that UTF8 encoded config file is opened and read by OpenConfigFile.
+        ///
+        TEST_METHOD(TestUTF8EncodedConfigFileReading)
+        {
+            //create a temp folder to hold config 
+            std::wstring tempDirectory = CreateTempDirectory();
+            Assert::IsFalse(tempDirectory.empty());
+            directoriesToDeleteAtCleanup.push_back(tempDirectory);
+            //
+            // Create subdirectory
+            //
+            std::wstring subDirectory = tempDirectory + L"\\LogMonitor";
+            long status = CreateDirectoryW(subDirectory.c_str(), NULL);
+            Assert::AreNotEqual(0L, status);
+
+            std::wstring fileName = L"LogMonitorConfigTesting.json";
+            std::wstring fullFileName = subDirectory + L"\\" + fileName;
+
+            //create the utf8 encoded config file
+            std::wstring configFileStr =
+                L"{    \
+                    \"LogConfig\": {    \
+                        \"sources\": [ \
+                        ]\
+                    }\
+                }";
+
+            std::wofstream wof;
+            wof.imbue(std::locale(std::locale::empty(), new std::codecvt_utf8<wchar_t, 0x10ffff, std::generate_header>));
+            wof.open(fullFileName);
+            wof << configFileStr;
+            wof.close();
+
+            //check if the file can be successfully read by OpenConfigFile
+            LoggerSettings settings;
+            bool succcess = OpenConfigFile((PWCHAR)fullFileName.c_str(), settings);
+            Assert::AreEqual(succcess, true);
+        }
+
     };
 }

LogMonitor/LogMonitorTests/LogFileMonitorTests.cpp

@@ -47,32 +47,6 @@ namespace LogMonitorTests
             return std::wstring(bigOutBuf);
         }
 
-
-        ///
-        /// Creates a new random-name directory inside the Temp directory. 
-        ///
-        /// \return The new directory path. If an error occurs, it's empty.
-        ///
-        std::wstring CreateTempDirectory()
-        {
-            WCHAR tempDirectory[L_tmpnam_s];
-            ZeroMemory(tempDirectory, sizeof(tempDirectory));
-
-            errno_t err = _wtmpnam_s(tempDirectory, L_tmpnam_s);
-            if (err)
-            {
-                return L"";
-            }
-
-            long status = CreateDirectoryW(tempDirectory, NULL);
-            if (status == 0)
-            {
-                return L"";
-            }
-
-            return std::wstring(tempDirectory);
-        }
-
         ///
         /// Writes to a new or existing file. 
         ///

LogMonitor/LogMonitorTests/LogMonitorTests.vcxproj

@@ -175,6 +175,7 @@
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Create</PrecompiledHeader>
     </ClCompile>
     <ClCompile Include="ConfigFileParserTests.cpp" />
+    <ClCompile Include="Utility.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="pch.h" />

LogMonitor/LogMonitorTests/LogMonitorTests.vcxproj.filters

@@ -33,6 +33,9 @@
     <ClCompile Include="EtwMonitorTests.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="Utility.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="pch.h">

LogMonitor/LogMonitorTests/Utility.cpp

@@ -0,0 +1,38 @@
+//
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+//
+#include "pch.h"
+
+using namespace std;
+
+///
+/// Utility.cpp
+///
+/// Contains utility functions that are used across the tests
+///
+
+///
+/// Creates a new random-name directory inside the Temp directory. 
+///
+/// \return The new directory path. If an error occurs, it's empty.
+///
+wstring CreateTempDirectory()
+{
+    WCHAR tempDirectory[L_tmpnam_s];
+    ZeroMemory(tempDirectory, sizeof(tempDirectory));
+
+    errno_t err = _wtmpnam_s(tempDirectory, L_tmpnam_s);
+    if (err)
+    {
+        return L"";
+    }
+
+    long status = CreateDirectoryW(tempDirectory, NULL);
+    if (status == 0)
+    {
+        return L"";
+    }
+
+    return wstring(tempDirectory);
+}

LogMonitor/LogMonitorTests/Utility.h

@@ -0,0 +1,3 @@
+#pragma once
+
+std::wstring CreateTempDirectory();

LogMonitor/LogMonitorTests/pch.h

@@ -47,7 +47,9 @@
 #include <fstream>
 #include <streambuf>
 #include <system_error>
+#include <codecvt>
 #include "shlwapi.h"
+#include <direct.h >
 #include <io.h> 
 #include <fcntl.h> 
 #include "../src/LogMonitor/Utility.h"
@@ -59,4 +61,5 @@
 #include "../src/LogMonitor/EventMonitor.h"
 #include "../src/LogMonitor/LogFileMonitor.h"
 #include "../src/LogMonitor/ProcessMonitor.h"
+#include "Utility.h"
 #endif //PCH_H

LogMonitor/src/LogMonitor/ConfigFileParser.cpp

@@ -12,9 +12,64 @@
 /// Reads the configuration file content (as a string), parsing it with a
 /// JsonFileParser object previously created.
 ///
-/// The main entry point in this file is ReadConfigFile.
+/// The main entry point in this file is OpenConfigFile.
 ///
 
+///
+/// Open the config file and convert the document content into json
+///
+/// \param FileName       Config File name.
+///
+/// \return True if the configuration file was valid. Otherwise false
+///
+bool OpenConfigFile(_In_ const PWCHAR ConfigFileName, _Out_ LoggerSettings& Config)
+{
+    bool success;
+    std::wifstream configFileStream(ConfigFileName);
+    configFileStream.imbue(std::locale(configFileStream.getloc(),
+        new std::codecvt_utf8_utf16<wchar_t, 0x10ffff, std::little_endian>));
+
+    if (configFileStream.is_open())
+    {
+        try
+        {
+            //
+            // Convert the document content to a string, to pass it to JsonFileParser constructor.
+            //
+            std::wstring configFileStr((std::istreambuf_iterator<wchar_t>(configFileStream)),
+                std::istreambuf_iterator<wchar_t>());
+            configFileStr.erase(remove(configFileStr.begin(), configFileStr.end(), 0xFEFF), configFileStr.end());
+
+            JsonFileParser jsonParser(configFileStr);
+
+            success = ReadConfigFile(jsonParser, Config);
+        }
+        catch (std::exception& ex)
+        {
+            logWriter.TraceError(
+                Utility::FormatString(L"Failed to read json configuration file. %S", ex.what()).c_str()
+            );
+            success = false;
+        }
+        catch (...)
+        {
+            logWriter.TraceError(
+                Utility::FormatString(L"Failed to read json configuration file. Unknown error occurred.").c_str()
+            );
+            success = false;
+        }
+    } else {
+        logWriter.TraceError(
+            Utility::FormatString(
+                L"Configuration file '%s' not found. Logs will not be monitored.",
+                ConfigFileName
+            ).c_str()
+        );
+        success = false;
+    }
+
+    return success;
+}
 
 ///
 /// Read the root JSON of the config file

LogMonitor/src/LogMonitor/Main.cpp

@@ -77,179 +77,122 @@ void PrintUsage()
     wprintf(L"\tfile.\n\n");
 }
 
-
-bool StartMonitors(_In_ const PWCHAR ConfigFileName)
+void StartMonitors(_In_ LoggerSettings& settings)
 {
-    bool success;
+    std::vector<EventLogChannel> eventChannels;
+    std::vector<ETWProvider> etwProviders;
+    bool eventMonMultiLine;
+    bool eventMonStartAtOldestRecord;
+    bool etwMonMultiLine;
 
-    std::wifstream configFileStream(ConfigFileName);
-    configFileStream.imbue(std::locale(configFileStream.getloc(),
-        new std::codecvt_utf8_utf16<wchar_t, 0x10ffff, std::little_endian>));
-    if (configFileStream.is_open())
+    for (auto source : settings.Sources)
     {
-        std::vector<EventLogChannel> eventChannels;
-        std::vector<ETWProvider> etwProviders;
-        bool eventMonMultiLine;
-        bool eventMonStartAtOldestRecord;
-        bool etwMonMultiLine;
+        switch (source->Type)
+        {
+            case LogSourceType::EventLog:
+            {
+                std::shared_ptr<SourceEventLog> sourceEventLog =
+                    std::reinterpret_pointer_cast<SourceEventLog>(source);
 
-        LoggerSettings settings;
+                for (auto channel : sourceEventLog->Channels)
+                {
+                    eventChannels.push_back(channel);
+                }
 
+                eventMonMultiLine = sourceEventLog->EventFormatMultiLine;
+                eventMonStartAtOldestRecord = sourceEventLog->StartAtOldestRecord;
+
+                break;
+            }
+            case LogSourceType::File:
+            {
+                std::shared_ptr<SourceFile> sourceFile = std::reinterpret_pointer_cast<SourceFile>(source);
+
+                try
+                {
+                    std::shared_ptr<LogFileMonitor> logfileMon = make_shared<LogFileMonitor>(
+                        sourceFile->Directory,
+                        sourceFile->Filter,
+                        sourceFile->IncludeSubdirectories,
+                        sourceFile->IncludeFileNames
+                    );
+                    g_logfileMonitors.push_back(std::move(logfileMon));
+                }
+                catch (std::exception& ex)
+                {
+                    logWriter.TraceError(
+                        Utility::FormatString(
+                            L"Instantiation of a LogFileMonitor object failed for directory %ws. %S",
+                            sourceFile->Directory.c_str(),
+                            ex.what()
+                        ).c_str()
+                    );
+                }
+                catch (...)
+                {
+                    logWriter.TraceError(
+                        Utility::FormatString(
+                            L"Instantiation of a LogFileMonitor object failed for directory %ws. Unknown error occurred.",
+                            sourceFile->Directory.c_str()
+                        ).c_str()
+                    );
+                }
+
+                break;
+            }
+            case LogSourceType::ETW:
+            {
+                std::shared_ptr<SourceETW> sourceETW = std::reinterpret_pointer_cast<SourceETW>(source);
+
+                for (auto provider : sourceETW->Providers)
+                {
+                    etwProviders.push_back(provider);
+                }
+
+                etwMonMultiLine = sourceETW->EventFormatMultiLine;
+
+                break;
+            }
+        } // Switch
+    }
+
+    if (!eventChannels.empty())
+    {
         try
         {
-            //
-            // Convert the document content to a string, to pass it to JsonFileParser constructor.
-            //
-            std::wstring configFileStr((std::istreambuf_iterator<wchar_t>(configFileStream)),
-                std::istreambuf_iterator<wchar_t>());
-            configFileStr.erase(remove(configFileStr.begin(), configFileStr.end(), 0xFEFF), configFileStr.end());
-
-            JsonFileParser jsonParser(configFileStr);
-
-            success = ReadConfigFile(jsonParser, settings);
+            g_eventMon = make_unique<EventMonitor>(eventChannels, eventMonMultiLine, eventMonStartAtOldestRecord);
         }
         catch (std::exception& ex)
         {
             logWriter.TraceError(
-                Utility::FormatString(L"Failed to read json configuration file. %S", ex.what()).c_str()
+                Utility::FormatString(
+                    L"Instantiation of a EventMonitor object failed. %S",
+                    ex.what()
+                ).c_str()
             );
-            success = false;
         }
         catch (...)
         {
             logWriter.TraceError(
-                Utility::FormatString(L"Failed to read json configuration file. Unknown error occurred.").c_str()
+                Utility::FormatString(
+                    L"Instantiation of a EventMonitor object failed. Unknown error occurred."
+                ).c_str()
             );
-            success = false;
-        }
-
-        if (success)
-        {
-            for (auto source : settings.Sources)
-            {
-                switch (source->Type)
-                {
-                case LogSourceType::EventLog:
-                {
-                    std::shared_ptr<SourceEventLog> sourceEventLog =
-                        std::reinterpret_pointer_cast<SourceEventLog>(source);
-
-                    for (auto channel : sourceEventLog->Channels)
-                    {
-                        eventChannels.push_back(channel);
-                    }
-
-                    eventMonMultiLine = sourceEventLog->EventFormatMultiLine;
-                    eventMonStartAtOldestRecord = sourceEventLog->StartAtOldestRecord;
-
-                    break;
-                }
-                case LogSourceType::File:
-                {
-                    std::shared_ptr<SourceFile> sourceFile = std::reinterpret_pointer_cast<SourceFile>(source);
-
-                    try
-                    {
-                        std::shared_ptr<LogFileMonitor> logfileMon = make_shared<LogFileMonitor>(
-                            sourceFile->Directory,
-                            sourceFile->Filter,
-                            sourceFile->IncludeSubdirectories,
-                            sourceFile->IncludeFileNames
-                        );
-                        g_logfileMonitors.push_back(std::move(logfileMon));
-                    }
-                    catch (std::exception& ex)
-                    {
-                        logWriter.TraceError(
-                            Utility::FormatString(
-                                L"Instantiation of a LogFileMonitor object failed for directory %ws. %S",
-                                sourceFile->Directory.c_str(),
-                                ex.what()
-                            ).c_str()
-                        );
-                    }
-                    catch (...)
-                    {
-                        logWriter.TraceError(
-                            Utility::FormatString(
-                                L"Instantiation of a LogFileMonitor object failed for directory %ws. Unknown error occurred.",
-                                sourceFile->Directory.c_str()
-                            ).c_str()
-                        );
-                    }
-
-                    break;
-                }
-                case LogSourceType::ETW:
-                {
-                    std::shared_ptr<SourceETW> sourceETW = std::reinterpret_pointer_cast<SourceETW>(source);
-
-                    for (auto provider : sourceETW->Providers)
-                    {
-                        etwProviders.push_back(provider);
-                    }
-
-                    etwMonMultiLine = sourceETW->EventFormatMultiLine;
-
-                    break;
-                }
-                } // Switch
-            }
-        }
-        else
-        {
-            logWriter.TraceError(L"Invalid configuration file.");
-        }
-
-        if (!eventChannels.empty())
-        {
-            try
-            {
-                g_eventMon = make_unique<EventMonitor>(eventChannels, eventMonMultiLine, eventMonStartAtOldestRecord);
-            }
-            catch (std::exception& ex)
-            {
-                logWriter.TraceError(
-                    Utility::FormatString(
-                        L"Instantiation of a EventMonitor object failed. %S",
-                        ex.what()
-                    ).c_str()
-                );
-            }
-            catch (...)
-            {
-                logWriter.TraceError(
-                    Utility::FormatString(
-                        L"Instantiation of a EventMonitor object failed. Unknown error occurred."
-                    ).c_str()
-                );
-            }
-        }
-
-        if (!etwProviders.empty())
-        {
-            try
-            {
-                g_etwMon = make_unique<EtwMonitor>(etwProviders, etwMonMultiLine);
-            }
-            catch (...)
-            {
-                logWriter.TraceError(L"Invalid providers. Check them using 'logman query providers'");
-            }
         }
     }
-    else
+
+    if (!etwProviders.empty())
     {
-        logWriter.TraceError(
-            Utility::FormatString(
-                L"Configuration file '%s' not found. Logs will not be monitored.",
-                ConfigFileName
-            ).c_str()
-        );
-        success = false;
+        try
+        {
+            g_etwMon = make_unique<EtwMonitor>(etwProviders, etwMonMultiLine);
+        }
+        catch (...)
+        {
+            logWriter.TraceError(L"Invalid providers. Check them using 'logman query providers'");
+        }
     }
 
-    return success;
 }
 
 
@@ -294,7 +237,17 @@ int __cdecl wmain(int argc, WCHAR *argv[])
         }
     }
 
-    StartMonitors(configFileName);
+    LoggerSettings settings;
+    //read the config file
+    bool configFileReadSuccess = OpenConfigFile(configFileName, settings);
+
+    //start the monitors
+    if (configFileReadSuccess)
+    {
+        StartMonitors(settings);
+    } else {
+        logWriter.TraceError(L"Invalid configuration file.");
+    }
 
     //
     // Set the Ctrl handler function, that propagates the Ctrl events to the child process.

LogMonitor/src/LogMonitor/Parser/ConfigFileParser.h

@@ -13,6 +13,11 @@
 #include <memory>
 #include <variant>
 
+bool OpenConfigFile(
+    _In_ const PWCHAR ConfigFileName,
+    _Out_ LoggerSettings& Config
+);
+
 bool ReadConfigFile(
     _In_ JsonFileParser& Parser,
     _Out_ LoggerSettings& Config