Alternator: allow CreateTable with streams explicitly turned off

While Alternator doesn't yet support creating a table with streams
(i.e., CDC) turned on, we should only failed the creation if streams
were really turned on. If the StreamSpecification option exists, but
does *not* ask to turn on streams, we should not fail the creation -
and this patch fixes this.

This patch also adds two tests - one where StreamSpecification is
passed but does not ask to turn on streams (so table creation should
succeed), and another test which explicitly requests to turn on
streams. The second test still xfails on Alternator, and should continue
to do so until we implement streams (we do *not* want to silently
ignore a request to turn on streams).

Fixes #5796

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20200212100546.16337-1-nyh@scylladb.com>

alternator-test/test_table.py

@@ -227,6 +227,34 @@ def test_create_table_billing_mode_errors(dynamodb, test_table):
             KeySchema=[{ 'AttributeName': 'p', 'KeyType': 'HASH' }],
             AttributeDefinitions=[{ 'AttributeName': 'p', 'AttributeType': 'S' }])
 
+# Even before Alternator gains full support for the DynamoDB stream API
+# and CreateTable's StreamSpecification option, we should support the
+# options which mean it is turned *off*.
+def test_table_streams_off(dynamodb):
+    # If StreamSpecification is given, but has StreamEnabled=false, it's as
+    # if StreamSpecification was missing. StreamViewType isn't needed.
+    table = create_test_table(dynamodb, StreamSpecification={'StreamEnabled': False},
+        KeySchema=[{ 'AttributeName': 'p', 'KeyType': 'HASH' }],
+        AttributeDefinitions=[{ 'AttributeName': 'p', 'AttributeType': 'S' }]);
+    table.delete();
+    # DynamoDB doesn't allow StreamSpecification to be empty map - if it
+    # exists, it must have a StreamEnabled
+    with pytest.raises(ClientError, match='ValidationException'):
+        table = create_test_table(dynamodb, StreamSpecification={},
+            KeySchema=[{ 'AttributeName': 'p', 'KeyType': 'HASH' }],
+            AttributeDefinitions=[{ 'AttributeName': 'p', 'AttributeType': 'S' }]);
+        table.delete();
+    # Unfortunately, boto3 doesn't allow us to pass StreamSpecification=None.
+    # This is what we had in issue #5796.
+
+@pytest.mark.xfail(reason="streams not yet implemented")
+def test_table_streams_on(dynamodb):
+    table = create_test_table(dynamodb,
+        StreamSpecification={'StreamEnabled': True, 'StreamViewType': 'OLD_IMAGE'},
+        KeySchema=[{ 'AttributeName': 'p', 'KeyType': 'HASH' }],
+        AttributeDefinitions=[{ 'AttributeName': 'p', 'AttributeType': 'S' }]);
+    table.delete();
+
 # Our first implementation had a special column name called "attrs" where
 # we stored a map for all non-key columns. If the user tried to name one
 # of the key columns with this same name, the result was a disaster - Scylla

alternator/executor.cc

@@ -805,8 +805,18 @@ future<executor::request_return_type> executor::create_table(client_state& clien
     if (rjson::find(table_info, "SSESpecification")) {
         return make_ready_future<request_return_type>(api_error("ValidationException", "SSESpecification: configuring encryption-at-rest is not yet supported."));
     }
-    if (rjson::find(table_info, "StreamSpecification")) {
-        return make_ready_future<request_return_type>(api_error("ValidationException", "StreamSpecification: streams (CDC) is not yet supported."));
+    // We don't yet support streams (CDC), but a StreamSpecification asking
+    // *not* to use streams should be accepted:
+    rjson::value* stream_specification = rjson::find(table_info, "StreamSpecification");
+    if (stream_specification && stream_specification->IsObject()) {
+        rjson::value* stream_enabled = rjson::find(*stream_specification, "StreamEnabled");
+        if (!stream_enabled || !stream_enabled->IsBool()) {
+            return make_ready_future<request_return_type>(api_error("ValidationException", "StreamSpecification needs boolean StreamEnabled"));
+        }
+        if (stream_enabled->GetBool()) {
+            // TODO: support streams
+            return make_ready_future<request_return_type>(api_error("ValidationException", "StreamSpecification: streams (CDC) is not yet supported."));
+        }
     }
 
     builder.set_extensions(schema::extensions_map{{sstring(tags_extension::NAME), ::make_shared<tags_extension>()}});