Fix such that legacy document ids without document type maps to default bucket space.

2 files changed, 23 insertions, 9 deletions

diff --git a/storage/src/tests/storageserver/configurable_bucket_resolver_test.cpp b/storage/src/tests/storageserver/configurable_bucket_resolver_test.cpp
index 4d80192d9e9..c10db0a1acd 100644
--- a/storage/src/tests/storageserver/configurable_bucket_resolver_test.cpp
+++ b/storage/src/tests/storageserver/configurable_bucket_resolver_test.cpp
@@ -20,6 +20,7 @@ struct ConfigurableBucketResolverTest : CppUnit::TestFixture {
     CPPUNIT_TEST(known_bucket_space_is_resolved_from_document_id);
     CPPUNIT_TEST(unknown_bucket_space_in_id_throws_exception);
     CPPUNIT_TEST(can_create_resolver_from_bucket_space_config);
+    CPPUNIT_TEST(legacy_document_id_without_document_type_maps_to_default_space);
     CPPUNIT_TEST_SUITE_END();
 
     using BucketSpaceMapping = ConfigurableBucketResolver::BucketSpaceMapping;
@@ -47,6 +48,7 @@ struct ConfigurableBucketResolverTest : CppUnit::TestFixture {
     void known_bucket_space_is_resolved_from_document_id();
     void unknown_bucket_space_in_id_throws_exception();
     void can_create_resolver_from_bucket_space_config();
+    void legacy_document_id_without_document_type_maps_to_default_space();
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(ConfigurableBucketResolverTest);
@@ -133,5 +135,11 @@ void ConfigurableBucketResolverTest::can_create_resolver_from_bucket_space_confi
                          resolver->bucketFromId(DocumentId("id::baz::xyz")).getBucketSpace());
 }
 
+void ConfigurableBucketResolverTest::legacy_document_id_without_document_type_maps_to_default_space() {
+    auto resolver = create_simple_resolver();
+    CPPUNIT_ASSERT_EQUAL(document::FixedBucketSpaces::default_space(),
+                         resolver.bucketFromId(DocumentId("userdoc:baz:1234:baz")).getBucketSpace());
+}
+
 }
 
diff --git a/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.cpp b/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.cpp
index bd4a60f851a..79e32551560 100644
--- a/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.cpp
+++ b/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.cpp
@@ -5,30 +5,36 @@
 #include <vespa/vespalib/util/exceptions.h>
 #include "configurable_bucket_resolver.h"
 
+using namespace document;
+
 namespace storage {
 
-document::Bucket ConfigurableBucketResolver::bucketFromId(const document::DocumentId& id) const {
+document::Bucket ConfigurableBucketResolver::bucketFromId(const DocumentId& id) const {
+    if (!id.hasDocType()) {
+        // Legacy document ids without document type maps to default bucket space
+        return Bucket(FixedBucketSpaces::default_space(), BucketId(0));
+    }
     auto iter = _type_to_space.find(id.getDocType());
     if (iter != _type_to_space.end()) {
-        return document::Bucket(iter->second, document::BucketId(0));
+        return Bucket(iter->second, BucketId(0));
     }
-    throw document::UnknownBucketSpaceException("Unknown bucket space mapping for document type '"
-                                           + id.getDocType() + "' in id: '" + id.toString() + "'", VESPA_STRLOC);
+    throw UnknownBucketSpaceException("Unknown bucket space mapping for document type '"
+                                      + id.getDocType() + "' in id: '" + id.toString() + "'", VESPA_STRLOC);
 }
 
-document::BucketSpace ConfigurableBucketResolver::bucketSpaceFromName(const vespalib::string& name) const {
-    return document::FixedBucketSpaces::from_string(name);
+BucketSpace ConfigurableBucketResolver::bucketSpaceFromName(const vespalib::string& name) const {
+    return FixedBucketSpaces::from_string(name);
 }
 
-vespalib::string ConfigurableBucketResolver::nameFromBucketSpace(const document::BucketSpace& space) const {
-    return document::FixedBucketSpaces::to_string(space);
+vespalib::string ConfigurableBucketResolver::nameFromBucketSpace(const BucketSpace& space) const {
+    return FixedBucketSpaces::to_string(space);
 }
 
 std::shared_ptr<ConfigurableBucketResolver> ConfigurableBucketResolver::from_config(
         const vespa::config::content::core::BucketspacesConfig& config) {
     ConfigurableBucketResolver::BucketSpaceMapping type_to_space;
     for (auto& mapping : config.documenttype) {
-        type_to_space.emplace(mapping.name, document::FixedBucketSpaces::from_string(mapping.bucketspace));
+        type_to_space.emplace(mapping.name, FixedBucketSpaces::from_string(mapping.bucketspace));
     }
     return std::make_shared<ConfigurableBucketResolver>(std::move(type_to_space));
 }