Move out utility functions from platform_manager to helpers

Summary:
Move out `createDirectories` and `getStringFileContent` from `fboss::platform::platform_manager::Utils` to `fboss::platform::PlatformUtils` (under `platform/helpers/` directory)

This allows re-use outside of Platform Manager; these utility functions are not tied to PM-specific logic.

In particular this is used by following diff which adds a caching feature to `PlatformNameLib` that saves the results of `dmidecode` to a file. Without this refactor, there would be a circular dependency since `Utils::getConfig` calls `PlatformNameLib`, while `PlatformNameLib` would call `Utils::createDirectories`/`Utils::getStringFileContent`.

Reviewed By: Scott8440

Differential Revision: D61427074

fbshipit-source-id: 8ecda71c18e64d6c26c94e0046a19c8bf87f394f

fboss/platform/helpers/MockPlatformUtils.h

@@ -14,5 +14,10 @@ class MockPlatformUtils : public PlatformUtils {
       execCommand,
       (const std::string&),
       (const));
+  MOCK_METHOD(
+      (std::optional<std::string>),
+      getStringFileContent,
+      (const std::string&),
+      (const));
 };
 } // namespace facebook::fboss::platform

fboss/platform/helpers/PlatformUtils.cpp

@@ -2,10 +2,13 @@
 
 #include "fboss/platform/helpers/PlatformUtils.h"
 
+#include <filesystem>
+
 #include <folly/Subprocess.h>
 #include <folly/logging/xlog.h>
 #include <folly/system/Shell.h>
 
+namespace fs = std::filesystem;
 using namespace folly::literals::shell_literals;
 
 namespace facebook::fboss::platform {
@@ -20,4 +23,23 @@ std::pair<int, std::string> PlatformUtils::execCommand(
   return std::make_pair(exitStatus, standardOut);
 }
 
+bool PlatformUtils::createDirectories(const std::string& path) {
+  std::error_code errCode;
+  std::filesystem::create_directories(fs::path(path), errCode);
+  if (errCode.value() != 0) {
+    XLOG(ERR) << fmt::format(
+        "Received error code {} from creating path {}", errCode.value(), path);
+  }
+  return errCode.value() == 0;
+}
+
+std::optional<std::string> PlatformUtils::getStringFileContent(
+    const std::string& path) const {
+  std::string value{};
+  if (!folly::readFile(path.c_str(), value)) {
+    return std::nullopt;
+  }
+  return folly::trimWhitespace(value).str();
+}
+
 } // namespace facebook::fboss::platform

fboss/platform/helpers/PlatformUtils.h

@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include <optional>
 #include <string>
 
 namespace facebook::fboss::platform {
@@ -11,6 +12,15 @@ class PlatformUtils {
   virtual ~PlatformUtils() = default;
   // Executes command and returns exit status and standard output
   virtual std::pair<int, std::string> execCommand(const std::string& cmd) const;
+
+  // Recursively create directories for the given path.
+  // - for given /x/y/z, directory y/z if x already exists.
+  // No-op if parent directories already exist.
+  // Returns true if created or already exist, otherwise false.
+  bool createDirectories(const std::string& path);
+
+  virtual std::optional<std::string> getStringFileContent(
+      const std::string& path) const;
 };
 
 } // namespace facebook::fboss::platform

fboss/platform/helpers/tests/BUCK

@@ -13,3 +13,14 @@ cpp_unittest(
         "//fboss/platform/helpers:platform_name_lib",
     ],
 )
+
+cpp_unittest(
+    name = "platform_utils_test",
+    srcs = [
+        "PlatformUtilsTest.cpp",
+    ],
+    deps = [
+        "//fboss/platform/helpers:platform_utils",
+        "//folly/testing:test_util",
+    ],
+)

fboss/platform/helpers/tests/PlatformUtilsTest.cpp

@@ -0,0 +1,30 @@
+// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
+
+#include <filesystem>
+
+#include <folly/testing/TestUtil.h>
+#include <gtest/gtest.h>
+
+#include "fboss/platform/helpers/PlatformUtils.h"
+
+namespace fs = std::filesystem;
+using namespace ::testing;
+using namespace facebook::fboss::platform;
+
+namespace facebook::fboss::platform {
+
+TEST(PlatformUtilsTest, CreateParentDirectories) {
+  auto tmpPath = folly::test::TemporaryDirectory().path();
+  EXPECT_TRUE(PlatformUtils().createDirectories(tmpPath.string()));
+  EXPECT_TRUE(fs::exists(tmpPath.string()));
+  tmpPath = tmpPath / "a";
+  EXPECT_TRUE(PlatformUtils().createDirectories(tmpPath.string()));
+  EXPECT_TRUE(fs::exists(tmpPath.string()));
+  tmpPath = tmpPath / "b" / "c";
+  EXPECT_TRUE(PlatformUtils().createDirectories(tmpPath.string()));
+  EXPECT_TRUE(fs::exists(tmpPath.string()));
+  EXPECT_TRUE(PlatformUtils().createDirectories("/"));
+  EXPECT_FALSE(PlatformUtils().createDirectories(""));
+}
+
+} // namespace facebook::fboss::platform

fboss/platform/platform_manager/BUCK

@@ -109,6 +109,7 @@ cpp_library(
     exported_deps = [
         ":device_path_resolver",
         ":utils",
+        "//fboss/platform/helpers:platform_utils",
     ],
 )
 
@@ -140,6 +141,7 @@ cpp_library(
         ":presence_checker",
         ":utils",
         "//fb303:service_data",
+        "//fboss/platform/helpers:platform_utils",
         "//fboss/platform/weutil:fboss_eeprom_lib",
         "//fboss/platform/weutil:ioctl_smbus_eeprom_reader",
         "//folly:file_util",

fboss/platform/platform_manager/PlatformExplorer.cpp

@@ -14,6 +14,7 @@
 #include <folly/FileUtil.h>
 #include <folly/logging/xlog.h>
 
+#include "fboss/platform/helpers/PlatformUtils.h"
 #include "fboss/platform/platform_manager/Utils.h"
 #include "fboss/platform/platform_manager/gen-cpp2/platform_manager_config_constants.h"
 
@@ -38,8 +39,7 @@ std::string getSlotPath(
 // TODO: Handle hwmon/info_rom cases (by standardizing them away, if possible).
 int readVersionNumber(const std::string& path) {
   const auto versionFileContent =
-      facebook::fboss::platform ::platform_manager::Utils()
-          .getStringFileContent(path);
+      facebook::fboss::platform::PlatformUtils().getStringFileContent(path);
   if (!versionFileContent) {
     // This log is necessary to distinguish read error vs reading "0".
     XLOGF(
@@ -494,7 +494,7 @@ void PlatformExplorer::createDeviceSymLink(
     const std::string& linkPath,
     const std::string& devicePath) {
   auto linkParentPath = std::filesystem::path(linkPath).parent_path();
-  if (!Utils().createDirectories(linkParentPath.string())) {
+  if (!PlatformUtils().createDirectories(linkParentPath.string())) {
     XLOG(ERR) << fmt::format(
         "Failed to create the parent path ({})", linkParentPath.string());
     return;

fboss/platform/platform_manager/PresenceChecker.cpp

@@ -1,14 +1,16 @@
 // (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
 
 #include "fboss/platform/platform_manager/PresenceChecker.h"
-#include "fboss/platform/platform_manager/Utils.h"
 
 using namespace facebook::fboss::platform::platform_manager;
 
 PresenceChecker::PresenceChecker(
     const DevicePathResolver& devicePathResolver,
-    const std::shared_ptr<Utils> utils)
-    : devicePathResolver_(devicePathResolver), utils_(utils) {}
+    const std::shared_ptr<Utils> utils,
+    const std::shared_ptr<PlatformUtils> platformUtils)
+    : devicePathResolver_(devicePathResolver),
+      utils_(utils),
+      platformUtils_(platformUtils) {}
 
 bool PresenceChecker::isPresent(
     const PresenceDetection& presenceDetection,
@@ -39,7 +41,8 @@ bool PresenceChecker::sysfsPresent(
       *handle.presenceFileName(),
       *handle.devicePath(),
       *presencePath);
-  auto presenceFileContent = utils_->getStringFileContent(*presencePath);
+  auto presenceFileContent =
+      platformUtils_->getStringFileContent(*presencePath);
   if (!presenceFileContent) {
     throw std::runtime_error(
         fmt::format("Could not read file {}", *presencePath));

fboss/platform/platform_manager/PresenceChecker.h

@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include "fboss/platform/helpers/PlatformUtils.h"
 #include "fboss/platform/platform_manager/DevicePathResolver.h"
 #include "fboss/platform/platform_manager/Utils.h"
 
@@ -11,7 +12,9 @@ class PresenceChecker {
  public:
   explicit PresenceChecker(
       const DevicePathResolver& devicePathResolver,
-      const std::shared_ptr<Utils> utils = std::make_shared<Utils>());
+      const std::shared_ptr<Utils> utils = std::make_shared<Utils>(),
+      const std::shared_ptr<PlatformUtils> platformUtils =
+          std::make_shared<PlatformUtils>());
 
   bool isPresent(
       const PresenceDetection& presenceDetection,
@@ -20,6 +23,7 @@ class PresenceChecker {
  private:
   const DevicePathResolver& devicePathResolver_;
   const std::shared_ptr<Utils> utils_;
+  const std::shared_ptr<PlatformUtils> platformUtils_;
 
   bool sysfsPresent(const SysfsFileHandle& handle, const std::string& slotPath);
   bool gpioPresent(const GpioLineHandle& handle, const std::string& slotPath);

fboss/platform/platform_manager/Utils.cpp

@@ -88,16 +88,6 @@ PlatformConfig Utils::getConfig() {
   return config;
 }
 
-bool Utils::createDirectories(const std::string& path) {
-  std::error_code errCode;
-  std::filesystem::create_directories(fs::path(path), errCode);
-  if (errCode.value() != 0) {
-    XLOG(ERR) << fmt::format(
-        "Received error code {} from creating path {}", errCode.value(), path);
-  }
-  return errCode.value() == 0;
-}
-
 std::pair<std::string, std::string> Utils::parseDevicePath(
     const std::string& devicePath) {
   if (!ConfigValidator().isValidDevicePath(devicePath)) {
@@ -180,15 +170,6 @@ std::string Utils::resolveWatchdogCharDevPath(const std::string& sysfsPath) {
   return charDevPath;
 }
 
-std::optional<std::string> Utils::getStringFileContent(
-    const std::string& path) const {
-  std::string value{};
-  if (!folly::readFile(path.c_str(), value)) {
-    return std::nullopt;
-  }
-  return folly::trimWhitespace(value).str();
-}
-
 int Utils::getGpioLineValue(const std::string& charDevPath, int lineIndex)
     const {
   struct gpiod_chip* chip = gpiod_chip_open(charDevPath.c_str());

fboss/platform/platform_manager/Utils.h

@@ -10,11 +10,6 @@ class Utils {
  public:
   virtual ~Utils() = default;
   PlatformConfig getConfig();
-  // Recursively create directories for the given path.
-  // - for given /x/y/z, directory y/z if x already exists.
-  // No-op if parent directories already exist.
-  // Returns true if created or already exist, otherwise false.
-  bool createDirectories(const std::string& path);
 
   // Extract (SlotPath, DeviceName) from DevicePath.
   // Returns a pair of (SlotPath, DeviceName). Throws if DevicePath is invalid.
@@ -37,9 +32,6 @@ class Utils {
   // Throws an exception when it fails to resolve CharDevicePath
   std::string resolveWatchdogCharDevPath(const std::string& sysfsPath);
 
-  virtual std::optional<std::string> getStringFileContent(
-      const std::string& path) const;
-
   virtual int getGpioLineValue(const std::string& charDevPath, int lineIndex)
       const;
 };

fboss/platform/platform_manager/tests/BUCK

@@ -32,8 +32,8 @@ cpp_unittest(
     ],
     deps = [
         "//fb303:service_data",
+        "//fboss/platform/helpers:platform_utils",
         "//fboss/platform/platform_manager:platform_explorer",
-        "//fboss/platform/platform_manager:utils",
         "//folly:file_util",
         "//folly/testing:test_util",
     ],
@@ -46,7 +46,6 @@ cpp_unittest(
     ],
     deps = [
         "//fboss/platform/platform_manager:utils",
-        "//folly/testing:test_util",
     ],
 )
 
@@ -78,6 +77,7 @@ cpp_unittest(
     ],
     deps = [
         "fbsource//third-party/googletest:gmock",
+        "//fboss/platform/helpers:mock_platform_utils",
         "//fboss/platform/platform_manager:presence_checker",
         "//fboss/platform/platform_manager:utils",
     ],

fboss/platform/platform_manager/tests/PlatformExplorerTest.cpp

@@ -5,8 +5,8 @@
 #include <folly/testing/TestUtil.h>
 #include <gtest/gtest.h>
 
+#include "fboss/platform/helpers/PlatformUtils.h"
 #include "fboss/platform/platform_manager/PlatformExplorer.h"
-#include "fboss/platform/platform_manager/Utils.h"
 
 using namespace ::testing;
 using namespace facebook::fboss::platform;
@@ -18,7 +18,7 @@ void writeVersions(
     std::string deviceType,
     const char* version,
     const char* subversion) {
-  Utils().createDirectories(path);
+  PlatformUtils().createDirectories(path);
   EXPECT_TRUE(folly::writeFile(
       std::string(version),
       fmt::format("{}/{}_ver", path, deviceType).c_str()));

fboss/platform/platform_manager/tests/PresenceCheckerTest.cpp

@@ -3,10 +3,12 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "fboss/platform/helpers/MockPlatformUtils.h"
 #include "fboss/platform/platform_manager/PresenceChecker.h"
 #include "fboss/platform/platform_manager/Utils.h"
 
 using namespace ::testing;
+using namespace facebook::fboss::platform;
 using namespace facebook::fboss::platform::platform_manager;
 
 class MockDevicePathResolver : public DevicePathResolver {
@@ -33,11 +35,6 @@ class MockDevicePathResolver : public DevicePathResolver {
 class MockUtils : public Utils {
  public:
   MOCK_METHOD(int, getGpioLineValue, (const std::string&, int), (const));
-  MOCK_METHOD(
-      std::optional<std::string>,
-      getStringFileContent,
-      (const std::string&),
-      (const));
 };
 
 class PresenceCheckerTest : public testing::Test {
@@ -56,7 +53,8 @@ class PresenceCheckerTest : public testing::Test {
 
 TEST_F(PresenceCheckerTest, gpioPresent) {
   auto utils = std::make_shared<MockUtils>();
-  PresenceChecker presenceChecker(devicePathResolver_, utils);
+  auto platformUtils = std::make_shared<MockPlatformUtils>();
+  PresenceChecker presenceChecker(devicePathResolver_, utils, platformUtils);
 
   PresenceDetection presenceDetection = PresenceDetection();
   presenceDetection.gpioLineHandle() = GpioLineHandle();
@@ -84,7 +82,8 @@ TEST_F(PresenceCheckerTest, gpioPresent) {
 
 TEST_F(PresenceCheckerTest, sysfsPresent) {
   auto utils = std::make_shared<MockUtils>();
-  PresenceChecker presenceChecker(devicePathResolver_, utils);
+  auto platformUtils = std::make_shared<MockPlatformUtils>();
+  PresenceChecker presenceChecker(devicePathResolver_, utils, platformUtils);
 
   PresenceDetection presenceDetection = PresenceDetection();
   presenceDetection.sysfsFileHandle() = SysfsFileHandle();
@@ -96,11 +95,11 @@ TEST_F(PresenceCheckerTest, sysfsPresent) {
       devicePathResolver_, resolvePresencePath("device/path", "presenceName"))
       .WillRepeatedly(Return("/file/path"));
 
-  EXPECT_CALL(*utils, getStringFileContent("/file/path"))
+  EXPECT_CALL(*platformUtils, getStringFileContent("/file/path"))
       .WillOnce(Return(std::make_optional("1")));
   EXPECT_TRUE(presenceChecker.isPresent(presenceDetection, "/slot/path"));
 
-  EXPECT_CALL(*utils, getStringFileContent("/file/path"))
+  EXPECT_CALL(*platformUtils, getStringFileContent("/file/path"))
       .WillOnce(Return(std::make_optional("0")));
   EXPECT_FALSE(presenceChecker.isPresent(presenceDetection, "/slot/path"));