fgerlits commented on code in PR #1634:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1634#discussion_r1317006695


##########
extensions/smb/tests/utils/TempSmbShare.h:
##########
@@ -0,0 +1,76 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+#include <filesystem>
+#include <utility>
+#include <string>
+#include "windows.h"
+#include "lm.h"
+#include "utils/OsUtils.h"
+#include "utils/expected.h"
+#include "TestUtils.h"
+#include "ListSmb.h"
+
+using namespace std::literals::chrono_literals;
+
+namespace org::apache::nifi::minifi::extensions::smb::test {
+
+class TempSmbShare {
+ public:
+  TempSmbShare(TempSmbShare&& other) = default;
+
+  ~TempSmbShare() {
+    if (!net_name_.empty())
+      NetShareDel(nullptr, net_name_.data(), 0);
+  }
+
+  static nonstd::expected<TempSmbShare, std::error_code> create(std::wstring 
net_name, std::wstring path) {
+    SHARE_INFO_502 shareInfo{};
+    std::wstring remark = L"SMB share to test SMB capabilities of minifi";
+    shareInfo.shi502_netname = net_name.data();
+    shareInfo.shi502_type = STYPE_DISKTREE;
+    shareInfo.shi502_remark = remark.data();
+    shareInfo.shi502_permissions = ACCESS_ALL;
+    shareInfo.shi502_max_uses = -1;
+    shareInfo.shi502_current_uses = 0;
+    shareInfo.shi502_path = path.data();
+    shareInfo.shi502_passwd = nullptr;
+    shareInfo.shi502_reserved = 0;
+    shareInfo.shi502_security_descriptor = nullptr;
+
+    DWORD netshare_result = NetShareAdd(nullptr, 502, 
reinterpret_cast<LPBYTE>(&shareInfo), nullptr);
+    if (netshare_result == NERR_Success) {
+      auto asd = TempSmbShare(std::move(net_name), std::move(path));
+      return asd;

Review Comment:
   was this left over from debugging?



##########
extensions/smb/tests/ListSmbTests.cpp:
##########
@@ -0,0 +1,136 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#include "TestBase.h"
+#include "Catch.h"
+#include "ListSmb.h"
+#include "Utils.h"
+#include "utils/MockSmbConnectionControllerService.h"
+#include "SingleProcessorTestController.h"
+#include "range/v3/algorithm/count_if.hpp"
+#include "range/v3/algorithm/find_if.hpp"
+#include "core/Resource.h"
+
+namespace org::apache::nifi::minifi::extensions::smb::test {
+
+REGISTER_RESOURCE(MockSmbConnectionControllerService, ControllerService);
+
+TEST_CASE("ListSmb invalid network path") {
+  const auto list_smb = std::make_shared<ListSmb>("ListSmb");
+  minifi::test::SingleProcessorTestController controller{list_smb};
+  auto smb_connection_node = 
controller.plan->addController("MockSmbConnectionControllerService", 
"smb_connection_controller_service");
+  REQUIRE(controller.plan->setProperty(smb_connection_node, 
SmbConnectionControllerService::Hostname, 
utils::OsUtils::getHostName().value_or("localhost")));
+  REQUIRE(controller.plan->setProperty(smb_connection_node, 
SmbConnectionControllerService::Share, "some_share_that_does_not_exists"));
+  REQUIRE(controller.plan->setProperty(list_smb, 
ListSmb::ConnectionControllerService, "smb_connection_controller_service"));
+  const auto trigger_results = controller.trigger();
+  CHECK(trigger_results.at(ListSmb::Success).empty());
+  CHECK(list_smb->isYield());
+}
+
+bool checkForFlowFileWithAttributes(const 
std::vector<std::shared_ptr<core::FlowFile>>& result, ListSmbExpectedAttributes 
expected_attributes) {
+  auto matching_flow_file = ranges::find_if(result, [&](const auto& flow_file) 
{ return flow_file->getAttribute(core::SpecialFlowAttribute::FILENAME) == 
expected_attributes.expected_filename; });
+  if (matching_flow_file == result.end()) {
+    return false;
+  }
+  expected_attributes.checkAttributes(**matching_flow_file);
+  return true;
+}
+
+TEST_CASE("ListSmb tests") {
+  const auto list_smb = std::make_shared<ListSmb>("ListSmb");
+  minifi::test::SingleProcessorTestController controller{list_smb};
+
+  auto smb_connection_node = 
controller.plan->addController("MockSmbConnectionControllerService", 
"smb_connection_controller_service");
+  auto mock_smb_connection_controller_service = 
std::dynamic_pointer_cast<MockSmbConnectionControllerService>(smb_connection_node->getControllerServiceImplementation());
+  REQUIRE(mock_smb_connection_controller_service);
+  
mock_smb_connection_controller_service->setPath(controller.createTempDirectory());
+
+  auto a_expected_attributes = 
mock_smb_connection_controller_service->addFile("a.foo", std::string(10_KiB, 
'a'), 5min);
+  auto b_expected_attributes = 
mock_smb_connection_controller_service->addFile("b.foo", std::string(13_KiB, 
'b'), 1h);
+  auto c_expected_attributes = 
mock_smb_connection_controller_service->addFile("c.bar", std::string(1_KiB, 
'c'), 2h);
+  auto d_expected_attributes = 
mock_smb_connection_controller_service->addFile("subdir/d.foo", 
std::string(100, 'd'), 10min);
+  auto e_expected_attributes = 
mock_smb_connection_controller_service->addFile("subdir2/e.foo", std::string(1, 
'e'), 0s);
+  auto f_expected_attributes = 
mock_smb_connection_controller_service->addFile("third/f.bar", 
std::string(50_KiB, 'f'), 30min);
+  auto g_expected_attributes = 
mock_smb_connection_controller_service->addFile("g.foo", std::string(50_KiB, 
'f'), 30min);
+  
REQUIRE_FALSE(minifi::test::utils::hide_file(mock_smb_connection_controller_service->getPath()
 / "g.foo"));

Review Comment:
   I think "false" is misleading here; I would write
   ```c++
     
REQUIRE(minifi::test::utils::hide_file(mock_smb_connection_controller_service->getPath()
 / "g.foo") == std::error_code());
   ```
   or
   ```c++
     const auto is_error = 
minifi::test::utils::hide_file(mock_smb_connection_controller_service->getPath()
 / "g.foo");
     REQUIRE_FALSE(is_error);
   ```



##########
libminifi/src/utils/OsUtils.cpp:
##########
@@ -336,4 +337,34 @@ std::optional<std::string> OsUtils::getHostName() {
   return {hostname};
 }
 
+#ifdef WIN32
+std::wstring OsUtils::stringToWideString(const std::string& string) {
+  if (string.empty())
+    return {};
+
+  const auto size_needed = MultiByteToWideChar(CP_UTF8, 0, &string.at(0), 
static_cast<int>(string.size()), nullptr, 0);

Review Comment:
   it's unlikely we'll get a string longer than 2GiB, but we could use 
`gsl::narrow` instead of `static_cast` just in case



##########
libminifi/include/utils/file/FileUtils.h:
##########
@@ -25,6 +26,7 @@
 #include <vector>
 #include <cstdio>
 #include <algorithm>
+#include "utils/expected.h"

Review Comment:
   this `#include` should be moved down, to the other `#include "utils/..."` 
lines



##########
libminifi/src/utils/OsUtils.cpp:
##########
@@ -336,4 +337,34 @@ std::optional<std::string> OsUtils::getHostName() {
   return {hostname};
 }
 
+#ifdef WIN32
+std::wstring OsUtils::stringToWideString(const std::string& string) {
+  if (string.empty())
+    return {};
+
+  const auto size_needed = MultiByteToWideChar(CP_UTF8, 0, &string.at(0), 
static_cast<int>(string.size()), nullptr, 0);
+  if (size_needed <= 0) {
+    throw std::runtime_error("MultiByteToWideChar() failed: " + 
std::to_string(size_needed));

Review Comment:
   `size_needed` is always 0 if there is an error; we should call 
`GetLastError` and include the result of that in the exception message



##########
extensions/smb/tests/ListSmbTests.cpp:
##########
@@ -0,0 +1,136 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#include "TestBase.h"
+#include "Catch.h"
+#include "ListSmb.h"
+#include "Utils.h"
+#include "utils/MockSmbConnectionControllerService.h"
+#include "SingleProcessorTestController.h"
+#include "range/v3/algorithm/count_if.hpp"
+#include "range/v3/algorithm/find_if.hpp"
+#include "core/Resource.h"
+
+namespace org::apache::nifi::minifi::extensions::smb::test {
+
+REGISTER_RESOURCE(MockSmbConnectionControllerService, ControllerService);
+
+TEST_CASE("ListSmb invalid network path") {
+  const auto list_smb = std::make_shared<ListSmb>("ListSmb");
+  minifi::test::SingleProcessorTestController controller{list_smb};
+  auto smb_connection_node = 
controller.plan->addController("MockSmbConnectionControllerService", 
"smb_connection_controller_service");
+  REQUIRE(controller.plan->setProperty(smb_connection_node, 
SmbConnectionControllerService::Hostname, 
utils::OsUtils::getHostName().value_or("localhost")));
+  REQUIRE(controller.plan->setProperty(smb_connection_node, 
SmbConnectionControllerService::Share, "some_share_that_does_not_exists"));
+  REQUIRE(controller.plan->setProperty(list_smb, 
ListSmb::ConnectionControllerService, "smb_connection_controller_service"));
+  const auto trigger_results = controller.trigger();
+  CHECK(trigger_results.at(ListSmb::Success).empty());
+  CHECK(list_smb->isYield());
+}
+
+bool checkForFlowFileWithAttributes(const 
std::vector<std::shared_ptr<core::FlowFile>>& result, ListSmbExpectedAttributes 
expected_attributes) {
+  auto matching_flow_file = ranges::find_if(result, [&](const auto& flow_file) 
{ return flow_file->getAttribute(core::SpecialFlowAttribute::FILENAME) == 
expected_attributes.expected_filename; });
+  if (matching_flow_file == result.end()) {
+    return false;
+  }
+  expected_attributes.checkAttributes(**matching_flow_file);
+  return true;
+}
+
+TEST_CASE("ListSmb tests") {
+  const auto list_smb = std::make_shared<ListSmb>("ListSmb");
+  minifi::test::SingleProcessorTestController controller{list_smb};
+
+  auto smb_connection_node = 
controller.plan->addController("MockSmbConnectionControllerService", 
"smb_connection_controller_service");
+  auto mock_smb_connection_controller_service = 
std::dynamic_pointer_cast<MockSmbConnectionControllerService>(smb_connection_node->getControllerServiceImplementation());
+  REQUIRE(mock_smb_connection_controller_service);
+  
mock_smb_connection_controller_service->setPath(controller.createTempDirectory());
+
+  auto a_expected_attributes = 
mock_smb_connection_controller_service->addFile("a.foo", std::string(10_KiB, 
'a'), 5min);
+  auto b_expected_attributes = 
mock_smb_connection_controller_service->addFile("b.foo", std::string(13_KiB, 
'b'), 1h);
+  auto c_expected_attributes = 
mock_smb_connection_controller_service->addFile("c.bar", std::string(1_KiB, 
'c'), 2h);
+  auto d_expected_attributes = 
mock_smb_connection_controller_service->addFile("subdir/d.foo", 
std::string(100, 'd'), 10min);
+  auto e_expected_attributes = 
mock_smb_connection_controller_service->addFile("subdir2/e.foo", std::string(1, 
'e'), 0s);
+  auto f_expected_attributes = 
mock_smb_connection_controller_service->addFile("third/f.bar", 
std::string(50_KiB, 'f'), 30min);
+  auto g_expected_attributes = 
mock_smb_connection_controller_service->addFile("g.foo", std::string(50_KiB, 
'f'), 30min);
+  
REQUIRE_FALSE(minifi::test::utils::hide_file(mock_smb_connection_controller_service->getPath()
 / "g.foo"));
+
+  REQUIRE((a_expected_attributes && b_expected_attributes && 
c_expected_attributes && d_expected_attributes && e_expected_attributes && 
f_expected_attributes));

Review Comment:
   missing `g_expected_attributes`



##########
libminifi/src/utils/OsUtils.cpp:
##########
@@ -336,4 +337,34 @@ std::optional<std::string> OsUtils::getHostName() {
   return {hostname};
 }
 
+#ifdef WIN32
+std::wstring OsUtils::stringToWideString(const std::string& string) {
+  if (string.empty())
+    return {};
+
+  const auto size_needed = MultiByteToWideChar(CP_UTF8, 0, &string.at(0), 
static_cast<int>(string.size()), nullptr, 0);
+  if (size_needed <= 0) {
+    throw std::runtime_error("MultiByteToWideChar() failed: " + 
std::to_string(size_needed));
+  }
+
+  std::wstring result(size_needed, 0);

Review Comment:
   nitpicking, but
   ```suggestion
     std::wstring result(size_needed, L'\0');
   ```



##########
extensions/smb/tests/utils/MockSmbConnectionControllerService.h:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+#include <utility>
+#include <string>
+#include "../../SmbConnectionControllerService.h"
+#include "core/controller/ControllerService.h"
+#include "utils/OsUtils.h"
+#include "ListSmb.h"
+#include "Catch.h"
+
+using namespace std::literals::chrono_literals;
+
+namespace org::apache::nifi::minifi::extensions::smb::test {
+
+struct ListSmbExpectedAttributes {
+  std::string expected_filename;
+  std::string expected_path;
+  std::string expected_service_location;
+  std::chrono::system_clock::time_point expected_last_modified_time;
+  std::chrono::system_clock::time_point expected_creation_time;
+  std::chrono::system_clock::time_point expected_last_access_time;
+  std::string expected_size;
+
+  void checkAttributes(core::FlowFile& flow_file) {
+    CHECK(flow_file.getAttribute(ListSmb::Filename.name) == expected_filename);
+    CHECK(flow_file.getAttribute(ListSmb::Path.name) == expected_path);
+    CHECK(flow_file.getAttribute(ListSmb::ServiceLocation.name) == 
expected_service_location);
+    auto last_modified_time_from_attribute = 
utils::timeutils::parseDateTimeStr(*flow_file.getAttribute(ListSmb::LastModifiedTime.name));
+    auto creation_time_from_attribute = 
utils::timeutils::parseDateTimeStr(*flow_file.getAttribute(ListSmb::CreationTime.name));
+    auto last_access_time_from_attribute = 
utils::timeutils::parseDateTimeStr(*flow_file.getAttribute(ListSmb::LastAccessTime.name));
+
+    CHECK(std::chrono::abs(expected_last_modified_time - 
*last_modified_time_from_attribute) < 1s);
+    CHECK(std::chrono::abs(expected_creation_time - 
*creation_time_from_attribute) < 1s);
+    CHECK(std::chrono::abs(expected_last_access_time - 
*last_access_time_from_attribute) < 1s);
+    CHECK(flow_file.getAttribute(ListSmb::Size.name) == expected_size);
+  }
+};
+
+class MockSmbConnectionControllerService : public 
SmbConnectionControllerService {
+ public:
+  using SmbConnectionControllerService::SmbConnectionControllerService;
+  EXTENSIONAPI static constexpr bool SupportsDynamicProperties = false;
+  ADD_COMMON_VIRTUAL_FUNCTIONS_FOR_CONTROLLER_SERVICES
+
+  void onEnable() override {}
+  void notifyStop() override {}
+
+  std::error_code validateConnection() override {
+    if (server_path_)
+      return {};
+    return std::make_error_code(std::errc::not_connected);
+  }
+  std::filesystem::path getPath() const override {
+    gsl_Expects(server_path_);
+    return *server_path_;
+  }
+
+  void setPath(std::filesystem::path path) { server_path_ = std::move(path);}
+
+  nonstd::expected<ListSmbExpectedAttributes, std::error_code> addFile(const 
std::filesystem::path& relative_path,
+      std::string_view content,
+      std::chrono::file_clock::duration age) {
+    auto full_path = getPath() / relative_path;
+    std::filesystem::create_directories(full_path.parent_path());
+    {
+      std::ofstream out_file(full_path, std::ios::binary | std::ios::out);
+      if (!out_file.is_open())
+        return 
nonstd::make_unexpected(std::make_error_code(std::errc::bad_file_descriptor));
+      out_file << content;
+    }
+    auto last_write_time_error = utils::file::set_last_write_time(full_path, 
std::chrono::file_clock::now() - age);
+    if (!last_write_time_error)
+      return 
nonstd::make_unexpected(std::make_error_code(std::errc::bad_file_descriptor));
+    auto current_time = std::chrono::system_clock::now();

Review Comment:
   there is a built-in inaccuracy because we call `now()` twice
   ```suggestion
       auto current_time = std::chrono::system_clock::now();
       auto last_write_time_error = utils::file::set_last_write_time(full_path, 
current_time - age);
       if (!last_write_time_error)
         return 
nonstd::make_unexpected(std::make_error_code(std::errc::bad_file_descriptor));
   ```



##########
extensions/standard-processors/processors/PutFile.cpp:
##########
@@ -61,132 +54,91 @@ void PutFile::onSchedule(core::ProcessContext *context, 
core::ProcessSessionFact
 #endif
 }
 
-void PutFile::onTrigger(core::ProcessContext *context, core::ProcessSession 
*session) {
-  if (IsNullOrEmpty(conflict_resolution_)) {
-    logger_->log_error("Conflict resolution value is invalid");
-    context->yield();
-    return;
-  }
-
-  std::shared_ptr<core::FlowFile> flowFile = session->get();
-
-  // Do nothing if there are no incoming files
-  if (!flowFile) {
-    return;
-  }
-
-  session->remove(flowFile);
-
+std::optional<std::filesystem::path> 
PutFile::getDestinationPath(core::ProcessContext& context, const 
std::shared_ptr<core::FlowFile>& flow_file) {
   std::filesystem::path directory;
-
-  if (auto directory_str = context->getProperty(Directory, flowFile)) {
+  if (auto directory_str = context.getProperty(Directory, flow_file); 
directory_str && !directory_str->empty()) {
     directory = *directory_str;
   } else {
-    logger_->log_error("Directory attribute is missing or invalid");
-  }
-
-  if (IsNullOrEmpty(directory)) {
     logger_->log_error("Directory attribute evaluated to invalid value");
-    session->transfer(flowFile, Failure);
-    return;
+    return std::nullopt;
   }
+  auto file_name_str = 
flow_file->getAttribute(core::SpecialFlowAttribute::FILENAME).value_or(flow_file->getUUIDStr());
 
-  std::string filename;
-  flowFile->getAttribute(core::SpecialFlowAttribute::FILENAME, filename);
-  auto tmpFile = tmpWritePath(filename, directory);
-
-  logger_->log_debug("PutFile using temporary file %s", tmpFile.string());
+  return directory / file_name_str;
+}
 
-  // Determine dest full file paths
-  auto destFile = directory / filename;
+bool PutFile::directoryIsFull(const std::filesystem::path& directory) const {
+  return max_dest_files_ && utils::file::is_directory(directory) && 
utils::file::countNumberOfFiles(directory) >= *max_dest_files_;

Review Comment:
   I would put the `utils::file::is_directory(directory)` check in 
`getDestinationPath()` (which should return a `nullopt` if the check fails) 
instead of here.



##########
extensions/smb/tests/SmbConnectionControllerServiceTests.cpp:
##########
@@ -0,0 +1,72 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#include "TestBase.h"
+#include "Catch.h"
+#include "SmbConnectionControllerService.h"
+#include "utils/TempSmbShare.h"
+
+namespace org::apache::nifi::minifi::extensions::smb::test {
+
+struct SmbConnectionControllerServiceFixture {
+  SmbConnectionControllerServiceFixture() = default;
+
+  TestController test_controller_{};
+  std::shared_ptr<TestPlan> plan_ = test_controller_.createPlan();
+  std::shared_ptr<minifi::core::controller::ControllerServiceNode>  
smb_connection_node_ = plan_->addController("SmbConnectionControllerService", 
"smb_connection_controller_service");
+  std::shared_ptr<SmbConnectionControllerService> smb_connection_ = 
std::dynamic_pointer_cast<SmbConnectionControllerService>(smb_connection_node_->getControllerServiceImplementation());
+};
+
+
+TEST_CASE_METHOD(SmbConnectionControllerServiceFixture, 
"SmbConnectionControllerService onEnable throws when empty") {
+  REQUIRE_THROWS(plan_->finalize());
+}
+
+TEST_CASE_METHOD(SmbConnectionControllerServiceFixture, 
"SmbConnectionControllerService anonymous connection") {
+  auto temp_directory = test_controller_.createTempDirectory();
+  auto share_local_name = temp_directory.filename().wstring();
+
+  auto temp_smb_share = TempSmbShare::create(share_local_name, 
temp_directory.wstring());
+  if (!temp_smb_share && temp_smb_share.error() == std::error_code(5, 
std::system_category())) {
+    SKIP("SmbConnectionControllerService tests needs administrator 
privileges");
+    return;

Review Comment:
   isn't `return` implicit in `SKIP`?  or does the compiler get confused 
without a `return`?



##########
extensions/smb/tests/utils/TempSmbShare.h:
##########
@@ -0,0 +1,76 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+#include <filesystem>
+#include <utility>
+#include <string>
+#include "windows.h"
+#include "lm.h"
+#include "utils/OsUtils.h"
+#include "utils/expected.h"
+#include "TestUtils.h"
+#include "ListSmb.h"
+
+using namespace std::literals::chrono_literals;
+
+namespace org::apache::nifi::minifi::extensions::smb::test {
+
+class TempSmbShare {
+ public:
+  TempSmbShare(TempSmbShare&& other) = default;
+
+  ~TempSmbShare() {
+    if (!net_name_.empty())
+      NetShareDel(nullptr, net_name_.data(), 0);
+  }
+
+  static nonstd::expected<TempSmbShare, std::error_code> create(std::wstring 
net_name, std::wstring path) {
+    SHARE_INFO_502 shareInfo{};
+    std::wstring remark = L"SMB share to test SMB capabilities of minifi";
+    shareInfo.shi502_netname = net_name.data();
+    shareInfo.shi502_type = STYPE_DISKTREE;
+    shareInfo.shi502_remark = remark.data();
+    shareInfo.shi502_permissions = ACCESS_ALL;
+    shareInfo.shi502_max_uses = -1;
+    shareInfo.shi502_current_uses = 0;
+    shareInfo.shi502_path = path.data();
+    shareInfo.shi502_passwd = nullptr;
+    shareInfo.shi502_reserved = 0;
+    shareInfo.shi502_security_descriptor = nullptr;

Review Comment:
   this could use designated initializers, too



##########
libminifi/include/utils/file/ListedFile.h:
##########
@@ -0,0 +1,104 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#pragma once
+
+#include <utility>
+#include <string>
+
+#include "../ListingStateManager.h"
+
+namespace org::apache::nifi::minifi::utils {
+
+struct FileFilter {
+  std::optional<std::regex> filename_filter;
+  std::optional<std::regex> path_filter;
+  std::optional<std::chrono::milliseconds> minimum_file_age;
+  std::optional<std::chrono::milliseconds> maximum_file_age;
+  std::optional<uint64_t> minimum_file_size;
+  std::optional<uint64_t> maximum_file_size;
+  bool ignore_hidden_files = true;
+};
+
+class ListedFile : public utils::ListedObject {
+ public:
+  explicit ListedFile(std::filesystem::path full_file_path, 
std::filesystem::path input_directory) : 
full_file_path_(std::move(full_file_path)), 
input_directory_(std::move(input_directory)) {
+    if (auto last_write_time = utils::file::last_write_time(full_file_path_)) {
+      last_modified_time_ = utils::file::to_sys(*last_write_time);
+    }
+  }
+
+  [[nodiscard]] std::chrono::system_clock::time_point getLastModified() const 
override {
+    return 
std::chrono::time_point_cast<std::chrono::milliseconds>(last_modified_time_);

Review Comment:
   why do we need this `time_point_cast`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to