Change user PK to an UUID instead of Integer

Author: dstufft

tests/unit/accounts/test_services.py

@@ -10,6 +10,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import uuid
+
 import pretend
 
 from zope.interface.verify import verifyClass
@@ -60,7 +62,7 @@ def test_find_userid_existing_user(self, db_session):
 
     def test_check_password_nonexistant_user(self, db_session):
         service = services.DatabaseUserService(db_session)
-        assert not service.check_password(1, None)
+        assert not service.check_password(uuid.uuid4(), None)
 
     def test_check_password_invalid(self, db_session):
         user = UserFactory.create()

tests/unit/accounts/test_views.py

@@ -11,6 +11,7 @@
 # limitations under the License.
 
 import datetime
+import uuid
 
 import freezegun
 import pretend
@@ -118,8 +119,9 @@ def test_post_validate_redirects(self, monkeypatch, pyramid_request,
 
         new_session = {}
 
+        user_id = uuid.uuid4()
         user_service = pretend.stub(
-            find_userid=pretend.call_recorder(lambda username: 1),
+            find_userid=pretend.call_recorder(lambda username: user_id),
             update_user=pretend.call_recorder(lambda *a, **kw: None),
         )
         pyramid_request.find_service = pretend.call_recorder(
@@ -134,7 +136,7 @@ def test_post_validate_redirects(self, monkeypatch, pyramid_request,
         )
 
         pyramid_request.set_property(
-            lambda r: 1234 if with_user else None,
+            lambda r: str(uuid.uuid4()) if with_user else None,
             name="unauthenticated_userid",
         )
 
@@ -161,15 +163,15 @@ def test_post_validate_redirects(self, monkeypatch, pyramid_request,
 
         assert user_service.find_userid.calls == [pretend.call("theuser")]
         assert user_service.update_user.calls == [
-            pretend.call(1, last_login=now),
+            pretend.call(user_id, last_login=now),
         ]
 
         if with_user:
             assert new_session == {}
         else:
             assert new_session == {"a": "b", "foo": "bar"}
 
-        assert remember.calls == [pretend.call(pyramid_request, 1)]
+        assert remember.calls == [pretend.call(pyramid_request, str(user_id))]
         assert pyramid_request.session.invalidate.calls == [pretend.call()]
         assert pyramid_request.find_service.calls == [
             pretend.call(IUserService, context=None),

warehouse/accounts/models.py

@@ -16,6 +16,7 @@
     Boolean, DateTime, Integer, String,
 )
 from sqlalchemy import orm, select, sql
+from sqlalchemy.dialects.postgresql import UUID
 from sqlalchemy.orm.exc import NoResultFound
 from sqlalchemy.ext.hybrid import hybrid_property
 
@@ -38,7 +39,7 @@ def __getitem__(self, username):
             raise KeyError from None
 
 
-class User(SitemapMixin, db.ModelBase):
+class User(SitemapMixin, db.Model):
 
     __tablename__ = "accounts_user"
     __table_args__ = (
@@ -51,7 +52,6 @@ class User(SitemapMixin, db.ModelBase):
 
     __repr__ = make_repr("username")
 
-    id = Column(Integer, primary_key=True, nullable=False)
     username = Column(CIText, nullable=False, unique=True)
     name = Column(String(length=100), nullable=False)
     password = Column(String(length=128), nullable=False)
@@ -104,12 +104,8 @@ class Email(db.ModelBase):
 
     id = Column(Integer, primary_key=True, nullable=False)
     user_id = Column(
-        Integer,
-        ForeignKey(
-            "accounts_user.id",
-            deferrable=True,
-            initially="DEFERRED",
-        ),
+        UUID(as_uuid=True),
+        ForeignKey("accounts_user.id", deferrable=True, initially="DEFERRED"),
         nullable=False,
     )
     email = Column(String(length=254), nullable=False)

warehouse/legacy/tables.py

@@ -23,6 +23,7 @@
     UniqueConstraint,
     Boolean, Date, DateTime, Integer, LargeBinary, String, Text,
 )
+from sqlalchemy.dialects.postgresql import UUID
 
 from warehouse import db
 
@@ -34,12 +35,8 @@
     Column("id", Integer(), primary_key=True, nullable=False),
     Column(
         "user_id",
-        Integer(),
-        ForeignKey(
-            "accounts_user.id",
-            deferrable=True,
-            initially="DEFERRED",
-        ),
+        UUID(as_uuid=True),
+        ForeignKey("accounts_user.id", deferrable=True, initially="DEFERRED"),
         nullable=False,
     ),
     Column("key_id", CIText(), nullable=False),

warehouse/migrations/versions/8c8be2c0e69e_switch_to_a_uuid_based_primary_key_for_.py

@@ -0,0 +1,113 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""
+Switch to a UUID based primary key for User
+
+Revision ID: 8c8be2c0e69e
+Revises: 039f45e2dbf9
+Create Date: 2016-07-01 18:20:42.072664
+"""
+
+
+import sqlalchemy as sa
+
+from alembic import op
+from sqlalchemy.dialects import postgresql
+
+
+revision = "8c8be2c0e69e"
+down_revision = "039f45e2dbf9"
+
+
+def upgrade():
+    # Add a new column which is going to hold all of our new IDs for this table
+    # with a temporary name until we can rename it.
+    op.add_column(
+        "accounts_user",
+        sa.Column(
+            "new_id",
+            postgresql.UUID(as_uuid=True),
+            server_default=sa.text("gen_random_uuid()"),
+            nullable=False,
+        ),
+    )
+
+    # Add a column to tables that refer to accounts_user so they can be updated
+    # to refer to it.
+    op.add_column(
+        "accounts_email",
+        sa.Column("new_user_id", postgresql.UUID(as_uuid=True), nullable=True),
+    )
+    op.add_column(
+        "accounts_gpgkey",
+        sa.Column("new_user_id", postgresql.UUID(as_uuid=True), nullable=True),
+    )
+
+    # Update our referring tables so that their new column points to the
+    # correct user account.
+    op.execute(
+        """ UPDATE accounts_email
+            SET new_user_id = accounts_user.new_id
+            FROM accounts_user
+            WHERE accounts_email.user_id = accounts_user.id
+        """
+    )
+    op.execute(
+        """ UPDATE accounts_gpgkey
+            SET new_user_id = accounts_user.new_id
+            FROM accounts_user
+            WHERE accounts_gpgkey.user_id = accounts_user.id
+        """
+    )
+
+    # Disallow any NULL values in our referring tables
+    op.alter_column("accounts_email", "new_user_id", nullable=False)
+    op.alter_column("accounts_gpgkey", "new_user_id", nullable=False)
+
+    # Delete our existing fields and move our new fields into their old places.
+    op.drop_constraint("accounts_email_user_id_fkey", "accounts_email")
+    op.drop_column("accounts_email", "user_id")
+    op.alter_column("accounts_email", "new_user_id", new_column_name="user_id")
+
+    op.drop_constraint("accounts_gpgkey_user_id_fkey", "accounts_gpgkey")
+    op.drop_column("accounts_gpgkey", "user_id")
+    op.alter_column(
+        "accounts_gpgkey", "new_user_id", new_column_name="user_id")
+
+    # Switch the primary key from the old to the new field, drop the old name,
+    # and rename the new field into it's place.
+    op.drop_constraint("accounts_user_pkey", "accounts_user")
+    op.create_primary_key(None, "accounts_user", ["new_id"])
+    op.drop_column("accounts_user", "id")
+    op.alter_column("accounts_user", "new_id", new_column_name="id")
+
+    # Finally, Setup our foreign key constraints for our referring tables.
+    op.create_foreign_key(
+        None,
+        "accounts_email",
+        "accounts_user",
+        ["user_id"],
+        ["id"],
+        deferrable=True,
+    )
+    op.create_foreign_key(
+        None,
+        "accounts_gpgkey",
+        "accounts_user",
+        ["user_id"],
+        ["id"],
+        deferrable=True,
+    )
+
+
+def downgrade():
+    raise RuntimeError("Order No. 227 - Ни шагу назад!")