Skip to content

Commit e9bf57a

Browse files
laramielcopybara-github
authored andcommitted
For Json flags, if the input does not parse as a json object try to treat it as a string.
PiperOrigin-RevId: 750371763 Change-Id: I5ad593e7e8ca6cbc11734975749ae52824503e0d
1 parent 279df3a commit e9bf57a

File tree

3 files changed

+40
-10
lines changed

3 files changed

+40
-10
lines changed

tensorstore/util/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,7 @@ tensorstore_cc_library(
438438
"@com_github_nlohmann_json//:json",
439439
"@com_google_absl//absl/flags:marshalling",
440440
"@com_google_absl//absl/status",
441+
"@com_google_absl//absl/strings",
441442
"@com_google_absl//absl/strings:str_format",
442443
],
443444
)

tensorstore/util/json_absl_flag.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include "absl/flags/marshalling.h"
2323
#include "absl/status/status.h"
24+
#include "absl/strings/ascii.h"
2425
#include "absl/strings/str_format.h"
2526
#include <nlohmann/json.hpp>
2627
#include "tensorstore/internal/json_binding/bindable.h"
@@ -58,22 +59,28 @@ struct JsonAbslFlag {
5859

5960
friend bool AbslParseFlag(std::string_view in, JsonAbslFlag* out,
6061
std::string* error) {
62+
// Whitespace is removed from JSON values: https://www.json.org/
63+
in = absl::StripAsciiWhitespace(in);
6164
if (in.empty()) {
62-
// empty string yields a default-constructed flag value.
6365
out->value = {};
6466
return true;
6567
}
6668

6769
::nlohmann::json j = ::nlohmann::json::parse(in, nullptr, false);
6870
if (j.is_discarded()) {
69-
*error = absl::StrFormat("Failed to parse JSON: '%s'", in);
70-
return false;
71+
if (in[0] == '"' || in[0] == '{' || in[0] == '[') {
72+
*error = absl::StrFormat("Failed to parse JSON: '%s'", in);
73+
return false;
74+
}
75+
j = ::nlohmann::json(std::string(in));
7176
}
77+
7278
T new_value = {};
7379
absl::Status status = internal_json_binding::DefaultBinder<>(
7480
std::true_type{}, internal_json_binding::NoOptions{}, &new_value, &j);
7581
if (!status.ok()) {
76-
*error = absl::StrFormat("Failed to bind JSON: %s", status.message());
82+
*error =
83+
absl::StrFormat("Failed to parse or bind JSON: %s", status.message());
7784
return false;
7885
}
7986
out->value = std::move(new_value);

tensorstore/util/json_absl_flag_test.cc

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,53 @@
1818

1919
#include <string>
2020

21+
#include <gmock/gmock.h>
2122
#include <gtest/gtest.h>
22-
#include "tensorstore/internal/json_binding/json_binding.h"
23+
#include "tensorstore/internal/json_binding/json_binding.h" // IWYU pragma: keep
2324
#include "tensorstore/kvstore/spec.h"
2425

2526
namespace {
2627

2728
TEST(JsonAbslFlag, IntFlag) {
2829
// Validate that the default value can roundtrip.
2930
tensorstore::JsonAbslFlag<int64_t> flag = {};
30-
std::string default_value = AbslUnparseFlag(flag);
31+
EXPECT_THAT(AbslUnparseFlag(flag), ::testing::StrEq("0"));
3132

3233
std::string error;
33-
EXPECT_TRUE(AbslParseFlag(default_value, &flag, &error));
34+
EXPECT_TRUE(AbslParseFlag("", &flag, &error));
3435
EXPECT_TRUE(error.empty());
36+
37+
EXPECT_TRUE(AbslParseFlag("1", &flag, &error));
38+
EXPECT_TRUE(error.empty()) << error;
3539
}
3640

3741
TEST(JsonAbslFlag, KvStoreSpecFlag) {
3842
// Validate that the default value can roundtrip.
3943
tensorstore::JsonAbslFlag<tensorstore::kvstore::Spec> flag = {};
40-
std::string default_value = AbslUnparseFlag(flag);
44+
EXPECT_THAT(AbslUnparseFlag(flag), ::testing::IsEmpty());
4145

46+
// Try to parse as an empty value JSON string.
4247
std::string error;
43-
EXPECT_TRUE(AbslParseFlag(default_value, &flag, &error))
44-
<< "value: " << default_value;
48+
EXPECT_TRUE(AbslParseFlag("", &flag, &error));
49+
EXPECT_TRUE(error.empty()) << error;
50+
51+
EXPECT_TRUE(AbslParseFlag(" ", &flag, &error));
4552
EXPECT_TRUE(error.empty()) << error;
53+
54+
// Try to parse a bad json object.
55+
error.clear();
56+
EXPECT_FALSE(AbslParseFlag("{ \"driver\": \"memory\" ", &flag, &error));
57+
EXPECT_THAT(error, testing::HasSubstr("Failed to parse JSON"));
58+
59+
// Try to parse as a json object.
60+
error.clear();
61+
EXPECT_FALSE(AbslParseFlag("{ \"driver\": \"memory\" }", &flag, &error));
62+
EXPECT_THAT(error, testing::HasSubstr("Failed to parse or bind JSON"));
63+
64+
// Try to parse as a raw string.
65+
error.clear();
66+
EXPECT_FALSE(AbslParseFlag("memory://", &flag, &error));
67+
EXPECT_THAT(error, testing::HasSubstr("Failed to parse or bind JSON"));
4668
}
4769

4870
} // namespace

0 commit comments

Comments
 (0)