diff --git a/Makefile b/Makefile index db9f90029977..971f9da4f6c3 100644 --- a/Makefile +++ b/Makefile @@ -152,7 +152,7 @@ initdb: docker-compose run --rm web psql -h db -d postgres -U postgres -c "CREATE DATABASE warehouse ENCODING 'UTF8'" xz -d -f -k dev/$(DB).sql.xz --stdout | docker-compose run --rm web psql -h db -d warehouse -U postgres -v ON_ERROR_STOP=1 -1 -f - docker-compose run --rm web python -m warehouse db upgrade head - $(MAKE) reindex + # $(MAKE) reindex reindex: docker-compose run --rm web python -m warehouse search reindex diff --git a/tests/unit/utils/test_project.py b/tests/unit/utils/test_project.py index 321cfa87c2e5..f5a5d2342047 100644 --- a/tests/unit/utils/test_project.py +++ b/tests/unit/utils/test_project.py @@ -27,8 +27,8 @@ from warehouse.utils.project import ( confirm_project, destroy_docs, + destroy_project, remove_documentation, - remove_project, ) from ...common.db.accounts import UserFactory @@ -92,8 +92,7 @@ def test_confirm_incorrect_input(): ] -@pytest.mark.parametrize("flash", [True, False]) -def test_remove_project(db_request, flash): +def test_destroy_project(db_request): user = UserFactory.create() project = ProjectFactory.create(name="foo") release = ReleaseFactory.create(project=project) @@ -105,15 +104,11 @@ def test_remove_project(db_request, flash): db_request.remote_addr = "192.168.1.1" db_request.session = stub(flash=call_recorder(lambda *a, **kw: stub())) - remove_project(project, db_request, flash=flash) - - if flash: - assert db_request.session.flash.calls == [ - call("Deleted the project 'foo'", queue="success") - ] - else: - assert db_request.session.flash.calls == [] + destroy_project(project, db_request) + assert db_request.session.flash.calls == [ + call("Deleted the project 'foo'", queue="success") + ] assert not (db_request.db.query(Role).filter(Role.project == project).count()) assert not ( db_request.db.query(File) @@ -144,8 +139,7 @@ def test_remove_project(db_request, flash): assert journal_entry.submitted_from == db_request.remote_addr -@pytest.mark.parametrize("flash", [True, False]) -def test_destroy_docs(db_request, flash): +def test_destroy_docs(db_request): user = UserFactory.create() project = ProjectFactory.create(name="foo", has_docs=True) RoleFactory.create(user=user, project=project) @@ -156,7 +150,7 @@ def test_destroy_docs(db_request, flash): remove_documentation_recorder = stub(delay=call_recorder(lambda *a, **kw: None)) db_request.task = call_recorder(lambda *a, **kw: remove_documentation_recorder) - destroy_docs(project, db_request, flash=flash) + destroy_docs(project, db_request) journal_entry = ( db_request.db.query(JournalEntry) @@ -176,13 +170,11 @@ def test_destroy_docs(db_request, flash): ) assert remove_documentation_recorder.delay.calls == [call("foo")] + assert db_request.session.flash.calls == [ + call("Deleted docs for project 'foo'", queue="success") + ] + - if flash: - assert db_request.session.flash.calls == [ - call("Deleted docs for project 'foo'", queue="success") - ] - else: - assert db_request.session.flash.calls == [] def test_remove_documentation(db_request): diff --git a/warehouse/admin/routes.py b/warehouse/admin/routes.py index ffda496c8467..a7832a54b4c4 100644 --- a/warehouse/admin/routes.py +++ b/warehouse/admin/routes.py @@ -102,6 +102,13 @@ def includeme(config): traverse="/{project_name}", domain=warehouse, ) + config.add_route( + "admin.project.restore", + "/admin/projects/{project_name}/restore/", + factory="warehouse.packaging.models:ProjectFactory", + traverse="/{project_name}", + domain=warehouse, + ) # Journal related Admin pages config.add_route("admin.journals.list", "/admin/journals/", domain=warehouse) diff --git a/warehouse/admin/templates/admin/projects/delete.html b/warehouse/admin/templates/admin/projects/delete.html index f650f2fc32d9..59cc3af5f49f 100644 --- a/warehouse/admin/templates/admin/projects/delete.html +++ b/warehouse/admin/templates/admin/projects/delete.html @@ -19,7 +19,7 @@
-

Delete Project

+

Destroy Project

@@ -27,13 +27,13 @@

Delete Project

- Deleting will irreversibly delete this project along with + This will irreversibly remove this project from the database along with {{ project.releases|length() }} releases.

diff --git a/warehouse/admin/templates/admin/projects/detail.html b/warehouse/admin/templates/admin/projects/detail.html index ec94536dbe7b..d9014893f50f 100644 --- a/warehouse/admin/templates/admin/projects/detail.html +++ b/warehouse/admin/templates/admin/projects/detail.html @@ -266,9 +266,12 @@

Project activity

- + {% if project.soft_deleted %} + + {% include 'restore.html' %} + {% endif %} + {% include 'delete.html' %} -
diff --git a/warehouse/admin/templates/admin/projects/restore.html b/warehouse/admin/templates/admin/projects/restore.html new file mode 100644 index 000000000000..17bd51801ca0 --- /dev/null +++ b/warehouse/admin/templates/admin/projects/restore.html @@ -0,0 +1,49 @@ +{# + # 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. + -#} + + {% set project_name = project.normalized_name %} + +
+ + +
+
+

Restore Project

+
+ +
+
+ +
+

+ This will restore the soft deleted project along with + {{ project.releases|length() }} releases. +

+ +
+ + +
+ +
+
+
+ +
+
+
+
+ \ No newline at end of file diff --git a/warehouse/admin/views/prohibited_project_names.py b/warehouse/admin/views/prohibited_project_names.py index c2848c41ce5e..8ecc5bf3c5a3 100644 --- a/warehouse/admin/views/prohibited_project_names.py +++ b/warehouse/admin/views/prohibited_project_names.py @@ -29,7 +29,7 @@ ) from warehouse.utils.http import is_safe_url from warehouse.utils.paginate import paginate_url_factory -from warehouse.utils.project import remove_project +from warehouse.utils.project import destroy_project @view_config( @@ -192,7 +192,7 @@ def add_prohibited_project_names(request): .first() ) if project is not None: - remove_project(project, request) + destroy_project(project, request) request.session.flash(f"Prohibited Project Name {project_name!r}", queue="success") diff --git a/warehouse/admin/views/projects.py b/warehouse/admin/views/projects.py index bc2d716a10fb..078d269fcb69 100644 --- a/warehouse/admin/views/projects.py +++ b/warehouse/admin/views/projects.py @@ -23,7 +23,7 @@ from warehouse.forklift.legacy import MAX_FILESIZE, MAX_PROJECT_SIZE from warehouse.packaging.models import JournalEntry, Project, Release, Role from warehouse.utils.paginate import paginate_url_factory -from warehouse.utils.project import confirm_project, remove_project +from warehouse.utils.project import confirm_project, destroy_project, soft_restore_project ONE_MB = 1024 * 1024 # bytes ONE_GB = 1024 * 1024 * 1024 # bytes @@ -469,6 +469,22 @@ def delete_role(project, request): ) + +@view_config( + route_name="admin.project.restore", + permission="admin", + request_method="POST", + uses_session=True, + require_methods=False, +) +def restore_project(project, request): + confirm_project(project, request, fail_route="admin.project.detail") + soft_restore_project(project, request) + + return HTTPSeeOther(request.route_path("admin.project.list")) + + + @view_config( route_name="admin.project.delete", permission="admin", @@ -478,6 +494,6 @@ def delete_role(project, request): ) def delete_project(project, request): confirm_project(project, request, fail_route="admin.project.detail") - remove_project(project, request) + destroy_project(project, request) return HTTPSeeOther(request.route_path("admin.project.list")) diff --git a/warehouse/email/__init__.py b/warehouse/email/__init__.py index ff451fb740d2..52fe9e3448a6 100644 --- a/warehouse/email/__init__.py +++ b/warehouse/email/__init__.py @@ -228,6 +228,22 @@ def send_removed_project_email( "recipient_role_descr": recipient_role_descr, } +@_email("restored-project") +def send_restored_project_email( + request, user, *, project_name, submitter_name, submitter_role, recipient_role +): + recipient_role_descr = "an owner" + if recipient_role == "Maintainer": + recipient_role_descr = "a maintainer" + #TODO 4440 : How do we customize the email? Who do we send the email to? + return { + "project_name": project_name, + "submitter_name": submitter_name, + "submitter_role": submitter_role.lower(), + "recipient_role_descr": recipient_role_descr, + } + + @_email("yanked-project-release") def send_yanked_project_release_email( diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 19a8e0504996..c2ca865ffcd2 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -69,7 +69,7 @@ ) from warehouse.utils.http import is_safe_url from warehouse.utils.paginate import paginate_url_factory -from warehouse.utils.project import confirm_project, destroy_docs, remove_project +from warehouse.utils.project import confirm_project, destroy_docs def user_projects(request): @@ -98,11 +98,29 @@ def user_projects(request): .all() ), "projects_sole_owned": ( - request.db.query(Project).join(with_sole_owner).order_by(Project.name).all() + request.db.query(Project) + .join(with_sole_owner) + .order_by(Project.name) + .all() ), } +def soft_delete_file(file_): + file_.soft_deleted = True + + +def soft_delete_release(release): + for file_ in release.files: + soft_delete_file(file_) + release.soft_deleted = True + + +def soft_delete_project(project): + for release in project.releases: + soft_delete_release(release) + project.soft_deleted = True + @view_defaults( route_name="manage.account", renderer="manage/account.html", @@ -964,7 +982,18 @@ def delete_project(project, request): recipient_role=contributor_role, ) - remove_project(project, request) + request.db.add( + JournalEntry( + name=project.name, + action="delete project", + submitted_by=request.user, + submitted_from=request.remote_addr, + ) + ) + + soft_delete_project(project) + + request.session.flash(f"Deleted the project {project.name!r}", queue="success") return HTTPSeeOther(request.route_path("manage.projects")) @@ -1006,6 +1035,7 @@ def manage_project_releases(project, request): .group_by(Release.id) .group_by(File.packagetype) .filter(Release.project == project) + .execution_options(include_deleted=True) .all() ) @@ -1276,7 +1306,7 @@ def delete_project_release(self): }, ) - self.request.db.delete(self.release) + soft_delete_release(self.release) self.request.session.flash( f"Deleted release {self.release.version!r}", queue="success" @@ -1385,7 +1415,7 @@ def _error(message): recipient_role=contributor_role, ) - self.request.db.delete(release_file) + soft_delete_release(self.release) self.request.session.flash( f"Deleted file {release_file.filename!r}", queue="success" diff --git a/warehouse/migrations/versions/9b6babb05318_text_soft_deletes.py b/warehouse/migrations/versions/9b6babb05318_text_soft_deletes.py new file mode 100644 index 000000000000..556f818bd164 --- /dev/null +++ b/warehouse/migrations/versions/9b6babb05318_text_soft_deletes.py @@ -0,0 +1,49 @@ +# 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. +""" +text + +Revision ID: 9b6babb05318 +Revises: bc8f7b526961 +Create Date: 2020-06-25 22:37:25.343522 +""" + +from alembic import op +import sqlalchemy as sa + + +revision = '9b6babb05318' +down_revision = 'bc8f7b526961' + +# Note: It is VERY important to ensure that a migration does not lock for a +# long period of time and to ensure that each individual migration does +# not break compatibility with the *previous* version of the code base. +# This is because the migrations will be ran automatically as part of the +# deployment process, but while the previous version of the code is still +# up and running. Thus backwards incompatible changes must be broken up +# over multiple migrations inside of multiple pull requests in order to +# phase them in over multiple deploys. + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('projects', sa.Column('soft_deleted', sa.Boolean(), server_default=sa.text('false'), nullable=False)) + op.add_column('release_files', sa.Column('soft_deleted', sa.Boolean(), server_default=sa.text('false'), nullable=False)) + op.add_column('releases', sa.Column('soft_deleted', sa.Boolean(), server_default=sa.text('false'), nullable=False)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('releases', 'soft_deleted') + op.drop_column('release_files', 'soft_deleted') + op.drop_column('projects', 'soft_deleted') + # ### end Alembic commands ### diff --git a/warehouse/migrations/versions/9e9c5c978e59_text.py b/warehouse/migrations/versions/9e9c5c978e59_text.py new file mode 100644 index 000000000000..65c78ad635f5 --- /dev/null +++ b/warehouse/migrations/versions/9e9c5c978e59_text.py @@ -0,0 +1,45 @@ +# 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. +""" +text + +Revision ID: 9e9c5c978e59 +Revises: 9b6babb05318 +Create Date: 2020-06-27 01:26:35.772184 +""" + +from alembic import op +import sqlalchemy as sa + + +revision = '9e9c5c978e59' +down_revision = '9b6babb05318' + +# Note: It is VERY important to ensure that a migration does not lock for a +# long period of time and to ensure that each individual migration does +# not break compatibility with the *previous* version of the code base. +# This is because the migrations will be ran automatically as part of the +# deployment process, but while the previous version of the code is still +# up and running. Thus backwards incompatible changes must be broken up +# over multiple migrations inside of multiple pull requests in order to +# phase them in over multiple deploys. + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + pass + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + pass + # ### end Alembic commands ### diff --git a/warehouse/migrations/versions/a9b326c18190_deleted_projects_table_update_1_manual.py b/warehouse/migrations/versions/a9b326c18190_deleted_projects_table_update_1_manual.py new file mode 100644 index 000000000000..96e3856473da --- /dev/null +++ b/warehouse/migrations/versions/a9b326c18190_deleted_projects_table_update_1_manual.py @@ -0,0 +1,45 @@ +# 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. +""" +deleted_projects_table_update_1_manual + +Revision ID: a9b326c18190 +Revises: 9e9c5c978e59 +Create Date: 2020-07-09 19:20:20.672355 +""" + +from alembic import op +import sqlalchemy as sa + + +revision = 'a9b326c18190' +down_revision = '9e9c5c978e59' + +# Note: It is VERY important to ensure that a migration does not lock for a +# long period of time and to ensure that each individual migration does +# not break compatibility with the *previous* version of the code base. +# This is because the migrations will be ran automatically as part of the +# deployment process, but while the previous version of the code is still +# up and running. Thus backwards incompatible changes must be broken up +# over multiple migrations inside of multiple pull requests in order to +# phase them in over multiple deploys. + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + pass + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + pass + # ### end Alembic commands ### diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index b96ba2b414eb..be36975c9a8e 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -34,8 +34,10 @@ String, Table, Text, + event, UniqueConstraint, func, + inspect, orm, sql, ) @@ -45,6 +47,7 @@ from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import validates from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound +from sqlalchemy.orm.query import Query from warehouse import db from warehouse.accounts.models import User @@ -54,6 +57,31 @@ from warehouse.utils.attrs import make_repr +@event.listens_for(Query, "before_compile", retval=True) +def before_compile(query): + """A query compilation rule that will add limiting criteria for every + subclass of SoftDeletable""" + if query._execution_options.get("include_deleted", False): + return query + + for ent in query.column_descriptions: + entity = ent["entity"] + if entity is None: + continue + insp = inspect(entity) + mapper = getattr(insp, "mapper", None) + if mapper and issubclass(mapper.class_, SoftDeleteable): + query = query.enable_assertions(False).filter(ent['entity'].soft_deleted == False) + # query = query + return query + + +class SoftDeleteable(object): + """Mixin that identifies a class as being soft-deleteable""" + + soft_deleted = Column(Boolean, nullable=False, server_default=sql.false()) + + class Role(db.Model): __tablename__ = "roles" @@ -92,7 +120,7 @@ def __getitem__(self, project): raise KeyError from None -class Project(SitemapMixin, db.Model): +class Project(SitemapMixin, SoftDeleteable, db.Model): __tablename__ = "projects" __table_args__ = ( @@ -296,7 +324,7 @@ class Description(db.Model): rendered_by = Column(Text, nullable=False) -class Release(db.Model): +class Release(SoftDeleteable, db.Model): __tablename__ = "releases" @@ -455,7 +483,7 @@ def has_meta(self): ) -class File(db.Model): +class File(SoftDeleteable, db.Model): __tablename__ = "release_files" diff --git a/warehouse/static/sass/warehouse.scss b/warehouse/static/sass/warehouse.scss index 07a449fa0016..eb4241948b74 100644 --- a/warehouse/static/sass/warehouse.scss +++ b/warehouse/static/sass/warehouse.scss @@ -160,6 +160,10 @@ } } +.deleted { + border: 3px solid #f00 !important; +} + // COLORS // Make text or icons red to draw attention .danger { diff --git a/warehouse/templates/manage/projects.html b/warehouse/templates/manage/projects.html index c80c713fa8a8..6c7ee65e8495 100644 --- a/warehouse/templates/manage/projects.html +++ b/warehouse/templates/manage/projects.html @@ -23,7 +23,7 @@

{% trans project_count=projects|length %}Your projects ({ {% if projects %} {% for project in projects %} {% set release = project.releases[0] if project.releases else None %} -
+

{{ project.name }} @@ -45,26 +45,28 @@

{{ project.name }} {% endif %}

- {% trans %}Manage{% endtrans %} - {% trans %}View{% endtrans %} + Manage + View + {% endif %}
diff --git a/warehouse/utils/project.py b/warehouse/utils/project.py index c4780d92d716..f4dd19abe11a 100644 --- a/warehouse/utils/project.py +++ b/warehouse/utils/project.py @@ -43,9 +43,11 @@ def confirm_project(project, request, fail_route): raise HTTPSeeOther(request.route_path(fail_route, project_name=project_name)) -def remove_project(project, request, flash=True): - # TODO: We don't actually delete files from the data store. We should add - # some kind of garbage collection at some point. +def destroy_project(project, request): + """ + This permanently removes the project and everything belonging to it. + Only for use by administrative routes. + """ request.db.add( JournalEntry( @@ -61,11 +63,44 @@ def remove_project(project, request, flash=True): # Flush so we can repeat this multiple times if necessary request.db.flush() - if flash: - request.session.flash(f"Deleted the project {project.name!r}", queue="success") + request.session.flash(f"Destroyed the project {project.name!r}", queue="success") -def destroy_docs(project, request, flash=True): +def _soft_restore_file(file_): + file_.soft_deleted = False + + +def _soft_restore_release(release): + release.soft_deleted = False + for file_ in release.files: + _soft_restore_file(file_) + + +def _soft_restore_project(project): + project.soft_deleted = False + for release in project.releases: + _soft_restore_release(release) + +def soft_restore_project(project, request): + + request.db.add( + JournalEntry( + name=project.name, + action="restore project", + submitted_by=request.user, + submitted_from=request.remote_addr, + ) + ) + + _soft_restore_project(project) + + request.db.flush() + + request.session.flash(f"Restored the project {project.name!r}", queue="success") + + + +def destroy_docs(project, request): request.db.add( JournalEntry( name=project.name, @@ -79,7 +114,5 @@ def destroy_docs(project, request, flash=True): project.has_docs = False - if flash: - request.session.flash( - f"Deleted docs for project {project.name!r}", queue="success" - ) + request.session.flash(f"Deleted docs for project {project.name!r}", queue="success") +