From 0dbe52df13bf3124c67752f42cb481d9618bf9ea Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 18:36:33 -0500 Subject: [PATCH 1/9] Add 'deprecated' column to Classifier --- warehouse/classifiers/models.py | 3 +- ...0b_add_deprecated_column_to_classifiers.py | 41 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 warehouse/migrations/versions/e0ca60b6a30b_add_deprecated_column_to_classifiers.py diff --git a/warehouse/classifiers/models.py b/warehouse/classifiers/models.py index e44ae2604a70..0815c07d4ef9 100644 --- a/warehouse/classifiers/models.py +++ b/warehouse/classifiers/models.py @@ -10,7 +10,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from sqlalchemy import Column, Index, Integer, Text +from sqlalchemy import Boolean, Column, Index, Integer, Text, sql from warehouse import db from warehouse.utils.attrs import make_repr @@ -28,6 +28,7 @@ class Classifier(db.ModelBase): id = Column(Integer, primary_key=True, nullable=False) classifier = Column(Text, unique=True) + deprecated = Column(Boolean, nullable=False, server_default=sql.false()) l2 = Column(Integer) l3 = Column(Integer) l4 = Column(Integer) diff --git a/warehouse/migrations/versions/e0ca60b6a30b_add_deprecated_column_to_classifiers.py b/warehouse/migrations/versions/e0ca60b6a30b_add_deprecated_column_to_classifiers.py new file mode 100644 index 000000000000..76d1051dd87c --- /dev/null +++ b/warehouse/migrations/versions/e0ca60b6a30b_add_deprecated_column_to_classifiers.py @@ -0,0 +1,41 @@ +# 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. +""" +Add 'deprecated' column to classifiers + +Revision ID: e0ca60b6a30b +Revises: 6714f3f04f0f +Create Date: 2018-04-18 23:24:13.009357 +""" + +from alembic import op +import sqlalchemy as sa + + +revision = "e0ca60b6a30b" +down_revision = "6714f3f04f0f" + + +def upgrade(): + op.add_column( + "trove_classifiers", + sa.Column( + "deprecated", + sa.Boolean(), + server_default=sa.text("false"), + nullable=False, + ), + ) + + +def downgrade(): + op.drop_column("trove_classifiers", "deprecated") From d7b45c211916407fac5abf57eb178674811c64fe Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 18:41:03 -0500 Subject: [PATCH 2/9] Get all classifiers, filter in template --- tests/unit/admin/views/test_classifiers.py | 5 ++--- warehouse/admin/templates/admin/classifiers/index.html | 3 +++ warehouse/admin/views/classifiers.py | 1 - 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unit/admin/views/test_classifiers.py b/tests/unit/admin/views/test_classifiers.py index e8d8eb9a3ef4..f417b09900c6 100644 --- a/tests/unit/admin/views/test_classifiers.py +++ b/tests/unit/admin/views/test_classifiers.py @@ -22,9 +22,8 @@ class TestGetClassifiers: def test_get_classifiers(self, db_request): - classifier_a = ClassifierFactory(l5=0, classifier='I am first') - classifier_b = ClassifierFactory(l5=0, classifier='I am last') - ClassifierFactory(l5=1) # Ignored because it has a nonzero L5 + classifier_a = ClassifierFactory(classifier='I am first') + classifier_b = ClassifierFactory(classifier='I am last') assert views.get_classifiers(db_request) == { 'classifiers': [classifier_a, classifier_b], diff --git a/warehouse/admin/templates/admin/classifiers/index.html b/warehouse/admin/templates/admin/classifiers/index.html index 0e5af8cee50f..628727314e49 100644 --- a/warehouse/admin/templates/admin/classifiers/index.html +++ b/warehouse/admin/templates/admin/classifiers/index.html @@ -56,7 +56,10 @@

Add Sub-Classifier

diff --git a/warehouse/admin/views/classifiers.py b/warehouse/admin/views/classifiers.py index 8a4515d7adb1..b50be76e4bd0 100644 --- a/warehouse/admin/views/classifiers.py +++ b/warehouse/admin/views/classifiers.py @@ -25,7 +25,6 @@ def get_classifiers(request): classifiers = ( request.db.query(Classifier) - .filter(Classifier.l5 == 0) # Can be a parent .order_by(Classifier.classifier) .all() ) From 1c696c29f2cbb7125ae74470001726f4da41d26a Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 19:10:28 -0500 Subject: [PATCH 3/9] Add UI to deprecate classifiers --- tests/unit/admin/test_routes.py | 5 +++ tests/unit/admin/views/test_classifiers.py | 15 ++++++++ warehouse/admin/routes.py | 5 +++ .../templates/admin/classifiers/index.html | 37 ++++++++++--------- warehouse/admin/views/classifiers.py | 25 +++++++++++++ 5 files changed, 70 insertions(+), 17 deletions(-) diff --git a/tests/unit/admin/test_routes.py b/tests/unit/admin/test_routes.py index 5e2159e8fd28..278cbef33e21 100644 --- a/tests/unit/admin/test_routes.py +++ b/tests/unit/admin/test_routes.py @@ -101,6 +101,11 @@ def test_includeme(): '/admin/classifiers/add/', domain=warehouse, ), + pretend.call( + 'admin.classifiers.deprecate', + '/admin/classifiers/deprecate/', + domain=warehouse, + ), pretend.call( "admin.blacklist.list", "/admin/blacklist/", diff --git a/tests/unit/admin/views/test_classifiers.py b/tests/unit/admin/views/test_classifiers.py index f417b09900c6..5886d27a16bd 100644 --- a/tests/unit/admin/views/test_classifiers.py +++ b/tests/unit/admin/views/test_classifiers.py @@ -88,3 +88,18 @@ def test_add_parent_classifier(self, db_request): assert new.l3 == 0 assert new.l4 == 0 assert new.l5 == 0 + + +class TestDeprecateClassifier: + + def test_deprecate_classifier(self, db_request): + classifier = ClassifierFactory(deprecated=False) + + db_request.params = {'classifier_id': classifier.id} + db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None) + db_request.route_path = lambda *a: '/the/path' + + views.deprecate_classifier(db_request) + db_request.db.flush() + + assert classifier.deprecated diff --git a/warehouse/admin/routes.py b/warehouse/admin/routes.py index 60f510184ad0..6b096b49268c 100644 --- a/warehouse/admin/routes.py +++ b/warehouse/admin/routes.py @@ -102,6 +102,11 @@ def includeme(config): '/admin/classifiers/add/', domain=warehouse, ) + config.add_route( + 'admin.classifiers.deprecate', + '/admin/classifiers/deprecate/', + domain=warehouse, + ) # Blacklist related Admin pages config.add_route( diff --git a/warehouse/admin/templates/admin/classifiers/index.html b/warehouse/admin/templates/admin/classifiers/index.html index 628727314e49..b9f695645aa1 100644 --- a/warehouse/admin/templates/admin/classifiers/index.html +++ b/warehouse/admin/templates/admin/classifiers/index.html @@ -76,27 +76,30 @@

Add Sub-Classifier

-{#

Deprecate Classifier

-
- -
-
- Deprecate classifier - ? -
- +
+ +
+
+ + +
-
+
+
+ +
+
+
-#} - {% endblock content %} diff --git a/warehouse/admin/views/classifiers.py b/warehouse/admin/views/classifiers.py index b50be76e4bd0..dd0a54c254de 100644 --- a/warehouse/admin/views/classifiers.py +++ b/warehouse/admin/views/classifiers.py @@ -96,3 +96,28 @@ def add_child_classifier(self): ) return HTTPSeeOther(self.request.route_path('admin.classifiers')) + + +@view_config( + route_name='admin.classifiers.deprecate', + permission='admin', + request_method='POST', + uses_session=True, + require_methods=False, + require_csrf=True, +) +def deprecate_classifier(request): + classifier = ( + request.db + .query(Classifier) + .get(request.params.get('classifier_id')) + ) + + classifier.deprecated = True + + request.session.flash( + f'Successfully deprecated classifier {classifier.classifier!r}', + queue='success', + ) + + return HTTPSeeOther(request.route_path('admin.classifiers')) From 2ef661ff441ab14ac6961f72a5d3425277412b08 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 19:13:14 -0500 Subject: [PATCH 4/9] Indicate deprecated classifiers in admin UI --- warehouse/admin/templates/admin/classifiers/index.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/warehouse/admin/templates/admin/classifiers/index.html b/warehouse/admin/templates/admin/classifiers/index.html index b9f695645aa1..147845f29f18 100644 --- a/warehouse/admin/templates/admin/classifiers/index.html +++ b/warehouse/admin/templates/admin/classifiers/index.html @@ -58,7 +58,9 @@

Add Sub-Classifier

{% for classifier in classifiers %} {% if classifier.l5 == 0 %} {# Only show classifiers that can be parent classifiers #} - + {% endif %} {% endfor %} From a66bd30d69434b12bfbc788b484f8dc27d151d05 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 19:13:32 -0500 Subject: [PATCH 5/9] Don't show deprecated classifiers in list_classifiers --- warehouse/legacy/api/pypi.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/warehouse/legacy/api/pypi.py b/warehouse/legacy/api/pypi.py index 85d23db18e3f..f2854629e4b9 100644 --- a/warehouse/legacy/api/pypi.py +++ b/warehouse/legacy/api/pypi.py @@ -81,8 +81,9 @@ def forbidden_legacy(exc, request): def list_classifiers(request): classifiers = ( request.db.query(Classifier.classifier) - .order_by(Classifier.classifier) - .all() + .filter(Classifier.deprecated.is_(False)) + .order_by(Classifier.classifier) + .all() ) return Response( From e155a9f1a2a808dbaef0af510c7ed0206931d02b Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 22:08:58 -0500 Subject: [PATCH 6/9] Don't show deprecated classifiers in search tree --- warehouse/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/warehouse/views.py b/warehouse/views.py index 13bfe40d92a4..0d43419fe781 100644 --- a/warehouse/views.py +++ b/warehouse/views.py @@ -309,6 +309,7 @@ def search(request): classifiers_q = ( request.db.query(Classifier) .with_entities(Classifier.classifier) + .filter(Classifier.deprecated.is_(False)) .filter( exists([release_classifiers.c.trove_id]) .where(release_classifiers.c.trove_id == Classifier.id) From bba9ee35775d01b4a549af14bfccae1c0e9a4ab7 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 22:28:40 -0500 Subject: [PATCH 7/9] Add a page with a list of classifiers --- tests/unit/test_routes.py | 1 + tests/unit/test_views.py | 15 +++++++-- warehouse/routes.py | 3 ++ warehouse/templates/pages/classifiers.html | 38 ++++++++++++++++++++++ warehouse/templates/pages/help.html | 9 +++-- warehouse/views.py | 17 ++++++++++ 6 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 warehouse/templates/pages/classifiers.html diff --git a/tests/unit/test_routes.py b/tests/unit/test_routes.py index 1eca0e123059..e06d0eb79979 100644 --- a/tests/unit/test_routes.py +++ b/tests/unit/test_routes.py @@ -114,6 +114,7 @@ def add_policy(name, filename): traverse="/{username}", domain=warehouse, ), + pretend.call("classifiers", "/classifiers/", domain=warehouse), pretend.call("search", "/search/", domain=warehouse), pretend.call( "accounts.profile", diff --git a/tests/unit/test_views.py b/tests/unit/test_views.py index f375789df022..5abe94c6eaf5 100644 --- a/tests/unit/test_views.py +++ b/tests/unit/test_views.py @@ -23,9 +23,9 @@ from warehouse import views from warehouse.views import ( - SEARCH_BOOSTS, SEARCH_FIELDS, current_user_indicator, forbidden, health, - httpexception_view, index, robotstxt, opensearchxml, search, force_status, - flash_messages, forbidden_include + SEARCH_BOOSTS, SEARCH_FIELDS, classifiers, current_user_indicator, + forbidden, health, httpexception_view, index, robotstxt, opensearchxml, + search, force_status, flash_messages, forbidden_include ) from ..common.db.accounts import UserFactory @@ -542,6 +542,15 @@ def test_raises_400_with_pagenum_type_str(self, monkeypatch, db_request): assert page_cls.calls == [] +def test_classifiers(db_request): + classifier_a = ClassifierFactory(classifier='I am first') + classifier_b = ClassifierFactory(classifier='I am last') + + assert classifiers(db_request) == { + 'classifiers': [(classifier_a.classifier,), (classifier_b.classifier,)] + } + + def test_health(): request = pretend.stub( db=pretend.stub( diff --git a/warehouse/routes.py b/warehouse/routes.py index 6eb2384f80ea..a04e4c3a5ce6 100644 --- a/warehouse/routes.py +++ b/warehouse/routes.py @@ -83,6 +83,9 @@ def includeme(config): domain=warehouse, ) + # Classifier Routes + config.add_route("classifiers", "/classifiers/", domain=warehouse) + # Search Routes config.add_route("search", "/search/", domain=warehouse) diff --git a/warehouse/templates/pages/classifiers.html b/warehouse/templates/pages/classifiers.html new file mode 100644 index 000000000000..f4059c1a0f51 --- /dev/null +++ b/warehouse/templates/pages/classifiers.html @@ -0,0 +1,38 @@ +{# + # 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. +-#} +{% extends "base.html" %} + +{% block title %}Classifiers{% endblock %} + +{% block content %} +
+
+

Classifiers

+

Each project's maintainers provide PyPI with a list of "trove classifiers" to categorize each release, describing who it's for, what systems it can run on, and how mature it is.

+

These standardized classifiers can then be used by community members to find projects based on their desired criteria.

+

Instructions for how to add trove classifiers to a project can be found on the Python Packaging User Guide. To read the original classifier specification, refer to PEP 301.

+ +

List of classifiers

+ +
+
+{% endblock %} diff --git a/warehouse/templates/pages/help.html b/warehouse/templates/pages/help.html index 668ed27fa3e3..ba9dcbacfa0f 100644 --- a/warehouse/templates/pages/help.html +++ b/warehouse/templates/pages/help.html @@ -132,9 +132,12 @@

{{ publishing() }}

{{ trove_classifier() }}

-

Each project's maintainers provide PyPI with a list of "trove classifiers" to categorize each release, describing who it's for, what systems it can run on, and how mature it is.

-

These standardized classifiers can then be used by community members to find projects based on their desired criteria.

-

Instructions for how to add trove classifiers to a project can be found on the Python Packaging User Guide. To read the original classifier specification, refer to PEP 301.

+

+ Classifiers are used to categorize projects on PyPI. + See + {{ request.route_url('classifiers') }} + for more information, as well as a list of valid classifiers. +

{{ verified_email() }}

diff --git a/warehouse/views.py b/warehouse/views.py index 0d43419fe781..b6012d9877b5 100644 --- a/warehouse/views.py +++ b/warehouse/views.py @@ -235,6 +235,23 @@ def index(request): } +@view_config( + route_name="classifiers", + renderer="pages/classifiers.html", +) +def classifiers(request): + classifiers = ( + request.db.query(Classifier.classifier) + .filter(Classifier.deprecated.is_(False)) + .order_by(Classifier.classifier) + .all() + ) + + return { + 'classifiers': classifiers + } + + @view_config( route_name="search", renderer="search/results.html", From 1f5ffb3262425e13d19c30927374d0a48ab4b07e Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Wed, 18 Apr 2018 22:35:58 -0500 Subject: [PATCH 8/9] Add copy to clipboard link --- warehouse/templates/pages/classifiers.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/warehouse/templates/pages/classifiers.html b/warehouse/templates/pages/classifiers.html index f4059c1a0f51..486e1efd2de3 100644 --- a/warehouse/templates/pages/classifiers.html +++ b/warehouse/templates/pages/classifiers.html @@ -28,7 +28,11 @@

List of classifiers

{% for classifier in classifiers %}
  • - {{ classifier.classifier }} + {{ classifier.classifier }}  + + + Copy classifier +
    • {% endfor %} From b91b0ca0954ed0893ab81225ad35c3d59d946039 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Thu, 19 Apr 2018 06:33:29 -0500 Subject: [PATCH 9/9] Validate deprecated classifiers on upload --- tests/unit/forklift/test_legacy.py | 68 ++++++++++++++++++++++++++++++ warehouse/forklift/legacy.py | 29 +++++++++++++ 2 files changed, 97 insertions(+) diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index a1a282fd7e73..50319fc732b2 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -39,6 +39,7 @@ from warehouse.admin.flags import AdminFlag from ...common.db.accounts import UserFactory, EmailFactory +from ...common.db.classifiers import ClassifierFactory from ...common.db.packaging import ( ProjectFactory, ReleaseFactory, FileFactory, RoleFactory, ) @@ -326,6 +327,31 @@ def test_validate_description_content_type_invalid(self, data): with pytest.raises(ValidationError): legacy._validate_description_content_type(form, field) + def test_validate_no_deprecated_classifiers_valid(self, db_request): + valid_classifier = ClassifierFactory(deprecated=False) + validator = legacy._no_deprecated_classifiers(db_request) + + form = pretend.stub() + field = pretend.stub(data=[valid_classifier.classifier]) + + validator(form, field) + + def test_validate_no_deprecated_classifiers_invalid(self, db_request): + deprecated_classifier = ClassifierFactory( + classifier='AA: BB', deprecated=True, + ) + validator = legacy._no_deprecated_classifiers(db_request) + db_request.registry = pretend.stub( + settings={'warehouse.domain': 'host'} + ) + db_request.route_url = pretend.call_recorder(lambda *a, **kw: '/url') + + form = pretend.stub() + field = pretend.stub(data=[deprecated_classifier.classifier]) + + with pytest.raises(ValidationError): + validator(form, field) + def test_construct_dependencies(): types = { @@ -1522,6 +1548,48 @@ def test_upload_fails_with_invalid_classifier(self, pyramid_config, "for this field" ) + def test_upload_fails_with_deprecated_classifier( + self, pyramid_config, db_request): + pyramid_config.testing_securitypolicy(userid=1) + + user = UserFactory.create() + EmailFactory.create(user=user) + project = ProjectFactory.create() + release = ReleaseFactory.create(project=project, version="1.0") + RoleFactory.create(user=user, project=project) + classifier = ClassifierFactory(classifier='AA :: BB', deprecated=True) + + filename = "{}-{}.tar.gz".format(project.name, release.version) + + db_request.POST = MultiDict({ + "metadata_version": "1.2", + "name": project.name, + "version": release.version, + "filetype": "sdist", + "md5_digest": "335c476dc930b959dda9ec82bd65ef19", + "content": pretend.stub( + filename=filename, + file=io.BytesIO(b"A fake file."), + type="application/tar", + ), + }) + db_request.POST.extend([ + ("classifiers", classifier.classifier), + ]) + db_request.route_url = pretend.call_recorder(lambda *a, **kw: '/url') + + with pytest.raises(HTTPBadRequest) as excinfo: + legacy.file_upload(db_request) + + resp = excinfo.value + + assert resp.status_code == 400 + assert resp.status == ( + "400 Invalid value for classifiers. " + "Error: Classifier 'AA :: BB' has been deprecated, see /url " + "for a list of valid classifiers." + ) + @pytest.mark.parametrize( "digests", [ diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index 561fdec5eec4..1d9b78777604 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -696,6 +696,32 @@ def _is_duplicate_file(db_session, filename, hashes): return None +def _no_deprecated_classifiers(request): + deprecated_classifiers = { + classifier.classifier + for classifier in ( + request.db.query(Classifier.classifier) + .filter(Classifier.deprecated.is_(True)) + .all() + ) + } + + def validate_no_deprecated_classifiers(form, field): + invalid_classifiers = set(field.data or []) & deprecated_classifiers + if invalid_classifiers: + first_invalid_classifier = sorted(invalid_classifiers)[0] + host = request.registry.settings.get("warehouse.domain") + classifiers_url = request.route_url('classifiers', _host=host) + + raise wtforms.validators.ValidationError( + f'Classifier {first_invalid_classifier!r} has been ' + f'deprecated, see {classifiers_url} for a list of valid ' + 'classifiers.' + ) + + return validate_no_deprecated_classifiers + + @view_config( route_name="forklift.legacy.file_upload", uses_session=True, @@ -757,6 +783,9 @@ def file_upload(request): # Validate and process the incoming metadata. form = MetadataForm(request.POST) + # Add a validator for deprecated classifiers + form.classifiers.validators.append(_no_deprecated_classifiers(request)) + form.classifiers.choices = [ (c.classifier, c.classifier) for c in all_classifiers ]