builds download "default" part if nothing is specified (#994)

4 files changed, 90 insertions, 24 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b7e800a86..ddfc47a5b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,7 @@
 - Improvement: Incremental hydration refreshes the modification time of existing S3 CAS entries during dehydrate so lifecycle-expiration policies do not evict objects still referenced by the module
 - Improvement: Sensitive values in command line arguments (credentials, tokens, passwords, connection strings) are scrubbed before being written to logs or sent to Sentry
 - Improvement: IMDS credential refresh log message no longer exposes the first 8 characters of the AWS AccessKeyId
+- Improvement: `zen builds download`, `zen builds ls`, and `zen builds prime-cache` now default to the part named `default` when no `--build-part-id`/`--build-part-name` is given (previously all parts were selected). Pass `--build-part-name=*` to select all parts.
 - Bugfix: `builds download` partial-block fetch decisions now account for build storage host latency
 - Bugfix: Transfer rate displays in `builds` commands now smooth correctly
 - Bugfix: Structured cache PUT errors with a detail body no longer write the HTTP response twice
diff --git a/src/zen/cmds/builds_cmd.cpp b/src/zen/cmds/builds_cmd.cpp
index 01b21a31f..820ca9c45 100644
--- a/src/zen/cmds/builds_cmd.cpp
+++ b/src/zen/cmds/builds_cmd.cpp
@@ -1536,17 +1536,12 @@ BuildsDownloadSubCmd::BuildsDownloadSubCmd(BuildsConfiguration& Config)
 
 	Opts.add_option("", "l", "local-path", "Root file system folder for build", cxxopts::value(m_Path), "<local-path>");
 	Opts.add_option("", "", "build-id", "Build Id", cxxopts::value(m_BuildId), "<id>");
-	Opts.add_option("",
-					"",
-					"build-part-id",
-					"Build part Ids list separated by ',', if no build-part-ids or build-part-names are given all parts will be downloaded",
-					cxxopts::value(m_BuildPartIds),
-					"<id>");
+	Opts.add_option("", "", "build-part-id", "Build part Ids list separated by ','.", cxxopts::value(m_BuildPartIds), "<id>");
 	Opts.add_option("",
 					"",
 					"build-part-name",
-					"Name of the build parts list separated by ',', if no build-part-ids or build-part-names are given "
-					"all parts will be downloaded",
+					"Build part names list separated by ','. If neither --build-part-id nor --build-part-name is given, "
+					"the part named 'default' is selected. Use '*' (alone) to select all parts.",
 					cxxopts::value(m_BuildPartNames),
 					"<name>");
 	Opts.add_option("",
@@ -1624,6 +1619,7 @@ BuildsDownloadSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/)
 
 	std::vector<Oid>		 BuildPartIds	= ParseBuildPartIds(m_BuildPartIds, Opts);
 	std::vector<std::string> BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts);
+	NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help());
 
 	EPartialBlockRequestMode PartialBlockRequestMode = ParseAllowPartialBlockRequests(Opts);
 
@@ -1691,17 +1687,12 @@ BuildsLsSubCmd::BuildsLsSubCmd(BuildsConfiguration& Config) : BuildsSubCmdBase(C
 	Config.AddWildcardOptions(Opts);
 
 	Opts.add_option("", "", "build-id", "Build Id", cxxopts::value(m_BuildId), "<id>");
-	Opts.add_option("",
-					"",
-					"build-part-id",
-					"Build part Ids list separated by ',', if no build-part-ids or build-part-names are given all parts will be downloaded",
-					cxxopts::value(m_BuildPartIds),
-					"<id>");
+	Opts.add_option("", "", "build-part-id", "Build part Ids list separated by ','.", cxxopts::value(m_BuildPartIds), "<id>");
 	Opts.add_option("",
 					"",
 					"build-part-name",
-					"Name of the build parts list separated by ',', if no build-part-ids or build-part-names are given "
-					"all parts will be downloaded",
+					"Build part names list separated by ','. If neither --build-part-id nor --build-part-name is given, "
+					"the part named 'default' is selected. Use '*' (alone) to select all parts.",
 					cxxopts::value(m_BuildPartNames),
 					"<name>");
 
@@ -1751,6 +1742,7 @@ BuildsLsSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/)
 
 	std::vector<Oid>		 BuildPartIds	= ParseBuildPartIds(m_BuildPartIds, Opts);
 	std::vector<std::string> BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts);
+	NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help());
 
 	std::unique_ptr<CbObjectWriter> StructuredOutput;
 	if (!m_ResultPath.empty())
@@ -1913,17 +1905,12 @@ BuildsPrimeCacheSubCmd::BuildsPrimeCacheSubCmd(BuildsConfiguration& Config)
 	Config.AddWorkerOptions(Opts);
 	Config.AddZenFolderOptions(Opts);
 	Opts.add_option("", "", "build-id", "Build Id", cxxopts::value(m_BuildId), "<id>");
-	Opts.add_option("",
-					"",
-					"build-part-id",
-					"Build part Ids list separated by ',', if no build-part-ids or build-part-names are given all parts will be downloaded",
-					cxxopts::value(m_BuildPartIds),
-					"<id>");
+	Opts.add_option("", "", "build-part-id", "Build part Ids list separated by ','.", cxxopts::value(m_BuildPartIds), "<id>");
 	Opts.add_option("",
 					"",
 					"build-part-name",
-					"Name of the build parts list separated by ',', if no build-part-ids or build-part-names are given "
-					"all parts will be downloaded",
+					"Build part names list separated by ','. If neither --build-part-id nor --build-part-name is given, "
+					"the part named 'default' is selected. Use '*' (alone) to select all parts.",
 					cxxopts::value(m_BuildPartNames),
 					"<name>");
 	Opts.add_option("",
@@ -1962,6 +1949,7 @@ BuildsPrimeCacheSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/)
 
 	std::vector<Oid>		 BuildPartIds	= ParseBuildPartIds(m_BuildPartIds, Opts);
 	std::vector<std::string> BuildPartNames = ParseBuildPartNames(m_BuildPartNames, Opts);
+	NormalizePartSelection(BuildPartIds, BuildPartNames, Opts.help());
 
 	std::uint64_t PreferredMultipartChunkSize = 32u * 1024u * 1024u;
 
diff --git a/src/zenremotestore/builds/buildstorageutil.cpp b/src/zenremotestore/builds/buildstorageutil.cpp
index 662f3f7e6..58cdfdeef 100644
--- a/src/zenremotestore/builds/buildstorageutil.cpp
+++ b/src/zenremotestore/builds/buildstorageutil.cpp
@@ -6,6 +6,7 @@
 #include <zencore/compactbinary.h>
 #include <zencore/compactbinaryutil.h>
 #include <zencore/compactbinaryvalue.h>
+#include <zencore/except.h>
 #include <zencore/fmtutils.h>
 #include <zencore/logging/broadcastsink.h>
 #include <zencore/parallelwork.h>
@@ -554,6 +555,26 @@ ResolveBuildPartNames(CbObjectView				   BuildObject,
 	return Result;
 }
 
+void
+NormalizePartSelection(std::vector<Oid>& BuildPartIds, std::vector<std::string>& BuildPartNames, std::string_view HelpText)
+{
+	const bool HasWildcard = std::find(BuildPartNames.begin(), BuildPartNames.end(), "*") != BuildPartNames.end();
+	if (HasWildcard)
+	{
+		if (BuildPartNames.size() != 1 || !BuildPartIds.empty())
+		{
+			throw OptionParseException("'*' cannot be combined with other part names or ids", std::string(HelpText));
+		}
+		BuildPartNames.clear();
+		return;
+	}
+
+	if (BuildPartIds.empty() && BuildPartNames.empty())
+	{
+		BuildPartNames.push_back("default");
+	}
+}
+
 ChunkedFolderContent
 GetRemoteContent(LoggerRef										 InLog,
 				 StorageInstance&								 Storage,
@@ -1199,6 +1220,60 @@ namespace buildstorageoperations_testutils {
 
 TEST_SUITE_BEGIN("remotestore.buildstorageutil");
 
+TEST_CASE("normalizepartselection.empty_defaults_to_default")
+{
+	std::vector<Oid>		 Ids;
+	std::vector<std::string> Names;
+	NormalizePartSelection(Ids, Names, {});
+	CHECK(Ids.empty());
+	REQUIRE_EQ(Names.size(), 1u);
+	CHECK_EQ(Names[0], "default");
+}
+
+TEST_CASE("normalizepartselection.wildcard_alone_clears_names")
+{
+	std::vector<Oid>		 Ids;
+	std::vector<std::string> Names = {"*"};
+	NormalizePartSelection(Ids, Names, {});
+	CHECK(Ids.empty());
+	CHECK(Names.empty());
+}
+
+TEST_CASE("normalizepartselection.wildcard_with_other_name_throws")
+{
+	std::vector<Oid>		 Ids;
+	std::vector<std::string> Names = {"*", "foo"};
+	CHECK_THROWS_AS(NormalizePartSelection(Ids, Names, {}), OptionParseException);
+}
+
+TEST_CASE("normalizepartselection.wildcard_with_ids_throws")
+{
+	std::vector<Oid>		 Ids   = {Oid::NewOid()};
+	std::vector<std::string> Names = {"*"};
+	CHECK_THROWS_AS(NormalizePartSelection(Ids, Names, {}), OptionParseException);
+}
+
+TEST_CASE("normalizepartselection.explicit_name_unchanged")
+{
+	std::vector<Oid>		 Ids;
+	std::vector<std::string> Names = {"foo"};
+	NormalizePartSelection(Ids, Names, {});
+	CHECK(Ids.empty());
+	REQUIRE_EQ(Names.size(), 1u);
+	CHECK_EQ(Names[0], "foo");
+}
+
+TEST_CASE("normalizepartselection.ids_only_unchanged")
+{
+	const Oid				 Id	 = Oid::NewOid();
+	std::vector<Oid>		 Ids = {Id};
+	std::vector<std::string> Names;
+	NormalizePartSelection(Ids, Names, {});
+	REQUIRE_EQ(Ids.size(), 1u);
+	CHECK_EQ(Ids[0], Id);
+	CHECK(Names.empty());
+}
+
 TEST_CASE("buildstorageoperations.upload.folder")
 {
 	using namespace buildstorageoperations_testutils;
diff --git a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
index d8b8b320d..df35f65be 100644
--- a/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
+++ b/src/zenremotestore/include/zenremotestore/builds/buildstorageutil.h
@@ -101,6 +101,8 @@ std::vector<std::pair<Oid, std::string>> ResolveBuildPartNames(CbObjectView
 															   std::span<const std::string> BuildPartNames,
 															   std::uint64_t&				OutPreferredMultipartChunkSize);
 
+void NormalizePartSelection(std::vector<Oid>& BuildPartIds, std::vector<std::string>& BuildPartNames, std::string_view HelpText);
+
 ChunkedFolderContent GetRemoteContent(LoggerRef										  InLog,
 									  StorageInstance&								  Storage,
 									  const Oid&									  BuildId,