Ensure org name and slug uniqueness is case-insensitive (#1929)
Fixes #1927 Also adds tests to ensure index is working as expected, and migration to rename orgs that have names or slugs identical to other orgs except for case before the new case-insensitive index is built.
This commit is contained in:
Tessa Walsh
GitHub
parent
b1ccdc4d16
commit
6ccaad26d8
@ -17,7 +17,7 @@ from pymongo.errors import InvalidName
|
||||
from .migrations import BaseMigration
|
||||
|
||||
|
||||
CURR_DB_VERSION = "0031"
|
||||
CURR_DB_VERSION = "0032"
|
||||
|
||||
|
||||
# ============================================================================
|
||||
|
||||
@ -0,0 +1,94 @@
|
||||
"""
|
||||
Migration 0032 - Case-insensitive org name duplicates
|
||||
"""
|
||||
|
||||
from uuid import UUID
|
||||
|
||||
from pymongo.errors import DuplicateKeyError
|
||||
|
||||
from btrixcloud.migrations import BaseMigration
|
||||
from btrixcloud.utils import slug_from_name
|
||||
|
||||
|
||||
MIGRATION_VERSION = "0032"
|
||||
|
||||
|
||||
class Migration(BaseMigration):
|
||||
"""Migration class."""
|
||||
|
||||
# pylint: disable=unused-argument
|
||||
def __init__(self, mdb, **kwargs):
|
||||
super().__init__(mdb, migration_version=MIGRATION_VERSION)
|
||||
|
||||
async def migrate_up(self):
|
||||
"""Perform migration up.
|
||||
|
||||
Check for case-insensitive duplicate org names and slugs.
|
||||
If found, rename org/slug as necessary to avoid duplicates
|
||||
regardless of case.
|
||||
"""
|
||||
orgs_db = self.mdb["organizations"]
|
||||
|
||||
org_name_set = set()
|
||||
org_slug_set = set()
|
||||
|
||||
cursor = orgs_db.find({})
|
||||
async for org_dict in cursor:
|
||||
name = org_dict.get("name", "")
|
||||
slug = org_dict.get("slug", "")
|
||||
|
||||
rename_org = False
|
||||
|
||||
if name.lower() in org_name_set:
|
||||
rename_org = True
|
||||
else:
|
||||
org_name_set.add(name.lower())
|
||||
|
||||
if slug.lower() in org_slug_set:
|
||||
rename_org = True
|
||||
else:
|
||||
org_slug_set.add(slug.lower())
|
||||
|
||||
if rename_org:
|
||||
await self.update_org_name_and_slug(
|
||||
orgs_db, org_name_set, org_slug_set, name, org_dict.get("_id")
|
||||
)
|
||||
|
||||
# pylint: disable=too-many-arguments
|
||||
async def update_org_name_and_slug(
|
||||
self,
|
||||
orgs_db,
|
||||
org_name_set: set[str],
|
||||
org_slug_set: set[str],
|
||||
old_name: str,
|
||||
oid: UUID,
|
||||
):
|
||||
"""Rename org"""
|
||||
count = 2
|
||||
suffix = f" {count}"
|
||||
|
||||
while True:
|
||||
org_name = f"{old_name}{suffix}"
|
||||
org_slug = slug_from_name(org_name)
|
||||
|
||||
if org_name.lower() in org_name_set or org_slug.lower() in org_slug_set:
|
||||
count += 1
|
||||
suffix = f" {count}"
|
||||
continue
|
||||
|
||||
try:
|
||||
await orgs_db.find_one_and_update(
|
||||
{"_id": oid}, {"$set": {"slug": org_slug, "name": org_name}}
|
||||
)
|
||||
print(
|
||||
f"Renamed org {oid} to {org_name} with slug {org_slug}", flush=True
|
||||
)
|
||||
break
|
||||
except DuplicateKeyError:
|
||||
# pylint: disable=raise-missing-from
|
||||
count += 1
|
||||
suffix = f" {count}"
|
||||
# pylint: disable=broad-exception-caught
|
||||
except Exception as err:
|
||||
print(f"Error renaming org {oid}: {err}", flush=True)
|
||||
break
|
||||
@ -16,6 +16,7 @@ from tempfile import NamedTemporaryFile
|
||||
from typing import Optional, TYPE_CHECKING, Dict, Callable, List, AsyncGenerator, Any
|
||||
|
||||
from pymongo import ReturnDocument
|
||||
from pymongo.collation import Collation
|
||||
from pymongo.errors import AutoReconnect, DuplicateKeyError
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request
|
||||
@ -160,13 +161,18 @@ class OrgOps:
|
||||
|
||||
async def init_index(self) -> None:
|
||||
"""init lookup index"""
|
||||
case_insensitive_collation = Collation(locale="en", strength=1)
|
||||
while True:
|
||||
try:
|
||||
await self.orgs.create_index("name", unique=True)
|
||||
await self.orgs.create_index(
|
||||
"name", unique=True, collation=case_insensitive_collation
|
||||
)
|
||||
await self.orgs.create_index(
|
||||
"subscription.subId", unique=True, sparse=True
|
||||
)
|
||||
await self.orgs.create_index("slug", unique=True)
|
||||
await self.orgs.create_index(
|
||||
"slug", unique=True, collation=case_insensitive_collation
|
||||
)
|
||||
break
|
||||
# pylint: disable=duplicate-code
|
||||
except AutoReconnect:
|
||||
|
||||
@ -92,10 +92,19 @@ def test_rename_org_invalid_slug(admin_auth_headers, default_org_id):
|
||||
assert r.json()["detail"] == "invalid_slug"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"name",
|
||||
[
|
||||
# Identical name
|
||||
(NON_DEFAULT_ORG_NAME),
|
||||
# Identical name, different case
|
||||
("Non-Default Org"),
|
||||
],
|
||||
)
|
||||
def test_rename_org_duplicate_name(
|
||||
admin_auth_headers, default_org_id, non_default_org_id
|
||||
admin_auth_headers, default_org_id, non_default_org_id, name
|
||||
):
|
||||
rename_data = {"name": NON_DEFAULT_ORG_NAME, "slug": "this-slug-should-be-okay"}
|
||||
rename_data = {"name": name, "slug": "this-slug-should-be-okay"}
|
||||
r = requests.post(
|
||||
f"{API_PREFIX}/orgs/{default_org_id}/rename",
|
||||
headers=admin_auth_headers,
|
||||
@ -106,10 +115,19 @@ def test_rename_org_duplicate_name(
|
||||
assert r.json()["detail"] == "duplicate_org_name"
|
||||
|
||||
|
||||
def test_rename_org_duplicate_name(
|
||||
admin_auth_headers, default_org_id, non_default_org_id
|
||||
@pytest.mark.parametrize(
|
||||
"slug",
|
||||
[
|
||||
# Identical slug
|
||||
(NON_DEFAULT_ORG_SLUG),
|
||||
# Identical slug, different case
|
||||
("Non-Default-Org"),
|
||||
],
|
||||
)
|
||||
def test_rename_org_duplicate_slug(
|
||||
admin_auth_headers, default_org_id, non_default_org_id, slug
|
||||
):
|
||||
rename_data = {"name": "Should be okay", "slug": NON_DEFAULT_ORG_SLUG}
|
||||
rename_data = {"name": "Should be okay", "slug": slug}
|
||||
r = requests.post(
|
||||
f"{API_PREFIX}/orgs/{default_org_id}/rename",
|
||||
headers=admin_auth_headers,
|
||||
@ -146,11 +164,20 @@ def test_create_org(admin_auth_headers):
|
||||
assert data["created"]
|
||||
|
||||
|
||||
def test_create_org_duplicate_name(admin_auth_headers, non_default_org_id):
|
||||
@pytest.mark.parametrize(
|
||||
"name",
|
||||
[
|
||||
# Identical name
|
||||
(NON_DEFAULT_ORG_NAME),
|
||||
# Identical name, different case
|
||||
("Non-Default Org"),
|
||||
],
|
||||
)
|
||||
def test_create_org_duplicate_name(admin_auth_headers, non_default_org_id, name):
|
||||
r = requests.post(
|
||||
f"{API_PREFIX}/orgs/create",
|
||||
headers=admin_auth_headers,
|
||||
json={"name": NON_DEFAULT_ORG_NAME, "slug": "another-new-org"},
|
||||
json={"name": name, "slug": "another-new-org"},
|
||||
)
|
||||
|
||||
assert r.status_code == 400
|
||||
@ -158,11 +185,20 @@ def test_create_org_duplicate_name(admin_auth_headers, non_default_org_id):
|
||||
assert data["detail"] == "duplicate_org_name"
|
||||
|
||||
|
||||
def test_create_org_duplicate_slug(admin_auth_headers, non_default_org_id):
|
||||
@pytest.mark.parametrize(
|
||||
"slug",
|
||||
[
|
||||
# Identical slug
|
||||
(NON_DEFAULT_ORG_SLUG),
|
||||
# Identical slug, different case
|
||||
("Non-Default-Org"),
|
||||
],
|
||||
)
|
||||
def test_create_org_duplicate_slug(admin_auth_headers, non_default_org_id, slug):
|
||||
r = requests.post(
|
||||
f"{API_PREFIX}/orgs/create",
|
||||
headers=admin_auth_headers,
|
||||
json={"name": "Yet another new org", "slug": NON_DEFAULT_ORG_SLUG},
|
||||
json={"name": "Yet another new org", "slug": slug},
|
||||
)
|
||||
|
||||
assert r.status_code == 400
|
||||
|
||||
Loading…
Reference in New Issue
Block a user