Merge 'ks_prop_defs: disallow empty replication factor string in NTS'…

… from Jan Ciołek

A CREATE KEYSPACE query which specifies an empty string ('') as the replication factor value is currently allowed:
```cql
CREATE KEYSPACE bad_ks WITH replication = {'class': 'NetworkTopologyStrategy', 'replication_factor': ''};
```

This is wrong, it's invalid to have an empty replication factor string.
It creates a keyspace without any replication, so the tables inside of it aren't writable.

Trying to create a `SimpleStrategy` keyspace with such replication factor throws an error, `NetworkTopolgyStrategy` should do the same.

The problem was in `prepare_options`, it treated an empty replication factor string as no replication factor.
Changing it to `std::optional` fixes the problem,
Now `std::nullopt` means no replication factor, and `make_optional("")` means that there is a replication factor, but it's described by an empty string.

Fixes: #13986

Closes #13988

* github.com:scylladb/scylladb:
  test/network_topology_strategy_test: Test NTS with replication_factor option in test_invalid_dcs
  ks_prop_defs: disallow empty replication factor string in NTS

cql3/statements/ks_prop_defs.cc

@@ -34,7 +34,7 @@ static std::map<sstring, sstring> prepare_options(
     // but the other strategy used the 'replication_factor' option, it will also be expanded.
     // See issue CASSANDRA-14303.
 
-    sstring rf;
+    std::optional<sstring> rf;
     auto it = options.find(ks_prop_defs::REPLICATION_FACTOR_KEY);
     if (it != options.end()) {
         // Expand: the user explicitly provided a 'replication_factor'.
@@ -49,10 +49,10 @@ static std::map<sstring, sstring> prepare_options(
         }
     }
 
-    if (!rf.empty()) {
+    if (rf.has_value()) {
         // The code below may end up not using "rf" at all (if all the DCs
         // already have rf settings), so let's validate it once (#8880).
-        locator::abstract_replication_strategy::validate_replication_factor(rf);
+        locator::abstract_replication_strategy::validate_replication_factor(*rf);
 
         // We keep previously specified DC factors for safety.
         for (const auto& opt : old_options) {
@@ -62,7 +62,7 @@ static std::map<sstring, sstring> prepare_options(
         }
 
         for (const auto& dc : tm.get_topology().get_datacenters()) {
-            options.emplace(dc, rf);
+            options.emplace(dc, *rf);
         }
     }
 

test/boost/network_topology_strategy_test.cc

@@ -756,6 +756,9 @@ SEASTAR_TEST_CASE(test_invalid_dcs) {
             BOOST_REQUIRE_THROW(e.execute_cql("CREATE KEYSPACE abc WITH REPLICATION "
                     "= {'class': 'NetworkTopologyStrategy', 'dc1':'" + incorrect + "'}").get(),
                     exceptions::configuration_exception);
+            BOOST_REQUIRE_THROW(e.execute_cql("CREATE KEYSPACE abc WITH REPLICATION "
+                    "= {'class': 'NetworkTopologyStrategy', 'replication_factor':'" + incorrect + "'}").get(),
+                    exceptions::configuration_exception);
             BOOST_REQUIRE_THROW(e.execute_cql("CREATE KEYSPACE abc WITH REPLICATION "
                     "= {'class': 'SimpleStrategy', 'replication_factor':'" + incorrect + "'}").get(),
                     exceptions::configuration_exception);