diff --git a/requirements/main.in b/requirements/main.in index f39a8dd9b5cb..5909c1630099 100644 --- a/requirements/main.in +++ b/requirements/main.in @@ -53,6 +53,7 @@ sqlalchemy-citext stdlib-list structlog transaction +trove-classifiers typeguard webauthn whitenoise diff --git a/requirements/main.txt b/requirements/main.txt index f33964e53f2d..66fbe01c5ea2 100644 --- a/requirements/main.txt +++ b/requirements/main.txt @@ -569,6 +569,9 @@ transaction==3.0.0 \ translationstring==1.3 \ --hash=sha256:4ee44cfa58c52ade8910ea0ebc3d2d84bdcad9fa0422405b1801ec9b9a65b72d \ --hash=sha256:e26c7bf383413234ed442e0980a2ebe192b95e3745288a8fd2805156d27515b4 +trove-classifiers==2020.4.1 \ + --hash=sha256:9e1dcd47920817eaeb4cc67004b3fee430f3fc692e926f6ab1e337035b7a590d \ + --hash=sha256:d8adb5d687ee15fe83c4c23404a8fbc0ff267ca997c6870419cc625fdea449e0 typeguard==2.7.1 \ --hash=sha256:1d3710251d3d3d6c64e0c49f45edec2e88ddc386a51e89c3ec0703efeb8b3b81 \ --hash=sha256:2d545c71e9439c21bcd7c28f5f55b3606e6106f7031ab58375656a1aed483ef2 diff --git a/tests/common/db/classifiers.py b/tests/common/db/classifiers.py index 2656d78860ae..01118c8bde67 100644 --- a/tests/common/db/classifiers.py +++ b/tests/common/db/classifiers.py @@ -10,9 +10,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import factory -import factory.fuzzy - from warehouse.classifiers.models import Classifier from .base import WarehouseFactory @@ -21,8 +18,3 @@ class ClassifierFactory(WarehouseFactory): class Meta: model = Classifier - - l2 = factory.fuzzy.FuzzyInteger(0) - l3 = factory.fuzzy.FuzzyInteger(0) - l4 = factory.fuzzy.FuzzyInteger(0) - l5 = factory.fuzzy.FuzzyInteger(0) diff --git a/tests/unit/admin/test_routes.py b/tests/unit/admin/test_routes.py index 18ea9a3738e2..f3739ef18b8c 100644 --- a/tests/unit/admin/test_routes.py +++ b/tests/unit/admin/test_routes.py @@ -101,15 +101,6 @@ def test_includeme(): domain=warehouse, ), pretend.call("admin.journals.list", "/admin/journals/", domain=warehouse), - pretend.call("admin.classifiers", "/admin/classifiers/", domain=warehouse), - pretend.call( - "admin.classifiers.add", "/admin/classifiers/add/", domain=warehouse - ), - pretend.call( - "admin.classifiers.deprecate", - "/admin/classifiers/deprecate/", - domain=warehouse, - ), pretend.call("admin.blacklist.list", "/admin/blacklist/", domain=warehouse), pretend.call("admin.blacklist.add", "/admin/blacklist/add/", domain=warehouse), pretend.call( diff --git a/tests/unit/admin/views/test_classifiers.py b/tests/unit/admin/views/test_classifiers.py deleted file mode 100644 index b3be0dad91ed..000000000000 --- a/tests/unit/admin/views/test_classifiers.py +++ /dev/null @@ -1,134 +0,0 @@ -# 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. - -import pretend -import pytest -import sqlalchemy - -from warehouse.admin.views import classifiers as views -from warehouse.classifiers.models import Classifier - -from ....common.db.classifiers import ClassifierFactory - - -class TestGetClassifiers: - def test_get_classifiers(self, db_request): - 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] - } - - -class TestAddClassifier: - @pytest.mark.parametrize( - "parent_levels, expected_levels", - [ - ((2, 0, 0, 0), (2, None, 0, 0)), - ((2, 3, 0, 0), (2, 3, None, 0)), - ((2, 3, 4, 0), (2, 3, 4, None)), - # This won't actually happen but it's needed for coverage - ((2, 3, 4, 5), (2, 3, 4, 5)), - ], - ) - def test_add_child_classifier(self, db_request, parent_levels, expected_levels): - l2, l3, l4, l5 = parent_levels - parent = ClassifierFactory(l2=l2, l3=l3, l4=l4, l5=l5, classifier="Parent") - - db_request.params = {"parent_id": parent.id, "child": "Foobar"} - db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None) - db_request.route_path = lambda *a: "/the/path" - - views.AddClassifier(db_request).add_child_classifier() - - new = ( - db_request.db.query(Classifier) - .filter(Classifier.classifier == "Parent :: Foobar") - .one() - ) - - new_l2, new_l3, new_l4, new_l5 = expected_levels - assert new.l2 == new_l2 if new_l2 is not None else new.id - assert new.l3 == new_l3 if new_l3 is not None else new.id - assert new.l4 == new_l4 if new_l4 is not None else new.id - assert new.l5 == new_l5 if new_l5 is not None else new.id - - def test_add_parent_classifier(self, db_request): - db_request.params = {"parent": "Foo :: Bar"} - db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None) - db_request.route_path = lambda *a: "/the/path" - - views.AddClassifier(db_request).add_parent_classifier() - - new = ( - db_request.db.query(Classifier) - .filter(Classifier.classifier == "Foo :: Bar") - .one() - ) - - assert new.l2 == new.id - assert new.l3 == 0 - assert new.l4 == 0 - assert new.l5 == 0 - - @pytest.mark.parametrize( - "parent_classifier, parent_levels, expected_levels", - [ - ("private", (2, 0, 0, 0), (2, None, 0, 0)), - ("private", (2, 3, 0, 0), (2, 3, None, 0)), - ("private", (2, 3, 4, 0), (2, 3, 4, None)), - ("Private", (2, 0, 0, 0), (2, None, 0, 0)), - ("Private", (2, 3, 0, 0), (2, 3, None, 0)), - ("Private", (2, 3, 4, 0), (2, 3, 4, None)), - ("PrIvAtE", (2, 0, 0, 0), (2, None, 0, 0)), - ("PrIvAtE", (2, 3, 0, 0), (2, 3, None, 0)), - ("PrIvAtE", (2, 3, 4, 0), (2, 3, 4, None)), - ], - ) - def test_add_private_child_classifier( - self, db_request, parent_classifier, parent_levels, expected_levels - ): - l2, l3, l4, l5 = parent_levels - parent = ClassifierFactory( - l2=l2, l3=l3, l4=l4, l5=l5, classifier=parent_classifier - ) - - db_request.params = {"parent_id": parent.id, "child": "Foobar"} - db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None) - db_request.route_path = lambda *a: "/the/path" - - with pytest.raises(sqlalchemy.exc.IntegrityError): - views.AddClassifier(db_request).add_child_classifier() - - @pytest.mark.parametrize("parent_classifier", ["private", "Private", "PrIvAtE"]) - def test_add_private_parent_classifier(self, db_request, parent_classifier): - db_request.params = {"parent": f"{parent_classifier} :: Do Not Upload"} - db_request.session.flash = pretend.call_recorder(lambda *a, **kw: None) - db_request.route_path = lambda *a: "/the/path" - - with pytest.raises(sqlalchemy.exc.IntegrityError): - views.AddClassifier(db_request).add_parent_classifier() - - -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/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index d3e8543f23ef..abe3dfa0d370 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -26,7 +26,9 @@ import requests from pyramid.httpexceptions import HTTPBadRequest, HTTPForbidden +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import joinedload +from trove_classifiers import classifiers from webob.multidict import MultiDict from wtforms.form import Form from wtforms.validators import ValidationError @@ -328,25 +330,42 @@ def test_validate_description_content_type_invalid(self, data): 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) + valid_classifier = ClassifierFactory(classifier="AA :: BB") form = pretend.stub() field = pretend.stub(data=[valid_classifier.classifier]) - validator(form, field) + legacy._validate_no_deprecated_classifiers(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") + @pytest.mark.parametrize( + "deprecated_classifiers", [({"AA :: BB": []}), ({"AA :: BB": ["CC :: DD"]})] + ) + def test_validate_no_deprecated_classifiers_invalid( + self, db_request, deprecated_classifiers, monkeypatch + ): + monkeypatch.setattr(legacy, "deprecated_classifiers", deprecated_classifiers) + + form = pretend.stub() + field = pretend.stub(data=["AA :: BB"]) + + with pytest.raises(ValidationError): + legacy._validate_no_deprecated_classifiers(form, field) + + def test_validate_classifiers_valid(self, db_request, monkeypatch): + monkeypatch.setattr(legacy, "classifiers", {"AA :: BB"}) + + form = pretend.stub() + field = pretend.stub(data=["AA :: BB"]) + legacy._validate_classifiers(form, field) + + @pytest.mark.parametrize("data", [(["AA :: BB"]), (["AA :: BB", "CC :: DD"])]) + def test_validate_classifiers_invalid(self, db_request, data): form = pretend.stub() - field = pretend.stub(data=[deprecated_classifier.classifier]) + field = pretend.stub(data=data) with pytest.raises(ValidationError): - validator(form, field) + legacy._validate_classifiers(form, field) def test_construct_dependencies(): @@ -1639,7 +1658,7 @@ def test_upload_fails_with_invalid_classifier(self, pyramid_config, db_request): ), } ) - db_request.POST.extend([("classifiers", "Environment :: Other Environment")]) + db_request.POST.extend([("classifiers", "Invalid :: Classifier")]) with pytest.raises(HTTPBadRequest) as excinfo: legacy.file_upload(db_request) @@ -1648,12 +1667,29 @@ def test_upload_fails_with_invalid_classifier(self, pyramid_config, db_request): assert resp.status_code == 400 assert resp.status == ( - "400 Invalid value for classifiers. " - "Error: 'Environment :: Other Environment' is not a valid choice " - "for this field" + "400 Invalid value for classifiers. Error: Classifier 'Invalid :: " + "Classifier' is not a valid classifier." ) - def test_upload_fails_with_deprecated_classifier(self, pyramid_config, db_request): + @pytest.mark.parametrize( + "deprecated_classifiers, expected", + [ + ( + {"AA :: BB": ["CC :: DD"]}, + "400 Invalid value for classifiers. Error: Classifier 'AA :: " + "BB' has been deprecated, use the following classifier(s) " + "instead: ['CC :: DD']", + ), + ( + {"AA :: BB": []}, + "400 Invalid value for classifiers. Error: Classifier 'AA :: " + "BB' has been deprecated.", + ), + ], + ) + def test_upload_fails_with_deprecated_classifier( + self, pyramid_config, db_request, monkeypatch, deprecated_classifiers, expected + ): pyramid_config.testing_securitypolicy(userid=1) user = UserFactory.create() @@ -1662,7 +1698,9 @@ def test_upload_fails_with_deprecated_classifier(self, pyramid_config, db_reques project = ProjectFactory.create() release = ReleaseFactory.create(project=project, version="1.0") RoleFactory.create(user=user, project=project) - classifier = ClassifierFactory(classifier="AA :: BB", deprecated=True) + classifier = ClassifierFactory(classifier="AA :: BB") + + monkeypatch.setattr(legacy, "deprecated_classifiers", deprecated_classifiers) filename = "{}-{}.tar.gz".format(project.name, release.version) @@ -1689,11 +1727,7 @@ def test_upload_fails_with_deprecated_classifier(self, pyramid_config, db_reques 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." - ) + assert resp.status == expected @pytest.mark.parametrize( "digests", @@ -2786,6 +2820,91 @@ def test_upload_succeeds_creates_release(self, pyramid_config, db_request, metri ), ] + def test_upload_succeeds_creates_classifier( + self, pyramid_config, db_request, metrics, monkeypatch + ): + pyramid_config.testing_securitypolicy(userid=1) + + user = UserFactory.create() + EmailFactory.create(user=user) + project = ProjectFactory.create() + RoleFactory.create(user=user, project=project) + + monkeypatch.setattr(legacy, "classifiers", {"AA :: BB", "CC :: DD"}) + + db_request.db.add(Classifier(classifier="AA :: BB")) + + filename = "{}-{}.tar.gz".format(project.name, "1.0") + + db_request.user = user + db_request.remote_addr = "10.10.10.20" + db_request.user_agent = "warehouse-tests/6.6.6" + db_request.POST = MultiDict( + { + "metadata_version": "1.2", + "name": project.name, + "version": "1.0", + "summary": "This is my summary!", + "filetype": "sdist", + "md5_digest": _TAR_GZ_PKG_MD5, + "content": pretend.stub( + filename=filename, + file=io.BytesIO(_TAR_GZ_PKG_TESTDATA), + type="application/tar", + ), + } + ) + db_request.POST.extend( + [ + ("classifiers", "AA :: BB"), + ("classifiers", "CC :: DD"), + ("requires_dist", "foo"), + ("requires_dist", "bar (>1.0)"), + ("project_urls", "Test, https://example.com/"), + ("requires_external", "Cheese (>1.0)"), + ("provides", "testing"), + ] + ) + + storage_service = pretend.stub(store=lambda path, filepath, meta: None) + db_request.find_service = lambda svc, name=None, context=None: { + IFileStorage: storage_service, + IMetricsService: metrics, + }.get(svc) + + resp = legacy.file_upload(db_request) + + assert resp.status_code == 200 + + # Ensure that a new Classifier has been created + classifier = ( + db_request.db.query(Classifier) + .filter(Classifier.classifier == "CC :: DD") + .one() + ) + assert classifier.classifier == "CC :: DD" + + # Ensure that the Release has the new classifier + release = ( + db_request.db.query(Release) + .filter((Release.project == project) & (Release.version == "1.0")) + .one() + ) + assert release.classifiers == ["AA :: BB", "CC :: DD"] + + def test_all_valid_classifiers_can_be_created(self, db_request): + for classifier in classifiers: + db_request.db.add(Classifier(classifier=classifier)) + db_request.db.commit() + + @pytest.mark.parametrize( + "parent_classifier", ["private", "Private", "PrIvAtE"], + ) + def test_private_classifiers_cannot_be_created(self, db_request, parent_classifier): + with pytest.raises(IntegrityError): + db_request.db.add(Classifier(classifier=f"{parent_classifier} :: Foo")) + db_request.db.commit() + def test_equivalent_version_one_release(self, pyramid_config, db_request, metrics): """ Test that if a release with a version like '1.0' exists, that a future diff --git a/tests/unit/legacy/api/test_pypi.py b/tests/unit/legacy/api/test_pypi.py index 9082b9a33cf5..429daa77227f 100644 --- a/tests/unit/legacy/api/test_pypi.py +++ b/tests/unit/legacy/api/test_pypi.py @@ -14,6 +14,7 @@ import pytest from pyramid.httpexceptions import HTTPBadRequest, HTTPMovedPermanently, HTTPNotFound +from trove_classifiers import classifiers from warehouse.legacy.api import pypi @@ -67,14 +68,10 @@ def test_forbidden_legacy(): def test_list_classifiers(db_request): - ClassifierFactory.create(classifier="foo :: bar") - ClassifierFactory.create(classifier="foo :: baz") - ClassifierFactory.create(classifier="fiz :: buz") - resp = pypi.list_classifiers(db_request) assert resp.status_code == 200 - assert resp.text == "fiz :: buz\nfoo :: bar\nfoo :: baz" + assert resp.text == "\n".join(sorted(classifiers)) def test_search(): diff --git a/tests/unit/test_views.py b/tests/unit/test_views.py index f03729e88eaf..ff0a9af04e1e 100644 --- a/tests/unit/test_views.py +++ b/tests/unit/test_views.py @@ -22,11 +22,11 @@ HTTPSeeOther, HTTPServiceUnavailable, ) +from trove_classifiers import classifiers from webob.multidict import MultiDict from warehouse import views from warehouse.views import ( - classifiers, current_user_indicator, flash_messages, forbidden, @@ -35,6 +35,7 @@ health, httpexception_view, index, + list_classifiers, locale, opensearchxml, robotstxt, @@ -424,12 +425,7 @@ def raiser(*args, **kwargs): 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,)] - } + assert list_classifiers(db_request) == {"classifiers": sorted(classifiers)} def test_stats(db_request): diff --git a/warehouse/admin/routes.py b/warehouse/admin/routes.py index 67e6ee495325..f9ed9d7fb1e3 100644 --- a/warehouse/admin/routes.py +++ b/warehouse/admin/routes.py @@ -99,15 +99,6 @@ def includeme(config): # Journal related Admin pages config.add_route("admin.journals.list", "/admin/journals/", domain=warehouse) - # Classifier related Admin pages - config.add_route("admin.classifiers", "/admin/classifiers/", domain=warehouse) - config.add_route( - "admin.classifiers.add", "/admin/classifiers/add/", domain=warehouse - ) - config.add_route( - "admin.classifiers.deprecate", "/admin/classifiers/deprecate/", domain=warehouse - ) - # Blacklist related Admin pages config.add_route("admin.blacklist.list", "/admin/blacklist/", domain=warehouse) config.add_route("admin.blacklist.add", "/admin/blacklist/add/", domain=warehouse) diff --git a/warehouse/admin/static/js/controllers/child_classifier_controller.js b/warehouse/admin/static/js/controllers/child_classifier_controller.js deleted file mode 100644 index 7cc8a07f881b..000000000000 --- a/warehouse/admin/static/js/controllers/child_classifier_controller.js +++ /dev/null @@ -1,36 +0,0 @@ -/** - * 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. - */ - - -import { Controller } from "stimulus"; - -export default class extends Controller { - static targets = [ - "parent", "input", "preview", "submit", - ] - - update() { - // Set the preview - this.previewTarget.innerHTML = [ - this.parentTarget.options[this.parentTarget.selectedIndex].text, - this.inputTarget.value, - ].join(" :: "); - - // Enable the input target - this.inputTarget.disabled = !this.parentTarget.value; - - // Enable the submit button - this.submitTarget.disabled = !(this.parentTarget.value && this.inputTarget.value); - } -} diff --git a/warehouse/admin/static/js/controllers/parent_classifier_controller.js b/warehouse/admin/static/js/controllers/parent_classifier_controller.js deleted file mode 100644 index dfd3cad456bd..000000000000 --- a/warehouse/admin/static/js/controllers/parent_classifier_controller.js +++ /dev/null @@ -1,42 +0,0 @@ -/** - * 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. - */ - - -import { Controller } from "stimulus"; - -export default class extends Controller { - static targets = ["input", "preview", "submit"] - - update() { - // Set the preview - this.previewTarget.innerHTML = this.inputTarget.value; - - // Classifier is made up of words which can contain a-z, A-Z, 0-9, - // underscore, hyphen, period, octothorp, plus, or parentheses - var word_sub = "[\\w.\\(\\)\\+#-]"; - - // Words can be repeated one or more times, separated by a space - var words_sub = `${word_sub}+(\\s${word_sub}*)*`; - - // Classifer must have two parts, separated by a ' :: ' - var classifier_re = new RegExp(`^${words_sub} :: ${words_sub}$`); - - if (classifier_re.test(this.inputTarget.value)) { - // Enable the submit button - this.submitTarget.disabled = false; - } else { - this.submitTarget.disabled = true; - } - } -} diff --git a/warehouse/admin/static/js/warehouse.js b/warehouse/admin/static/js/warehouse.js index acd0c11943e3..1374a3e98450 100644 --- a/warehouse/admin/static/js/warehouse.js +++ b/warehouse/admin/static/js/warehouse.js @@ -11,10 +11,6 @@ * limitations under the License. */ -// Import stimulus -import { Application } from "stimulus"; -import { definitionsFromContext } from "stimulus/webpack-helpers"; - document.querySelectorAll("a[data-form-submit]").forEach(function (element) { element.addEventListener("click", function(event) { // We're turning this element into a form submission, so instead of the @@ -25,7 +21,3 @@ document.querySelectorAll("a[data-form-submit]").forEach(function (element) { document.querySelector("form#" + element.dataset.formSubmit).submit(); }); }); - -const application = Application.start(); -const context = require.context("./controllers", true, /\.js$/); -application.load(definitionsFromContext(context)); diff --git a/warehouse/admin/templates/admin/base.html b/warehouse/admin/templates/admin/base.html index 18be097d6576..3049334de164 100644 --- a/warehouse/admin/templates/admin/base.html +++ b/warehouse/admin/templates/admin/base.html @@ -100,11 +100,6 @@ Journals -
  • - - Classifiers - -
  • Blacklist diff --git a/warehouse/admin/templates/admin/classifiers/index.html b/warehouse/admin/templates/admin/classifiers/index.html deleted file mode 100644 index 08b30e100357..000000000000 --- a/warehouse/admin/templates/admin/classifiers/index.html +++ /dev/null @@ -1,107 +0,0 @@ -{# - # 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 "admin/base.html" %} - -{% import "admin/utils/pagination.html" as pagination %} - -{% block title %}Classifiers{% endblock %} - -{% block breadcrumb %} -
  • Classifiers
    • -{% endblock %} - -{% block content %} -
    -
    -

    Add New Top-Level Classifier

    -
    -
    - -
    -
    - - -
    -
    -
    - Add new classifier - ? -
    - -
    -
    -
    -
    - -
    -
    -

    Add Sub-Classifier

    -
    -
    - -
    -
    - - - - -
    -
    -
    - Add new classifier - ? -
    - -
    -
    -
    -
    - -
    -
    -

    Deprecate Classifier

    -
    -
    - -
    -
    - - -
    -
    -
    -
    - -
    -
    -
    -
    -{% endblock content %} diff --git a/warehouse/admin/views/classifiers.py b/warehouse/admin/views/classifiers.py deleted file mode 100644 index dc16910b0fad..000000000000 --- a/warehouse/admin/views/classifiers.py +++ /dev/null @@ -1,106 +0,0 @@ -# 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. - -from pyramid.httpexceptions import HTTPSeeOther -from pyramid.view import view_config, view_defaults - -from warehouse.packaging.models import Classifier - - -@view_config( - route_name="admin.classifiers", - renderer="admin/classifiers/index.html", - permission="moderator", - request_method="GET", - uses_session=True, -) -def get_classifiers(request): - classifiers = request.db.query(Classifier).order_by(Classifier.classifier).all() - - return {"classifiers": classifiers} - - -@view_defaults( - route_name="admin.classifiers.add", - permission="moderator", - request_method="POST", - uses_session=True, - require_methods=False, - require_csrf=True, -) -class AddClassifier: - def __init__(self, request): - self.request = request - - @view_config(request_param=["parent"]) - def add_parent_classifier(self): - classifier = Classifier( - classifier=self.request.params.get("parent"), l3=0, l4=0, l5=0 - ) - - self.request.db.add(classifier) - self.request.db.flush() # To get the ID - - classifier.l2 = classifier.id - - self.request.session.flash( - f"Added classifier {classifier.classifier!r}", queue="success" - ) - - return HTTPSeeOther(self.request.route_path("admin.classifiers")) - - @view_config(request_param=["parent_id", "child"]) - def add_child_classifier(self): - parent = self.request.db.query(Classifier).get( - self.request.params.get("parent_id") - ) - - classifier = Classifier( - l2=parent.l2, - l3=parent.l3, - l4=parent.l4, - l5=parent.l5, - classifier=(parent.classifier + " :: " + self.request.params.get("child")), - ) - self.request.db.add(classifier) - self.request.db.flush() # To get the ID - - for level in ["l3", "l4", "l5"]: - if getattr(classifier, level) == 0: - setattr(classifier, level, classifier.id) - break - - self.request.session.flash( - f"Added classifier {classifier.classifier!r}", queue="success" - ) - - return HTTPSeeOther(self.request.route_path("admin.classifiers")) - - -@view_config( - route_name="admin.classifiers.deprecate", - permission="moderator", - 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"Deprecated classifier {classifier.classifier!r}", queue="success" - ) - - return HTTPSeeOther(request.route_path("admin.classifiers")) diff --git a/warehouse/classifiers/models.py b/warehouse/classifiers/models.py index f4eecefce20c..3564a7ac5a30 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 Boolean, CheckConstraint, Column, Integer, Text, sql +from sqlalchemy import CheckConstraint, Column, Integer, Text from warehouse import db from warehouse.utils.attrs import make_repr @@ -28,8 +28,3 @@ 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) - l5 = Column(Integer) diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index 5d90ccf2963e..2159762e8307 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -37,6 +37,7 @@ from pyramid.view import view_config from sqlalchemy import exists, func, orm from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound +from trove_classifiers import classifiers, deprecated_classifiers from warehouse import forms from warehouse.admin.flags import AdminFlagValue @@ -340,6 +341,38 @@ def _raise(message): ) +def _validate_no_deprecated_classifiers(form, field): + invalid_classifiers = set(field.data or []) & deprecated_classifiers.keys() + if invalid_classifiers: + first_invalid_classifier_name = sorted(invalid_classifiers)[0] + deprecated_by = deprecated_classifiers[first_invalid_classifier_name] + + if deprecated_by: + raise wtforms.validators.ValidationError( + f"Classifier {first_invalid_classifier_name!r} has been " + "deprecated, use the following classifier(s) instead: " + f"{deprecated_by}" + ) + else: + raise wtforms.validators.ValidationError( + f"Classifier {first_invalid_classifier_name!r} has been deprecated." + ) + + +def _validate_classifiers(form, field): + invalid = sorted(set(field.data or []) - classifiers) + + if invalid: + if len(invalid) == 1: + raise wtforms.validators.ValidationError( + f"Classifier {invalid[0]!r} is not a valid classifier." + ) + else: + raise wtforms.validators.ValidationError( + f"Classifiers {invalid!r} are not valid classifiers." + ) + + def _construct_dependencies(form, types): for name, kind in types.items(): for item in getattr(form, name).data: @@ -437,7 +470,10 @@ class MetadataForm(forms.Form): keywords = wtforms.StringField( description="Keywords", validators=[wtforms.validators.Optional()] ) - classifiers = wtforms.fields.SelectMultipleField(description="Classifier") + classifiers = ListField( + description="Classifier", + validators=[_validate_no_deprecated_classifiers, _validate_classifiers], + ) platform = wtforms.StringField( description="Platform", validators=[wtforms.validators.Optional()] ) @@ -702,32 +738,6 @@ 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, @@ -816,16 +826,9 @@ def file_upload(request): if any(isinstance(value, FieldStorage) for value in values): raise _exc_with_message(HTTPBadRequest, f"{field}: Should not be a tuple.") - # Look up all of the valid classifiers - all_classifiers = request.db.query(Classifier).all() - # 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] if not form.validate(): for field_name in _error_message_order: if field_name in form.errors: @@ -1054,11 +1057,29 @@ def file_upload(request): .one() ) except NoResultFound: + # Look up all of the valid classifiers + all_classifiers = request.db.query(Classifier).all() + + # Get all the classifiers for this release + release_classifiers = [ + c for c in all_classifiers if c.classifier in form.classifiers.data + ] + + # Determine if we need to add any new classifiers to the database + missing_classifiers = set(form.classifiers.data or []) - set( + c.classifier for c in release_classifiers + ) + + # Add any new classifiers to the database + if missing_classifiers: + for missing_classifier_name in missing_classifiers: + missing_classifier = Classifier(classifier=missing_classifier_name) + request.db.add(missing_classifier) + release_classifiers.append(missing_classifier) + release = Release( project=project, - _classifiers=[ - c for c in all_classifiers if c.classifier in form.classifiers.data - ], + _classifiers=release_classifiers, dependencies=list( _construct_dependencies( form, diff --git a/warehouse/legacy/api/pypi.py b/warehouse/legacy/api/pypi.py index d595ab2702fc..c0f41b72d3be 100644 --- a/warehouse/legacy/api/pypi.py +++ b/warehouse/legacy/api/pypi.py @@ -13,6 +13,7 @@ from pyramid.httpexceptions import HTTPGone, HTTPMovedPermanently, HTTPNotFound from pyramid.response import Response from pyramid.view import forbidden_view_config, view_config +from trove_classifiers import classifiers from warehouse.classifiers.models import Classifier @@ -74,16 +75,8 @@ def forbidden_legacy(exc, request): @view_config(route_name="legacy.api.pypi.list_classifiers") def list_classifiers(request): - classifiers = ( - request.db.query(Classifier.classifier) - .filter(Classifier.deprecated.is_(False)) - .order_by(Classifier.classifier) - .all() - ) - return Response( - text="\n".join(c[0] for c in classifiers), - content_type="text/plain; charset=utf-8", + text="\n".join(sorted(classifiers)), content_type="text/plain; charset=utf-8", ) diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot index 2c2608a88260..dad97eac7dd3 100644 --- a/warehouse/locale/messages.pot +++ b/warehouse/locale/messages.pot @@ -1,4 +1,4 @@ -#: warehouse/views.py:255 +#: warehouse/views.py:254 msgid "Locale updated" msgstr "" diff --git a/warehouse/migrations/versions/d15f020ee3df_simplify_classifier_model.py b/warehouse/migrations/versions/d15f020ee3df_simplify_classifier_model.py new file mode 100644 index 000000000000..58051f21e3c3 --- /dev/null +++ b/warehouse/migrations/versions/d15f020ee3df_simplify_classifier_model.py @@ -0,0 +1,62 @@ +# 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. +""" +Simplify classifier model + +Revision ID: d15f020ee3df +Revises: 6af76ffb9612 +Create Date: 2020-02-03 03:28:22.838779 +""" + +import sqlalchemy as sa + +from alembic import op + +revision = "d15f020ee3df" +down_revision = "6af76ffb9612" + + +def upgrade(): + op.drop_column("trove_classifiers", "l4") + op.drop_column("trove_classifiers", "l5") + op.drop_column("trove_classifiers", "l3") + op.drop_column("trove_classifiers", "deprecated") + op.drop_column("trove_classifiers", "l2") + + +def downgrade(): + op.add_column( + "trove_classifiers", + sa.Column("l2", sa.INTEGER(), autoincrement=False, nullable=True), + ) + op.add_column( + "trove_classifiers", + sa.Column( + "deprecated", + sa.BOOLEAN(), + server_default=sa.text("false"), + autoincrement=False, + nullable=False, + ), + ) + op.add_column( + "trove_classifiers", + sa.Column("l3", sa.INTEGER(), autoincrement=False, nullable=True), + ) + op.add_column( + "trove_classifiers", + sa.Column("l5", sa.INTEGER(), autoincrement=False, nullable=True), + ) + op.add_column( + "trove_classifiers", + sa.Column("l4", sa.INTEGER(), autoincrement=False, nullable=True), + ) diff --git a/warehouse/templates/pages/classifiers.html b/warehouse/templates/pages/classifiers.html index f762a1c5699f..07b37fda92c1 100644 --- a/warehouse/templates/pages/classifiers.html +++ b/warehouse/templates/pages/classifiers.html @@ -32,9 +32,9 @@

    {% trans %}List of classifiers{% endtrans %}