mirror of
https://github.com/facebook/rocksdb.git
synced 2026-07-07 14:47:40 +08:00
Peter Dillinger
9ef369c606
Summary:
StringToMap previously stripped the outer braces from nested values,
making single-entry round-trip via `key=value;` (e.g. SetOptions)
silently corrupt values that contain ';'. For example, a filter_policy
with sub-options serialized as
filter_policy={id=ribbonfilter:10:-1;bloom_before_level=-1;}
parsed to map[filter_policy] = "id=ribbonfilter:10:-1;bloom_before_level=-1;",
and embedding that map entry directly into another `key=value;` string
re-exposed the inner ';' as a top-level delimiter.
This change is a step toward the "essential view": the outer `{...}` of
a nested value is part of the value's identity. StringToMap preserves it, so
each map entry is in self-contained form (a simple value or a single
balanced `{...}` block) and can be embedded directly without further
escaping by the caller. However some permissiveness is kept for
compatibility: list-style typed parsers (ParseVector, ParseArray,
kStringMap, the listener parser) and scalar dispatch in
OptionTypeInfo::Parse accept either the bare or the wrapped form via a
new OptionTypeInfo::StripOuterBraces helper that peels at most one
outer-pair-matched layer. So braced scalar input like `key={42};` and
`key={true};` still parses, and `key={};` denotes an empty list,
distinct from `key={{}};` which denotes a one-element list with an
empty element.
A new public MapToString is added to convenience.h as the symmetric
inverse of StringToMap: a trivial `key=value;` joiner that relies on
each value already being self-contained (which StringToMap guarantees).
Also fixed and refactored our trim() function because it would do the wrong
thing for a single space. This problem was detected by expanding the
StringToMapRandomTest based on code review feedback, and should only
improve existing callers to trim().
The OPTIONS-file serializer code is untouched, so the on-disk format
is byte-for-byte identical and format-compatibility is preserved.
Bonus: fixes / clarifications to CLAUDE.md's build-system note.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14805
Test Plan:
db_bloom_filter_test extends MutableFilterPolicy with the OPTIONS-file
format for filter_policy to confirm that form round-trips through
SetOptions.
options_test updates StringToMapTest expectations for the new
brace-preserving behavior, and adds:
- MapToStringTest covering the new joiner.
- StringToMapMapToStringRoundTripTest including a single-entry
pull-and-embed scenario that previously corrupted on round-trip.
- FullOptionsStringToMapRoundTripTest that drives a populated
DBOptions and CFOptions through GetStringFromXxx -> StringToMap ->
per-entry single round-trip -> MapToString -> GetXxxFromString ->
VerifyXxxOptions, catching any custom serializer that emits a
value with ';' without enclosing it in '{}'.
- EmptyBracedVectorAndBracedScalarTest covering `key={};` (empty
list) and braced scalar input like `key={42};`, `key={true};`.
- Expanded StringToMapRandomTest with round-tripping.
Some explicit trim() tests added to string_util_test.cc
Format compatibility verified with
SHORT_TEST=1 tools/check_format_compatible.sh
Other existing tests in options_test, customizable_test, options_settable_test,
listener_test, options_file_test, options_util_test, db_options_test,
and compaction_service_test have extensive coverage of serialization /
deserialization logic.
Reviewed By: hx235, xingbowang
Differential Revision: D106855466
Pulled By: pdillinger
fbshipit-source-id: 872aac8d819c7b90c807b92bab85569b48fa2aa0
61 lines
1.9 KiB
C++
61 lines
1.9 KiB
C++
// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
|
|
// This source code is licensed under both the GPLv2 (found in the
|
|
// COPYING file in the root directory) and Apache 2.0 License
|
|
// (found in the LICENSE.Apache file in the root directory).
|
|
//
|
|
|
|
#include "string_util.h"
|
|
|
|
#include <gtest/gtest.h>
|
|
#include <port/stack_trace.h>
|
|
|
|
#include "test_util/testutil.h"
|
|
|
|
namespace ROCKSDB_NAMESPACE {
|
|
|
|
TEST(StringUtilTest, NumberToHumanString) {
|
|
ASSERT_EQ("-9223372036G", NumberToHumanString(INT64_MIN));
|
|
ASSERT_EQ("9223372036G", NumberToHumanString(INT64_MAX));
|
|
ASSERT_EQ("0", NumberToHumanString(0));
|
|
ASSERT_EQ("9999", NumberToHumanString(9999));
|
|
ASSERT_EQ("10K", NumberToHumanString(10000));
|
|
ASSERT_EQ("10M", NumberToHumanString(10000000));
|
|
ASSERT_EQ("10G", NumberToHumanString(10000000000));
|
|
ASSERT_EQ("-9999", NumberToHumanString(-9999));
|
|
ASSERT_EQ("-10K", NumberToHumanString(-10000));
|
|
ASSERT_EQ("-10M", NumberToHumanString(-10000000));
|
|
ASSERT_EQ("-10G", NumberToHumanString(-10000000000));
|
|
}
|
|
|
|
TEST(StringUtilTest, Trim) {
|
|
// Empty input.
|
|
EXPECT_EQ("", trim(""));
|
|
// No whitespace to strip.
|
|
EXPECT_EQ("a", trim("a"));
|
|
EXPECT_EQ("abc", trim("abc"));
|
|
// Leading whitespace only.
|
|
EXPECT_EQ("a", trim(" a"));
|
|
EXPECT_EQ("abc", trim(" abc"));
|
|
// Trailing whitespace only.
|
|
EXPECT_EQ("a", trim("a "));
|
|
EXPECT_EQ("abc", trim("abc "));
|
|
// Both ends.
|
|
EXPECT_EQ("a", trim(" a "));
|
|
EXPECT_EQ("abc", trim(" abc "));
|
|
EXPECT_EQ("a b c", trim(" a b c ")); // interior whitespace preserved
|
|
// All-whitespace inputs of various lengths must trim to empty.
|
|
EXPECT_EQ("", trim(" "));
|
|
EXPECT_EQ("", trim(" "));
|
|
EXPECT_EQ("", trim(" "));
|
|
EXPECT_EQ("", trim("\t"));
|
|
EXPECT_EQ("", trim("\n"));
|
|
EXPECT_EQ("", trim(" \t\n\r"));
|
|
}
|
|
|
|
} // namespace ROCKSDB_NAMESPACE
|
|
|
|
int main(int argc, char** argv) {
|
|
ROCKSDB_NAMESPACE::port::InstallStackTraceHandler();
|
|
::testing::InitGoogleTest(&argc, argv);
|
|
return RUN_ALL_TESTS();
|
|
} |