Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ json.debug-mode yes

When `debug-mode` is not enabled (the default), these subcommands return an error.

## Supported Module Commands
## Supported Module Commands

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't usually make commits to drop spaces... this one seems like a pretty good candidate for dropping...

```text
JSON.ARRAPPEND
JSON.ARRINDEX
Expand Down
2 changes: 1 addition & 1 deletion src/commands/json.mget.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"JSON.MGET": {
"summary": "Get serialized JSONs at the path from multiple document keys. Return null for non-existent key or JSON path.",
"summary": "Get serialized JSONs at the path from multiple document keys. Return null for nonexistent key or JSON path.",
"complexity": "O(N) where N is the number of keys",
"group": "json",
"module_since": "1.0.0",
Expand Down
10 changes: 5 additions & 5 deletions src/json/json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@

#define ERRMSG_JSON_DOCUMENT_NOT_FOUND "NONEXISTENT JSON document is not found"
#define ERRMSG_NEW_VALKEY_KEY_PATH_NOT_ROOT "SYNTAXERR A new Valkey key's path must be root"
#define ERRMSG_CANNOT_DISABLE_MODULE_DUE_TO_OUTSTADING_DATA \
#define ERRMSG_CANNOT_DISABLE_MODULE_DUE_TO_OUTSTANDING_DATA \
Comment thread
jsoref marked this conversation as resolved.
"Cannot disable the module because there are outstanding document keys"

#define STATIC /* decorator for static functions, remove so that backtrace symbols include these */
Expand Down Expand Up @@ -1047,7 +1047,7 @@ int Command_JsonDel(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {

if (rc != JSONUTIL_SUCCESS) {
if (rc == JSONUTIL_INVALID_JSON_PATH || rc == JSONUTIL_JSON_PATH_NOT_EXIST) {
// ignore invalid or non-existent path
// ignore invalid or nonexistent path
return ValkeyModule_ReplyWithLongLong(ctx, 0);
} else {
return ValkeyModule_ReplyWithError(ctx, jsonutil_code_to_message(rc));
Expand Down Expand Up @@ -2372,7 +2372,7 @@ int Command_JsonDebug(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc)

// ATTENTION:
// THIS IS AN UNDOCUMENTED SUBCOMMAND, DON'T RUN IT ON A PRODUCTION SYSTEM
// UNLESS YOU KNOW WHAT YOU'RE DOING -- IT CAN LOCK THE MAINTHREAD FOR SEVERAL SECONDS
// UNLESS YOU KNOW WHAT YOU'RE DOING -- IT CAN LOCK THE MAIN THREAD FOR SEVERAL SECONDS
//


Expand Down Expand Up @@ -2609,7 +2609,7 @@ int handleHashTableFactor(T rapidjson::HashTableFactors::*f, const void *v, T sc
//
// Resize the number of shards in the keyTable. this isn't multi-thread safe. But the current AppConfig architecture
// doesn't provide a good way to solve this problem. Also, we only do it when the table is empty. As long as there
// are no background operations in progress (slot migration, threadsave) we're good. Sadly there's no easy way for
Comment thread
roshkhatri marked this conversation as resolved.
// are no background operations in progress (slot migration, threadsave), we're good. Sadly there's no easy way for
// a module to detect that. Once we have RM_ApplyConfig, we'll restrict this to only happen at initialization time.
// and close this small timing hole.
//
Expand Down Expand Up @@ -3256,7 +3256,7 @@ extern "C" int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx) {
}

//
// Setup the global string table
// Set up the global string table
Comment thread
jsoref marked this conversation as resolved.
//
initKeyTable(KeyTable::MAX_SHARDS, KeyTable::Factors());
if (configKeyTable() == VALKEYMODULE_ERR) return VALKEYMODULE_ERR;
Expand Down
2 changes: 1 addition & 1 deletion src/json/keytable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ struct KeyTable_Shard {
};

/*
* Setup the KeyTable itself.
* Set up the KeyTable itself.
*/
KeyTable::KeyTable(const Config& cfg) :
malloc(cfg.malloc),
Expand Down
2 changes: 1 addition & 1 deletion src/json/keytable.h
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ struct KeyTable {
const Factors& getFactors() const { return factors; }
//
// Query if this set of factors is valid.
// returns: NULL, If the factors are valid. Otherwise an error string
// returns: NULL, If the factors are valid. Otherwise, an error string
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// This is used to validate a set of factors before setting them.
//
static const char *isValidFactors(const Factors& f);
Expand Down
12 changes: 6 additions & 6 deletions src/rapidjson/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ struct GenericStringRef {
//! Explicitly create string reference from \c const character pointer
#ifndef __clang__ // -Wdocumentation
/*!
This constructor can be used to \b explicitly create a reference to
This constructor can be used to \b explicitly create a reference to
a constant string pointer.

\see StringRef(const CharType*)
Expand Down Expand Up @@ -1040,9 +1040,9 @@ class GenericValue {
*/
GenericValue& operator=(GenericValue& rhs) RAPIDJSON_NOEXCEPT {
if (RAPIDJSON_LIKELY(this != &rhs)) {
// Can't destroy "this" before assigning "rhs", otherwise "rhs"
// could be used after free if it's an sub-Value of "this",
// hence the temporary danse.
// Can't destroy "this" before assigning "rhs"; otherwise, "rhs"
// could be used after free if it's a sub-Value of "this",
// hence the temporary dance.
GenericValue temp;
temp.RawAssign(rhs, false); // valid
this->~GenericValue();
Expand Down Expand Up @@ -1915,7 +1915,7 @@ class GenericValue {
uint64_t GetUint64() const { RAPIDJSON_ASSERT(data_.f.flags & kUint64Flag); return data_.n.u64; }

//! Get the value as double type.
/*! \note If the value is 64-bit integer type, it may lose precision. Use \c IsLosslessDouble() to check whether the converison is lossless.
/*! \note If the value is 64-bit integer type, it may lose precision. Use \c IsLosslessDouble() to check whether the conversion is lossless.
*/
double GetDouble() const {
RAPIDJSON_ASSERT(IsNumber());
Expand Down Expand Up @@ -2791,7 +2791,7 @@ class GenericValue {
for (size_t count = 0; count <= data_.o.capacity; ++count) {
MemberHT& thisEntry = m[ix];
if (!thisEntry.name) {
trace("Removemember, scan complete");
trace("RemoveMember, scan complete");
if (loadFactor() < hashTableFactors.minLoad) {
//
// See DoConstructHT
Expand Down
10 changes: 5 additions & 5 deletions src/rapidjson/reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ RAPIDJSON_DIAG_OFF(effc++)
\ingroup RAPIDJSON_ERRORS
\brief Macro to indicate a parse error.
\param parseErrorCode \ref rapidjson::ParseErrorCode of the error
\param offset position of the error in JSON input (\c size_t)
\param offset position of the error in JSON input (\c size_t)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there were a couple of places where things appeared to be intentionally indented. This doesn't appear to be one of those cases...


This macros can be used as a customization point for the internal
error handling mechanism of RapidJSON.
Expand Down Expand Up @@ -109,7 +109,7 @@ RAPIDJSON_DIAG_OFF(effc++)
\ingroup RAPIDJSON_ERRORS
\brief (Internal) macro to indicate and handle a parse error.
\param parseErrorCode \ref rapidjson::ParseErrorCode of the error
\param offset position of the error in JSON input (\c size_t)
\param offset position of the error in JSON input (\c size_t)

Invokes RAPIDJSON_PARSE_ERROR_NORETURN and stops the parsing.

Expand Down Expand Up @@ -1210,7 +1210,7 @@ class GenericReader {
is.dst_ = q;
}

// When read/write pointers are the same for insitu stream, just skip unescaped characters
// When read/write pointers are the same for in situ stream, just skip unescaped characters

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thi is the proper spelling for the phrase.

https://en.wikipedia.org/wiki/In_situ

Note that I'm not changingt the class as that seemed a bit too invasive...

static RAPIDJSON_FORCEINLINE void SkipUnescapedString(InsituStringStream& is) {
RAPIDJSON_ASSERT(is.src_ == is.dst_);
char* p = is.src_;
Expand Down Expand Up @@ -1382,7 +1382,7 @@ class GenericReader {
is.dst_ = q;
}

// When read/write pointers are the same for insitu stream, just skip unescaped characters
// When read/write pointers are the same for in situ stream, just skip unescaped characters
static RAPIDJSON_FORCEINLINE void SkipUnescapedString(InsituStringStream& is) {
RAPIDJSON_ASSERT(is.src_ == is.dst_);
char* p = is.src_;
Expand Down Expand Up @@ -1428,7 +1428,7 @@ class GenericReader {
is.src_ = is.dst_ = p;
}
#else
// When read/write pointers are the same for insitu stream, just skip unescaped characters
// When read/write pointers are the same for in situ stream, just skip unescaped characters
static RAPIDJSON_FORCEINLINE void SkipUnescapedString(InsituStringStream& is) {
RAPIDJSON_ASSERT(is.src_ == is.dst_);
char* p = is.src_;
Expand Down
2 changes: 1 addition & 1 deletion src/rapidjson/writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ class Writer {
struct Level {
Level(bool inArray_) : valueCount(0), inArray(inArray_) {}
size_t valueCount; //!< number of values in this level
bool inArray; //!< true if in array, otherwise in object
bool inArray; //!< true if in array; otherwise, in object
};

bool WriteNull() {
Expand Down
24 changes: 12 additions & 12 deletions tst/integration/test_json_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,8 +514,8 @@ def test_json_get_command_supports_all_datatypes(self):
for (path, value) in [('.firstName', '"John"'), # string
('.address.city', '"New York"'), # string
('.spouse', 'null'), # null
('.children', '[]'), # empy array
('.groups', '{}'), # empy object
('.children', '[]'), # empty array
('.groups', '{}'), # empty object
('.isAlive', 'true'), # boolean
('.age', '27')]: # float number
assert value.encode() == client.execute_command(
Expand All @@ -525,7 +525,7 @@ def test_json_get_command_supports_all_datatypes(self):
assert value == client.execute_command(
'JSON.GET', wikipedia, path).decode()

def test_json_path_syntax_objectkeys(self):
def test_json_path_syntax_object_keys(self):
client = self.server.get_new_client()
for (path, value) in [('["firstName"]', '"John"'),
('address[\'city\']', '"New York"'),
Expand Down Expand Up @@ -832,7 +832,7 @@ def test_json_get_legacy_and_v2path_wildcard(self):
assert exp == client.execute_command(
'JSON.GET', key, path)

# Legacy path returns non-existent error if no value is selected.
# Legacy path returns nonexistent error if no value is selected.
for (key, path, exp) in [
(k1, '.a[*]', None),
(k2, '.a.*', None)
Expand Down Expand Up @@ -1013,7 +1013,7 @@ def test_json_get_v2path_array_union(self):
]:
assert exp.encode() == client.execute_command('JSON.GET', k2, path)

# we do not support mixing of unions and slices, nor do we support extraneous commas
# we do not support mixing of unions and slices, nor do we support extraneous commas

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is why I included the commit to fix the doubled whitespace.

actually, this line helped me realize that one of my own rules was broken -- one of the benefits I get from running my rules against other projects is identifying bugs in them and fixing them...

for path in [
'$[0,1,2:4]',
'$[0:2,3,4]',
Expand Down Expand Up @@ -1541,7 +1541,7 @@ def test_json_double_consistency(self):
'''
Test that double values remain consistent when going through JSON Engine.
This tests a tolerance of 2^-50 for a decent number of iterations,
but is not enough to guarantee that level of presicion to our customers.
but is not enough to guarantee that level of precision to our customers.
Also verify that regular and pretty print double values have the same output.
'''
client = self.server.get_new_client()
Expand Down Expand Up @@ -1902,7 +1902,7 @@ def test_json_strappend_command_legacy_and_jsonpath_wildcard(self):
assert exp_new_str == client.execute_command(
'JSON.GET', key, path).decode()

def test_json_objectlen_command(self):
def test_json_object_len_command(self):

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the actual things are objlen or objkeys, so I don't see any particular reason to invent additional new words just for these tests...

client = self.server.get_new_client()
assert 4 == client.execute_command(
'JSON.OBJLEN', wikipedia, '.address')
Expand Down Expand Up @@ -1973,7 +1973,7 @@ def test_json_objlen_command_jsonpath_wildcard(self):
'JSON.OBJLEN', key, path)
assert self.error_class.is_wrongtype_error(str(e.value))

def test_json_objectkeys_command(self):
def test_json_object_keys_command(self):
client = self.server.get_new_client()
obj_keys = [b'street', b'city', b'state', b'zipcode']
assert obj_keys == client.execute_command(
Expand Down Expand Up @@ -2709,13 +2709,13 @@ def test_json_resp_command_jsonpath(self):
[b'number', b'646 555-4567']]

def test_json_debug_memory(self):
# non-existent key
# nonexistent key
client = self.server.get_new_client()

assert None == client.execute_command(
'JSON.DEBUG MEMORY', nonexistentkey)

# non-existent path
# nonexistent path
with pytest.raises(ResponseError) as e:
client.execute_command(
'JSON.DEBUG MEMORY', wikipedia, nonexistentpath)
Expand Down Expand Up @@ -2743,9 +2743,9 @@ def test_json_debug_memory(self):

# Verify the document size calculated by the "per document memory tracking" machinery matches the size
# calculated by the method of walking the JSON tree.
metadate_val = client.execute_command('JSON.DEBUG','MEMORY',wikipedia)
metadata_val = client.execute_command('JSON.DEBUG','MEMORY',wikipedia)
exp_val = client.execute_command('JSON.DEBUG','MEMORY',wikipedia,'.')
assert exp_val == metadate_val
assert exp_val == metadata_val

def test_keytable_corrupt_injects_handle(self):
"""JSON.DEBUG KEYTABLE-CORRUPT should inject a handle that KEYTABLE-CHECK detects as a mismatch."""
Expand Down
4 changes: 2 additions & 2 deletions tst/integration/test_rdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ def setup_data(self):
# Otherwise, data from previous test cases will interfere current test case.
client.execute_command("FLUSHDB")

# Load strore sample JSONs. We use strore.json as input to create a document key. Then, use
# strore_compact.json, which does not have indent/space/newline, to verify correctness of serialization.
# Load store sample JSONs. We use store.json as input to create a document key. Then, use
# store_compact.json, which does not have indent/space/newline, to verify correctness of serialization.
with open(DEFAULT_STORE_PATH, 'r') as file:
self.data_store = file.read()
assert b'OK' == client.execute_command(
Expand Down
2 changes: 1 addition & 1 deletion tst/unit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ target_link_libraries(unitTests
# To get this to work properly in a cross-compile environment, you need to set up
# CROSSCOMPILING_EMULATOR (see https://cmake.org/cmake/help/v3.12/prop_tgt/CROSSCOMPILING_EMULATOR.html)
# DISCOVERY_TIMEOUT - number of seconds given for Gtest to discover the tests to run, it should
# be big enough so the tests can start on MacOS and can be any number, 59 is just prime number
# be big enough so the tests can start on macOS and can be any number, 59 is just prime number
# close to 1 minute ;)
gtest_discover_tests(unitTests
TEST_PREFIX unit_
Expand Down
16 changes: 8 additions & 8 deletions tst/unit/dom_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ extern size_t hash_function(const char *, size_t);
void SetupAllocFuncs(size_t numShards) {
setupValkeyModulePointers();
//
// Now setup the KeyTable, the RapidJson library now depends on it
// Now set up the KeyTable, the RapidJson library now depends on it
//
KeyTable::Config c;
c.malloc = dom_alloc;
Expand Down Expand Up @@ -1865,7 +1865,7 @@ TEST_F(DomTest, testToggle_v2path) {
dom_free_doc(d1);
}

TEST_F(DomTest, testNumMutiBy_int64) {
TEST_F(DomTest, testNumMultiBy_int64) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd personally favor Multiply, but this was the simplest fix

jsn::vector<double> res;
bool isV2Path;
JParser parser;
Expand All @@ -1888,7 +1888,7 @@ TEST_F(DomTest, testNumMutiBy_int64) {
EXPECT_FALSE(isV2Path);
}

TEST_F(DomTest, testNumMutiBy_double) {
TEST_F(DomTest, testNumMultiBy_double) {
const char *new_val = "1";
JsonUtilCode rc = dom_set_value(nullptr, doc1, ".foo", new_val, false, false);
EXPECT_EQ(rc, JSONUTIL_SUCCESS);
Expand All @@ -1912,7 +1912,7 @@ TEST_F(DomTest, testNumMutiBy_double) {
EXPECT_FALSE(isV2Path);
}

TEST_F(DomTest, testNumMutiBy_int64_overflow) {
TEST_F(DomTest, testNumMultiBy_int64_overflow) {
const char *new_val = "9223372036854775800";
JsonUtilCode rc = dom_set_value(nullptr, doc1, ".foo", new_val, false, false);
EXPECT_EQ(rc, JSONUTIL_SUCCESS);
Expand Down Expand Up @@ -1941,7 +1941,7 @@ TEST_F(DomTest, testNumMutiBy_int64_overflow) {
EXPECT_FALSE(isV2Path);
}

TEST_F(DomTest, testNumMutiBy_int64_overflow_negative) {
TEST_F(DomTest, testNumMultiBy_int64_overflow_negative) {
const char *new_val = "-9223372036854775808";
JsonUtilCode rc = dom_set_value(nullptr, doc1, ".foo", new_val, false, false);
EXPECT_EQ(rc, JSONUTIL_SUCCESS);
Expand Down Expand Up @@ -1975,7 +1975,7 @@ TEST_F(DomTest, testNumMutiBy_int64_overflow_negative) {
EXPECT_FALSE(isV2Path);
}

TEST_F(DomTest, testNumMutiBy_double_overflow) {
TEST_F(DomTest, testNumMultiBy_double_overflow) {
const char *new_val = "1.7e308";
JsonUtilCode rc = dom_set_value(nullptr, doc1, ".foo", new_val, false, false);
EXPECT_EQ(rc, JSONUTIL_SUCCESS);
Expand All @@ -2001,7 +2001,7 @@ TEST_F(DomTest, testNumMutiBy_double_overflow) {
EXPECT_FALSE(isV2Path);
}

TEST_F(DomTest, testNumMutiBy_double_overflow_negative) {
TEST_F(DomTest, testNumMultiBy_double_overflow_negative) {
const char *new_val = "1.7e308";
JsonUtilCode rc = dom_set_value(nullptr, doc1, ".foo", new_val, false, false);
EXPECT_EQ(rc, JSONUTIL_SUCCESS);
Expand Down Expand Up @@ -2895,7 +2895,7 @@ TEST_F(DomTest, testSelector_get_array_legacyPath) {
EXPECT_TRUE(selector.getResultSet().empty());
}

TEST_F(DomTest, testSelector_get_array_negativeIndex_legacy_and_v2ath) {
TEST_F(DomTest, testSelector_get_array_negativeIndex_legacy_and_v2path) {
const char *path = ".phoneNumbers[-1]";
Selector selector;
JsonUtilCode rc = selector.getValues(*doc1, path);
Expand Down
4 changes: 2 additions & 2 deletions tst/unit/selector_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ TEST_F(SelectorTest, test_filterExpr_expression_part7) {
" \"poquo value\" : \"\\\"\","
" \"my key\" : \"key inside here\""
" },"
" \"anonther object\" : {"
" \"another object\" : {"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub CI passes...

" \"weight\" : 400,"
" \"a value\" : 400,"
" \"poquo value\" : \"'\","
Expand Down Expand Up @@ -612,7 +612,7 @@ TEST_F(SelectorTest, test_filterExpr_single_recursion_array) {
dom_free_doc(d1);
}

TEST_F(SelectorTest, test_filertExpr_array_index_single_recursion) {
TEST_F(SelectorTest, test_filterExpr_array_index_single_recursion) {
JDocument *d1;
JsonUtilCode rc = dom_parse(nullptr, node_accounts, strlen(node_accounts), &d1);
EXPECT_EQ(rc, JSONUTIL_SUCCESS);
Expand Down
2 changes: 1 addition & 1 deletion tst/unit/traps_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ extern size_t hash_function(const char *, size_t);
static void SetupAllocFuncs(size_t numShards) {
setupValkeyModulePointers();
//
// Now setup the KeyTable, the RapidJson library now depends on it
// Now set up the KeyTable, the RapidJson library now depends on it
//
KeyTable::Config c;
c.malloc = memory_alloc;
Expand Down
Loading