Logger format improvements (#78)

* Consolidate spacing in logger class
* Configurable formats
* Support microseconds
* Remove ryan from log names
* Make new logger values configurable
* Update config sample with log format options
* Fix null/default config handling for log formatting
* Remove readme section
* Basic tests for logger
* Fix NS Client Logger construction

.env.example

@@ -18,6 +18,12 @@ NETSUITE_PASSWORD="mySecretPwd"
 NETSUITE_APP_ID="AF44ECC6-6691-400C-85A2-9FD7BFABE36E"
 NETSUITE_ROLE="1"
 
-# Logging Options
+## Logging Configuration
+# Boolean to control whether to create log files.
 #NETSUITE_LOGGING=false
+# The directory where log files will be written.
 #NETSUITE_LOG_PATH=
+# The filename format. %date and %operation are the only usable tokens.
+#NETSUITE_LOG_FORMAT="netsuite-php-%date-%operation"
+# The DateTime-compatible format used to build the filename
+#NETSUITE_LOG_DATEFORMAT="Ymd.His.u"

.gitignore

@@ -7,3 +7,4 @@
 .DS_Store
 composer.lock
 *.bat
+.phpunit.result.cache

README.md

@@ -63,7 +63,9 @@ $config = [
     // optional -------------------------------------
     "signatureAlgorithm" => 'sha256', // Defaults to 'sha256'
     "logging"  => true,
-    "log_path" => "/var/www/myapp/logs/netsuite"
+    "log_path" => "/var/www/myapp/logs/netsuite",
+    "log_format"     => "netsuite-php-%date-%operation",
+    "log_dateformat" => "Ymd.His.u",
 ];
 $service = new NetSuiteService($config);
 ```
@@ -89,7 +91,7 @@ $config = [
     "app_id"   => "4AD027CA-88B3-46EC-9D3E-41C6E6A325E2",
     // optional -------------------------------------
     "logging"  => true,
-    "log_path" => "/var/www/myapp/logs/netsuite"
+    "log_path" => "/var/www/myapp/logs/netsuite",
 ];
 $service = new NetSuiteService($config);
 ```

composer.json

@@ -18,7 +18,9 @@
     "paragonie/random_compat": ">=1"
   },
   "require-dev": {
-    "phpspec/phpspec": ">=5"
+    "phpspec/phpspec": ">=5",
+    "phpunit/phpunit": "^9.5",
+    "mikey179/vfsstream": "^1.6"
   },
   "suggest": {
     "vlucas/phpdotenv": "Provides loading a .env file for obtaining config variables",
@@ -32,6 +34,12 @@
       "src/includes/functions.php"
     ]
   },
+  "autoload-dev": {
+    "psr-4": {
+      "spec\\NetSuite\\": "spec",
+      "tests\\Netsuite\\": "tests"
+    }
+  },
   "minimum-stability": "stable",
   "scripts": {
     "generate": "php utilities/separate_classes.php"

phpunit.xml.dist

@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit bootstrap="./vendor/autoload.php"
+         colors="true"
+         convertErrorsToExceptions="true"
+         convertNoticesToExceptions="true"
+         convertWarningsToExceptions="true"
+         stopOnFailure="false">
+    <testsuites>
+        <testsuite name="Tests">
+            <directory>./tests/</directory>
+        </testsuite>
+    </testsuites>
+
+    <filter>
+        <whitelist processUncoveredFilesFromWhitelist="true">
+            <directory suffix=".php">./src/</directory>
+        </whitelist>
+    </filter>
+</phpunit>

samples/config.php

@@ -1,14 +1,16 @@
 <?php
 
 // An example of loading your config from environment variables with optional defaults.
-return [
-    'endpoint' => getenv('NETSUITE_ENDPOINT') ?: '2021_1',
-    'host'     => getenv('NETSUITE_HOST')     ?: 'https://webservices.netsuite.com',
-    'email'    => getenv('NETSUITE_EMAIL')    ?: '[email protected]',
-    'password' => getenv('NETSUITE_PASSWORD') ?: 'mySecretPwd',
-    'role'     => getenv('NETSUITE_ROLE')     ?: '3',
-    'account'  => getenv('NETSUITE_ACCOUNT')  ?: 'MYACCT1',
-    'appid'    => getenv('NETSUITE_APP_ID')   ?: '4AD027CA-88B3-46EC-9D3E-41C6E6A325E2',
-    'logging'  => getenv('NETSUITE_LOGGING')  ?: false,
-    'log_path' => getenv('NETSUITE_LOG_PATH') ?: '',
-];
+return array(
+    'endpoint'       => getenv('NETSUITE_ENDPOINT')       ?: '2021_1',
+    'host'           => getenv('NETSUITE_HOST')           ?: 'https://webservices.netsuite.com',
+    'email'          => getenv('NETSUITE_EMAIL')          ?: '[email protected]',
+    'password'       => getenv('NETSUITE_PASSWORD')       ?: 'mySecretPwd',
+    'role'           => getenv('NETSUITE_ROLE')           ?: '3',
+    'account'        => getenv('NETSUITE_ACCOUNT')        ?: 'MYACCT1',
+    'appid'          => getenv('NETSUITE_APP_ID')         ?: '4AD027CA-88B3-46EC-9D3E-41C6E6A325E2',
+    'logging'        => getenv('NETSUITE_LOGGING')        ?: false,
+    'log_path'       => getenv('NETSUITE_LOG_PATH')       ?: '',
+    'log_format'     => getenv('NETSUITE_LOG_FORMAT')     ?: 'netsuite-php-%date-%operation',
+    'log_dateformat' => getenv('NETSUITE_LOG_DATEFORMAT') ?: 'Ymd.His.u',
+);

src/Logger.php

@@ -6,9 +6,36 @@ class Logger
 {
     private $path;
 
-    public function __construct($path = null)
+    /**
+     * @var string
+     */
+    private $format;
+
+    /**
+     * @var string
+     */
+    private $dateFormat;
+
+    const DEFAULT_LOG_FORMAT = 'netsuite-php-%date-%operation';
+
+    const DEFAULT_DATE_FORMAT = 'Ymd.His.u';
+
+    public function __construct($path = null, $format = self::DEFAULT_LOG_FORMAT, $dateFormat = self::DEFAULT_DATE_FORMAT)
+    {
+        $this->path = $path ?: __DIR__ . '/../logs';
+        $this->format = $format;
+        $this->dateFormat = $dateFormat;
+    }
+
+    /**
+     * Update the logging path in the Logger object
+     *
+     * @param string $path
+     * @return void
+     */
+    public function setPath($path)
     {
-        $this->path = $path ?: __DIR__.'/../logs';
+        $this->path = $path;
     }
 
     /**
@@ -20,11 +47,14 @@ public function __construct($path = null)
     public function logSoapCall($client, $operation)
     {
         if (file_exists($this->path)) {
-            $fileName = "ryanwinchester-netsuite-php-" . date("Ymd.His") . "-" . $operation;
-            $logFile = $this->path ."/". $fileName;
+            $fileName = strtr($this->format, [
+                '%date' => (new \DateTime())->format($this->dateFormat),
+                '%operation' => $operation,
+            ]);
+            $logFile = $this->path . '/' . $fileName;
 
             // REQUEST
-            $request = $logFile . "-request.xml";
+            $request = $logFile . '-request.xml';
             $Handle = fopen($request, 'w');
             $Data = $client->__getLastRequest();
             $Data = cleanUpNamespaces($Data);
@@ -42,16 +72,16 @@ public function logSoapCall($client, $operation)
                 '//socialSecurityNumber',
             ];
 
-            $privateFields = $xml->xpath(implode(" | ", $privateFieldXpaths));
+            $privateFields = $xml->xpath(implode(' | ', $privateFieldXpaths));
 
             foreach ($privateFields as &$field) {
-                $field[0] = "[Content Removed for Security Reasons]";
+                $field[0] = '[Content Removed for Security Reasons]';
             }
 
             $stringCustomFields = $xml->xpath("//customField[@xsitype='StringCustomFieldRef']");
 
             foreach ($stringCustomFields as $field) {
-                $field->value = "[Content Removed for Security Reasons]";
+                $field->value = '[Content Removed for Security Reasons]';
             }
 
             $xml_string = str_replace('xsitype', 'xsi:type', $xml->asXML());
@@ -60,7 +90,7 @@ public function logSoapCall($client, $operation)
             fclose($Handle);
 
             // RESPONSE
-            $response = $logFile . "-response.xml";
+            $response = $logFile . '-response.xml';
             $Handle = fopen($response, 'w');
             $Data = $client->__getLastResponse();
 

src/NetSuiteClient.php

@@ -64,9 +64,10 @@ public function __construct($config = null, $options = [], $client = null)
           $this->client = $client;
         }
         $this->logger = new Logger(
-            isset($this->config['log_path']) ? $this->config['log_path'] : NULL
+            !empty($this->config['log_path']) ? $this->config['log_path'] : NULL,
+            !empty($this->config['log_format']) ? $this->config['log_format'] : Logger::DEFAULT_LOG_FORMAT,
+            !empty($this->config['log_dateformat']) ? $this->config['log_dateformat'] : Logger::DEFAULT_DATE_FORMAT
         );
-
     }
 
     /**
@@ -103,6 +104,8 @@ public static function getEnvConfig()
             'app_id'             => getenv('NETSUITE_APP_ID') ?: '4AD027CA-88B3-46EC-9D3E-41C6E6A325E2',
             'logging'            => getenv('NETSUITE_LOGGING'),
             'log_path'           => getenv('NETSUITE_LOG_PATH'),
+            'log_format'         => getenv('NETSUITE_LOG_FORMAT'),
+            'log_dateformat'     => getenv('NETSUITE_LOG_DATEFORMAT'),
         ];
 
         // These config keys aren't required by all users, but if they are
@@ -420,7 +423,7 @@ public function logRequests($on = true)
     public function setLogPath($logPath)
     {
         $this->config['log_path'] = $logPath;
-        $this->logger = new Logger($logPath);
+        $this->logger->setPath($logPath);
     }
 
     /**

tests/LoggerTest.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace tests\Netsuite;
+
+use NetSuite\Logger;
+use org\bovigo\vfs\vfsStream;
+use PHPUnit\Framework\TestCase;
+use Prophecy\PhpUnit\ProphecyTrait;
+
+/**
+ * @coversDefault \NetSuite\Logger
+ */
+class LoggerTest extends TestCase
+{
+    /**
+     * @var \org\bovigo\vfs\vfsStreamDirectory
+     */
+    private $root;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+        $this->root = vfsStream::setup('/app/log');
+    }
+
+    public function testLogSoapCall()
+    {
+        $logger = new Logger('vfs://app/log/');
+        $logger->logSoapCall($this->getSoapClient(), 'upsert');
+
+        // Find logs in log directory. scandir() + hack because glob() doesn't support stream wrappers.
+        $files = scandir('vfs://app/log/');
+        $files = array_filter($files, function (string $file) { return 0 !== strncmp($file, '.', 1); });
+        $this->assertCount(2, $files);
+
+        // Find the date.
+        $first = current($files);
+        preg_match('/netsuite-php-(.*)-upsert/', $first, $matches);
+        $this->assertNotEmpty($matches, 'Log matches format');
+        $timestamp = $matches[1];
+
+        $this->assertFileExists( "vfs://app/log/netsuite-php-$timestamp-upsert-request.xml");
+        $this->assertFileExists( "vfs://app/log/netsuite-php-$timestamp-upsert-response.xml");
+        $this->assertEquals(file_get_contents("vfs://app/log/netsuite-php-$timestamp-upsert-request.xml"), "<?xml version=\"1.0\"?>\n<request/>\n");
+        // Should this be pretty and cleaned up too?
+        $this->assertEquals(file_get_contents("vfs://app/log/netsuite-php-$timestamp-upsert-response.xml"), "<response/>");
+
+        // @todo assert sanitization.
+    }
+
+    public function testSetPath()
+    {
+        $logger = new Logger(null, 'file');
+        $logger->setPath($this->root->url());
+        $logger->logSoapCall($this->getSoapClient(), 'upsert');
+        $this->assertFileExists($this->root->url() . '/file-request.xml');
+        $this->assertFileExists($this->root->url() . '/file-response.xml');
+    }
+
+
+    private function getSoapClient($request = '<request/>', $response = '<response/>')
+    {
+        // This would be easier with prophecy, but it doesn't support "virtual private" methods.
+        // @see https://github.com/phpspec/prophecy/issues/108
+        $client = $this->getMockBuilder(\SoapClient::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $client
+            ->method('__getLastRequest')
+            ->willReturn($request);
+        $client
+            ->method('__getLastResponse')
+            ->willReturn($response);
+        return $client;
+    }
+
+}