remotestore op refactorings (#637)

* broke out BuildLogOutput to separate file
* refactored out GetBlockDescriptions
* log progress improvements
* refactorings to accomodate oplog download operations

3 files changed, 138 insertions, 90 deletions

diff --git a/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h b/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h
index a409d08ec..6eef794fd 100644
--- a/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h
+++ b/src/zenremotestore/include/zenremotestore/builds/buildstorageoperations.h
@@ -21,13 +21,14 @@ namespace zen {
 
 class CloneQueryInterface;
 
+class OperationLogOutput;
 class BuildStorageBase;
-class BuildStorageCache;
 class HttpClient;
 class ParallelWork;
 class WorkerThreadPool;
 class FilteredRate;
 class ReadFileCache;
+struct StorageInstance;
 
 class BufferedWriteFileCache;
 struct ChunkBlockDescription;
@@ -106,16 +107,6 @@ struct RebuildFolderStateStatistics
 	uint64_t			  FinalizeTreeElapsedWallTimeUs = 0;
 };
 
-struct StorageInstance
-{
-	std::unique_ptr<HttpClient>		   BuildStorageHttp;
-	std::unique_ptr<BuildStorageBase>  BuildStorage;
-	std::string						   StorageName;
-	std::unique_ptr<HttpClient>		   CacheHttp;
-	std::unique_ptr<BuildStorageCache> BuildCacheStorage;
-	std::string						   CacheName;
-};
-
 enum EPartialBlockRequestMode
 {
 	Off,
@@ -130,56 +121,6 @@ EPartialBlockRequestMode PartialBlockRequestModeFromString(const std::string_vie
 std::filesystem::path ZenStateFilePath(const std::filesystem::path& ZenFolderPath);
 std::filesystem::path ZenTempFolderPath(const std::filesystem::path& ZenFolderPath);
 
-class BuildOpLogOutput
-{
-public:
-	virtual void EmitLogMessage(int LogLevel, std::string_view Format, fmt::format_args Args) = 0;
-
-	virtual void	 SetLogOperationName(std::string_view Name)						 = 0;
-	virtual void	 SetLogOperationProgress(uint32_t StepIndex, uint32_t StepCount) = 0;
-	virtual uint32_t GetProgressUpdateDelayMS()										 = 0;
-
-	class ProgressBar
-	{
-	public:
-		struct State
-		{
-			bool		operator==(const State&) const = default;
-			std::string Task;
-			std::string Details;
-			uint64_t	TotalCount	   = 0;
-			uint64_t	RemainingCount = 0;
-			enum class EStatus
-			{
-				Running,
-				Aborted,
-				Paused
-			};
-			EStatus Status = EStatus::Running;
-
-			static EStatus CalculateStatus(bool IsAborted, bool IsPaused)
-			{
-				if (IsAborted)
-				{
-					return EStatus::Aborted;
-				}
-				if (IsPaused)
-				{
-					return EStatus::Paused;
-				}
-				return EStatus::Running;
-			}
-		};
-
-		virtual ~ProgressBar() {}
-
-		virtual void UpdateState(const State& NewState, bool DoLinebreak) = 0;
-		virtual void Finish()											  = 0;
-	};
-
-	virtual ProgressBar* CreateProgressBar(std::string_view InSubTask) = 0;
-};
-
 class BuildsOperationUpdateFolder
 {
 public:
@@ -203,7 +144,7 @@ public:
 		std::vector<std::string> ExcludeExtensions;
 	};
 
-	BuildsOperationUpdateFolder(BuildOpLogOutput&						  LogOutput,
+	BuildsOperationUpdateFolder(OperationLogOutput&						  OperationLogOutput,
 								StorageInstance&						  Storage,
 								std::atomic<bool>&						  AbortFlag,
 								std::atomic<bool>&						  PauseFlag,
@@ -437,7 +378,7 @@ private:
 	void				  FinalizeChunkSequences(std::span<const uint32_t> RemoteSequenceIndexes);
 	void				  VerifySequence(uint32_t RemoteSequenceIndex);
 
-	BuildOpLogOutput&						  m_LogOutput;
+	OperationLogOutput&						  m_LogOutput;
 	StorageInstance&						  m_Storage;
 	std::atomic<bool>&						  m_AbortFlag;
 	std::atomic<bool>&						  m_PauseFlag;
@@ -572,7 +513,7 @@ public:
 
 		std::vector<std::string> NonCompressableExtensions;
 	};
-	BuildsOperationUploadFolder(BuildOpLogOutput&			 LogOutput,
+	BuildsOperationUploadFolder(OperationLogOutput&			 OperationLogOutput,
 								StorageInstance&			 Storage,
 								std::atomic<bool>&			 AbortFlag,
 								std::atomic<bool>&			 PauseFlag,
@@ -685,15 +626,15 @@ private:
 								  uint32_t					  ChunkIndex,
 								  LooseChunksStatistics&	  TempLooseChunksStats);
 
-	BuildOpLogOutput&  m_LogOutput;
-	StorageInstance&   m_Storage;
-	std::atomic<bool>& m_AbortFlag;
-	std::atomic<bool>& m_PauseFlag;
-	WorkerThreadPool&  m_IOWorkerPool;
-	WorkerThreadPool&  m_NetworkPool;
-	const Oid		   m_BuildId;
-	const Oid		   m_BuildPartId;
-	const std::string  m_BuildPartName;
+	OperationLogOutput& m_LogOutput;
+	StorageInstance&	m_Storage;
+	std::atomic<bool>&	m_AbortFlag;
+	std::atomic<bool>&	m_PauseFlag;
+	WorkerThreadPool&	m_IOWorkerPool;
+	WorkerThreadPool&	m_NetworkPool;
+	const Oid			m_BuildId;
+	const Oid			m_BuildPartId;
+	const std::string	m_BuildPartName;
 
 	const std::filesystem::path m_Path;
 	const std::filesystem::path m_ManifestPath;
@@ -723,7 +664,7 @@ public:
 		bool IsQuiet   = false;
 		bool IsVerbose = false;
 	};
-	BuildsOperationValidateBuildPart(BuildOpLogOutput&		LogOutput,
+	BuildsOperationValidateBuildPart(OperationLogOutput&	OperationLogOutput,
 									 BuildStorageBase&		Storage,
 									 std::atomic<bool>&		AbortFlag,
 									 std::atomic<bool>&		PauseFlag,
@@ -745,16 +686,16 @@ private:
 											 uint64_t&	   OutCompressedSize,
 											 uint64_t&	   OutDecompressedSize);
 
-	BuildOpLogOutput&  m_LogOutput;
-	BuildStorageBase&  m_Storage;
-	std::atomic<bool>& m_AbortFlag;
-	std::atomic<bool>& m_PauseFlag;
-	WorkerThreadPool&  m_IOWorkerPool;
-	WorkerThreadPool&  m_NetworkPool;
-	const Oid		   m_BuildId;
-	Oid				   m_BuildPartId;
-	const std::string  m_BuildPartName;
-	const Options	   m_Options;
+	OperationLogOutput& m_LogOutput;
+	BuildStorageBase&	m_Storage;
+	std::atomic<bool>&	m_AbortFlag;
+	std::atomic<bool>&	m_PauseFlag;
+	WorkerThreadPool&	m_IOWorkerPool;
+	WorkerThreadPool&	m_NetworkPool;
+	const Oid			m_BuildId;
+	Oid					m_BuildPartId;
+	const std::string	m_BuildPartName;
+	const Options		m_Options;
 };
 
 class BuildsOperationPrimeCache
@@ -770,7 +711,7 @@ public:
 		bool				  ForceUpload				  = false;
 	};
 
-	BuildsOperationPrimeCache(BuildOpLogOutput&				 LogOutput,
+	BuildsOperationPrimeCache(OperationLogOutput&			 OperationLogOutput,
 							  StorageInstance&				 Storage,
 							  std::atomic<bool>&			 AbortFlag,
 							  std::atomic<bool>&			 PauseFlag,
@@ -785,7 +726,7 @@ public:
 	DownloadStatistics m_DownloadStats;
 
 private:
-	BuildOpLogOutput&	  m_LogOutput;
+	OperationLogOutput&	  m_LogOutput;
 	StorageInstance&	  m_Storage;
 	std::atomic<bool>&	  m_AbortFlag;
 	std::atomic<bool>&	  m_PauseFlag;
diff --git a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
index 258266a6a..3816822be 100644
--- a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
+++ b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
@@ -6,12 +6,12 @@
 #include <zenhttp/httpclient.h>
 #include <zenremotestore/builds/buildstorage.h>
 
-namespace cxxopts {
-class Options;
-}
-
 namespace zen {
 
+class OperationLogOutput;
+class BuildStorageBase;
+class BuildStorageCache;
+
 struct BuildStorageResolveResult
 {
 	std::string HostUrl;
@@ -37,4 +37,24 @@ BuildStorageResolveResult ResolveBuildStorage(const HttpClientSettings& ClientSe
 											  std::string_view			ZenCacheHost,
 											  ZenCacheResolveMode		ZenResolveMode);
 
+std::vector<ChunkBlockDescription> GetBlockDescriptions(OperationLogOutput&		Output,
+														BuildStorageBase&		Storage,
+														BuildStorageCache*		OptionalCacheStorage,
+														const Oid&				BuildId,
+														const Oid&				BuildPartId,
+														std::span<const IoHash> BlockRawHashes,
+														bool					AttemptFallback,
+														bool					IsQuiet,
+														bool					IsVerbose);
+
+struct StorageInstance
+{
+	std::unique_ptr<HttpClient>		   BuildStorageHttp;
+	std::unique_ptr<BuildStorageBase>  BuildStorage;
+	std::string						   StorageName;
+	std::unique_ptr<HttpClient>		   CacheHttp;
+	std::unique_ptr<BuildStorageCache> BuildCacheStorage;
+	std::string						   CacheName;
+};
+
 }  // namespace zen
diff --git a/src/zenremotestore/include/zenremotestore/operationlogoutput.h b/src/zenremotestore/include/zenremotestore/operationlogoutput.h
new file mode 100644
index 000000000..b84d77703
--- /dev/null
+++ b/src/zenremotestore/include/zenremotestore/operationlogoutput.h
@@ -0,0 +1,87 @@
+// Copyright Epic Games, Inc. All Rights Reserved.
+
+#pragma once
+
+#include <zencore/fmtutils.h>
+
+namespace zen {
+
+class OperationLogOutput
+{
+public:
+	virtual void EmitLogMessage(int LogLevel, std::string_view Format, fmt::format_args Args) = 0;
+
+	virtual void	 SetLogOperationName(std::string_view Name)						 = 0;
+	virtual void	 SetLogOperationProgress(uint32_t StepIndex, uint32_t StepCount) = 0;
+	virtual uint32_t GetProgressUpdateDelayMS()										 = 0;
+
+	class ProgressBar
+	{
+	public:
+		struct State
+		{
+			bool		operator==(const State&) const = default;
+			std::string Task;
+			std::string Details;
+			uint64_t	TotalCount	   = 0;
+			uint64_t	RemainingCount = 0;
+			enum class EStatus
+			{
+				Running,
+				Aborted,
+				Paused
+			};
+			EStatus Status = EStatus::Running;
+
+			static EStatus CalculateStatus(bool IsAborted, bool IsPaused)
+			{
+				if (IsAborted)
+				{
+					return EStatus::Aborted;
+				}
+				if (IsPaused)
+				{
+					return EStatus::Paused;
+				}
+				return EStatus::Running;
+			}
+		};
+
+		virtual ~ProgressBar() {}
+
+		virtual void UpdateState(const State& NewState, bool DoLinebreak) = 0;
+		virtual void Finish()											  = 0;
+	};
+
+	virtual ProgressBar* CreateProgressBar(std::string_view InSubTask) = 0;
+};
+
+#define ZEN_OPERATION_LOG(OutputTarget, InLevel, fmtstr, ...)                                         \
+	do                                                                                                \
+	{                                                                                                 \
+		using namespace std::literals;                                                                \
+		ZEN_CHECK_FORMAT_STRING(fmtstr##sv, ##__VA_ARGS__);                                           \
+		OutputTarget.EmitLogMessage(InLevel, fmtstr, zen::logging::LogCaptureArguments(__VA_ARGS__)); \
+	} while (false)
+
+#define ZEN_OPERATION_LOG_INFO(OutputTarget, fmtstr, ...) \
+	ZEN_OPERATION_LOG((OutputTarget), zen::logging::level::Info, fmtstr, ##__VA_ARGS__)
+#define ZEN_OPERATION_LOG_DEBUG(OutputTarget, fmtstr, ...) \
+	ZEN_OPERATION_LOG((OutputTarget), zen::logging::level::Debug, fmtstr, ##__VA_ARGS__)
+#define ZEN_OPERATION_LOG_WARN(OutputTarget, fmtstr, ...) \
+	ZEN_OPERATION_LOG((OutputTarget), zen::logging::level::Warn, fmtstr, ##__VA_ARGS__)
+
+class ConsoleLogOutput : public OperationLogOutput
+{
+public:
+	ConsoleLogOutput();
+	virtual void EmitLogMessage(int LogLevel, std::string_view Format, fmt::format_args Args) override;
+
+	virtual void	 SetLogOperationName(std::string_view Name) override;
+	virtual void	 SetLogOperationProgress(uint32_t StepIndex, uint32_t StepCount) override;
+	virtual uint32_t GetProgressUpdateDelayMS() override;
+
+	virtual ProgressBar* CreateProgressBar(std::string_view InSubTask) override;
+};
+
+}  // namespace zen