From 9ee912d5cfe70c05a9bfb62c87cb2a67461ed30e Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Tue, 21 May 2019 21:57:49 +0300 Subject: [PATCH] alternator: correct handling of missing item in GetItem According to the documentation, trying to GetItem a non-existant item should result in an empty response - NOT a response with an empty "Item" map as we do before this patch. This patch fixes this case, and adds a test case for it. As usual, we verify that the test case also works on Amazon DynamoDB, to verify DynamoDB really behaves the way we thik it does. Signed-off-by: Nadav Har'El --- alternator-test/test_item.py | 8 ++++++++ alternator/executor.cc | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/alternator-test/test_item.py b/alternator-test/test_item.py index bf9a783042..5f1c12b790 100644 --- a/alternator-test/test_item.py +++ b/alternator-test/test_item.py @@ -166,3 +166,11 @@ def test_item_operations_nonexistent_table(dynamodb): with pytest.raises(ClientError, match='ResourceNotFoundException'): dynamodb.meta.client.put_item(TableName='non_existent_table', Item={'a':{'S':'b'}}) + +# Fetching a non-existant item. According to the DynamoDB doc, "If there is no +# matching item, GetItem does not return any data and there will be no Item +# element in the response." +def test_get_item_missing_item(test_table): + p = random_string() + c = random_string() + assert not "Item" in test_table.get_item(Key={'p': p, 'c': c}, ConsistentRead=True) diff --git a/alternator/executor.cc b/alternator/executor.cc index a476966ffe..98dcd44d3c 100644 --- a/alternator/executor.cc +++ b/alternator/executor.cc @@ -478,6 +478,13 @@ static Json::Value describe_item(schema_ptr schema, const query::partition_slice query::result_view::consume(*query_result, slice, cql3::selection::result_set_builder::visitor(builder, *schema, selection)); auto result_set = builder.build(); + if (result_set->empty()) { + // If there is no matching item, we're supposed to return an empty + // object without an Item member - not one with an empty Item member + return Json::objectValue; + } + // FIXME: I think this can't really be a loop, there should be exactly + // one result after above we handled the 0 result case for (auto& result_row : result_set->rows()) { const auto& columns = selection.get_columns(); auto column_it = columns.begin();