Repository: man-group/ArcticDB (language: python)

Issue:
You can create invalid library names with Mongo backend
### Describe the bug

ArcticDB lets me create an unusable library with a `/` on MongoDB backend.  `/` is not a valid char for a Mongo database name.

### Steps/Code to Reproduce

```python
import numpy as np
import arcticdb as adb

arctic = adb.Arctic('mongodb://localhost:27017')
a.create_library('a/b')
lib = a.get_library('a/b')
lib.write('c', np.array([1, 2, 3]))
```
Fails with
```
StorageException: E_UNEXPECTED_MONGO_ERROR Unexpected Mongo Error: MongoError#73: mongodb generic server error Invalid database name: 'arcticc_a/b': generic server error
```



### Expected Results

`create_library` should fail instead

### OS, Python Version and ArcticDB Version

```
Python: 3.11.9 | packaged by conda-forge | (main, Apr 19 2024, 18:36:13) [GCC 12.3.0]
OS: Linux-5.15.0-101-generic-x86_64-with-glibc2.35
ArcticDB: 4.4.2
```

### Backend storage used

Mongo

### Additional Context

_No response_


Discussion / hints:

Fix patch:
diff --git a/cpp/arcticdb/storage/mongo/mongo_storage.cpp b/cpp/arcticdb/storage/mongo/mongo_storage.cpp
index 7523f3c725..b4511b83e9 100644
--- a/cpp/arcticdb/storage/mongo/mongo_storage.cpp
+++ b/cpp/arcticdb/storage/mongo/mongo_storage.cpp
@@ -25,6 +25,8 @@
 
 namespace arcticdb::storage::mongo {
 
+const auto UNSUPPORTED_MONGO_CHARS = std::unordered_set<char>{'/'};
+
 std::string MongoStorage::collection_name(KeyType k) {
     return (fmt::format("{}{}", prefix_, k));
 }
@@ -218,6 +220,12 @@ bool MongoStorage::do_iterate_type_until_match(KeyType key_type, const IterateTy
     return false;
 }
 
+bool MongoStorage::do_is_path_valid(const std::string_view path) const {
+    return std::none_of(path.cbegin(), path.cend(), [](auto c) {
+        return UNSUPPORTED_MONGO_CHARS.contains(c);
+    });
+}
+
 bool MongoStorage::do_key_exists(const VariantKey& key) {
     auto collection = collection_name(variant_key_type(key));
     try {
diff --git a/cpp/arcticdb/storage/mongo/mongo_storage.hpp b/cpp/arcticdb/storage/mongo/mongo_storage.hpp
index 7f774a6aa9..7dbe22f999 100644
--- a/cpp/arcticdb/storage/mongo/mongo_storage.hpp
+++ b/cpp/arcticdb/storage/mongo/mongo_storage.hpp
@@ -49,6 +49,8 @@ class MongoStorage final : public Storage {
 
     std::string do_key_path(const VariantKey&) const final { return {}; };
 
+    bool do_is_path_valid(const std::string_view path) const final;
+
     std::string collection_name(KeyType k);
 
     std::shared_ptr<MongoClientWrapper> client_;


Test patch:
diff --git a/python/tests/integration/arcticdb/test_arctic_library_management.py b/python/tests/integration/arcticdb/test_arctic_library_management.py
index 225eff7b0b..b8c1d1e6ac 100644
--- a/python/tests/integration/arcticdb/test_arctic_library_management.py
+++ b/python/tests/integration/arcticdb/test_arctic_library_management.py
@@ -5,6 +5,7 @@
 
 As of the Change Date specified in that file, in accordance with the Business Source License, use of this software will be governed by the Apache License, version 2.0.
 """
+import sys
 
 import pytest
 import pandas as pd
@@ -273,12 +274,18 @@ def test_create_library_with_invalid_name(arctic_client):
     ac = arctic_client
 
     # These should succeed because the names are valid
-    valid_names = ["lib", "lib/with/slash", "lib-with-dash", "lib.with.dot", "lib123"]
+    valid_names = ["lib", "lib-with-dash", "lib.with.dot", "lib123"]
+    # These should fail because the names are invalid
+    invalid_names = [chr(0), "lib>", "lib<", "lib*", "/lib", "lib...lib", "lib" * 1000]
+    # This name should fail on mongo, and succeed on other storages
+    if "mongo" in ac.get_uri():
+        invalid_names += ["lib/with/slash"]
+    else:
+        valid_names += ["lib/with/slash"]
+
     for lib_name in valid_names:
         ac.create_library(lib_name)
 
-    # These should fail because the names are invalid
-    invalid_names = [chr(0), "lib>", "lib<", "lib*", "/lib", "lib...lib", "lib" * 1000]
     for lib_name in invalid_names:
         with pytest.raises(UserInputException):
             ac.create_library(lib_name)
@@ -287,15 +294,15 @@ def test_create_library_with_invalid_name(arctic_client):
     assert set(ac.list_libraries()) == set(valid_names)
 
 
-# TODO: Fix issue #1247, then use "arctic_client" instead of "arctic_client_no_lmdb"
 @pytest.mark.parametrize("prefix", ["", "prefix"])
 @pytest.mark.parametrize("suffix", ["", "suffix"])
-def test_create_library_with_all_chars(arctic_client_no_lmdb, prefix, suffix):
+def test_create_library_with_all_chars(arctic_client, prefix, suffix):
+    ac = arctic_client
+    if sys.platform == "win32" and "lmdb" in ac.get_uri():
+        pytest.skip(reason="Github actions runners run out of disk space on Windows in this test with lmdb")
     # Create library names with each character (except '\' because Azure replaces it with '/' in some cases)
     names = [f"{prefix}{chr(i)}{suffix}" for i in range(256) if chr(i) != "\\"]
 
-    ac = arctic_client_no_lmdb
-
     created_libraries = set()
     for name in names:
         try: