Skip to content

Commit

Permalink
refactor: drop custom warehouse.forms.Form (#16998)
Browse files Browse the repository at this point in the history
  • Loading branch information
miketheman authored Nov 1, 2024
1 parent 69e8352 commit 80c5fad
Show file tree
Hide file tree
Showing 30 changed files with 180 additions and 311 deletions.
33 changes: 21 additions & 12 deletions tests/unit/forklift/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,45 +10,54 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import hashlib

import pytest

from webob.multidict import MultiDict
from wtforms.validators import ValidationError

from warehouse.forklift.forms import UploadForm


class TestUploadForm:
_sha256 = hashlib.sha256().hexdigest()
_blake2b = hashlib.blake2b(digest_size=32).hexdigest()

@pytest.mark.parametrize(
"data",
[
# Test for singular supported digests
{"filetype": "sdist", "md5_digest": "bad"},
{"filetype": "bdist_wheel", "pyversion": "3.4", "md5_digest": "bad"},
{"filetype": "sdist", "sha256_digest": "bad"},
{"filetype": "bdist_wheel", "pyversion": "3.4", "sha256_digest": "bad"},
{"filetype": "sdist", "blake2_256_digest": "bad"},
{"filetype": "bdist_wheel", "pyversion": "3.4", "blake2_256_digest": "bad"},
{"filetype": "sdist", "sha256_digest": _sha256},
{"filetype": "bdist_wheel", "pyversion": "3.4", "sha256_digest": _sha256},
{"filetype": "sdist", "blake2_256_digest": _blake2b},
{
"filetype": "bdist_wheel",
"pyversion": "3.4",
"blake2_256_digest": _blake2b,
},
# Tests for multiple digests passing through
{
"filetype": "sdist",
"md5_digest": "bad",
"sha256_digest": "bad",
"blake2_256_digest": "bad",
"sha256_digest": _sha256,
"blake2_256_digest": _blake2b,
},
{
"filetype": "bdist_wheel",
"pyversion": "3.4",
"md5_digest": "bad",
"sha256_digest": "bad",
"blake2_256_digest": "bad",
"sha256_digest": _sha256,
"blake2_256_digest": _blake2b,
},
],
)
def test_full_validate_valid(self, data):
# `name` is required for any submission
data["name"] = "fake-package"
form = UploadForm(MultiDict(data))
form.full_validate()
assert form.validate(), form.errors

@pytest.mark.parametrize(
"data",
Expand All @@ -59,6 +68,6 @@ def test_full_validate_valid(self, data):
],
)
def test_full_validate_invalid(self, data):
data["name"] = "fake-package"
form = UploadForm(MultiDict(data))
with pytest.raises(ValidationError):
form.full_validate()
assert not form.validate()
6 changes: 5 additions & 1 deletion tests/unit/forklift/test_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,10 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version):
"metadata_version": "1.2",
"name": "example",
"version": "1.0",
"filetype": "bdist_wat",
"filetype": "bdist_wheel",
"content": "fake binary content",
},
"Invalid value for pyversion. "
"Error: Python version is required for binary distribution uploads.",
),
(
Expand All @@ -614,6 +616,7 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version):
"filetype": "sdist",
"pyversion": "1.0",
},
"Invalid value for pyversion. "
"Error: Use 'source' as Python version for an sdist.",
),
# digest errors.
Expand All @@ -623,6 +626,7 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version):
"name": "example",
"version": "1.0",
"filetype": "sdist",
"content": "fake binary content",
},
"Error: Include at least one message digest.",
),
Expand Down
14 changes: 6 additions & 8 deletions tests/unit/manage/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -899,14 +899,12 @@ def test_validate_name_with_fewer_than_max_applications(self, db_session):

form.validate__max_apps(pretend.stub())

assert form.errors == {
"__all__": [
(
"You have already submitted the maximum number of "
"Organization requests (3)."
)
]
}
assert form.form_errors == [
(
"You have already submitted the maximum number of "
"Organization requests (3)."
)
]

assert organization_service.get_organization_applications_by_name.calls == []
assert organization_service.find_organizationid.calls == []
Expand Down
114 changes: 2 additions & 112 deletions tests/unit/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
import pytest

from webob.multidict import MultiDict
from wtforms.validators import StopValidation, ValidationError
from wtforms.validators import ValidationError

from warehouse.forms import Form, PasswordStrengthValidator, SetLocaleForm, URIValidator
from warehouse.forms import PasswordStrengthValidator, SetLocaleForm, URIValidator


class TestURIValidator:
Expand Down Expand Up @@ -83,116 +83,6 @@ def test_invalid_password(self, password, expected):
assert str(exc.value) == expected


def _raiser(exc):
raise exc


class TestForm:
def test_empty_form_no_errors(self):
form = Form()
assert form.errors == {}

def test_errors_is_not_cached(self):
# Changed in wtforms==2.3.0 (https://github.com/wtforms/wtforms/pull/568)
form = Form()
assert form.errors == {}
form._form_errors.append("An Error")
assert form.errors == {"__all__": ["An Error"]}

def test_form_level_validation_no_validators(self):
class TestForm(Form):
pass

form = TestForm()

assert form.validate()
assert form.errors == {}

def test_form_level_validation_full_validate(self):
class TestForm(Form):
@pretend.call_recorder
def full_validate(self):
pass

form = TestForm()

assert form.validate()
assert form.errors == {}
assert form.full_validate.calls == [pretend.call(form)]

def test_form_level_validation_full_validate_fails(self):
class TestForm(Form):
@pretend.call_recorder
def full_validate(self):
raise ValueError("A Value Error")

form = TestForm()

assert not form.validate()
assert form.errors == {"__all__": ["A Value Error"]}
assert form.full_validate.calls == [pretend.call(form)]

@pytest.mark.parametrize("validator_funcs", [[], [lambda f: None]])
def test_form_level_validation_meta_works(self, validator_funcs):
validator_funcs = [pretend.call_recorder(v) for v in validator_funcs]

class TestForm(Form):
class Meta:
validators = validator_funcs

form = TestForm()

assert form.validate()
assert form.errors == {}
for v in validator_funcs:
assert v.calls == [pretend.call(form)]

@pytest.mark.parametrize(
("validator_funcs", "errors", "stop_after"),
[
(
[
lambda f: _raiser(ValueError("An Error")),
lambda f: None,
lambda f: _raiser(ValueError("Another Error")),
lambda f: _raiser(StopValidation("Stop!")),
lambda f: _raiser(ValueError("This Won't Show.")),
],
["An Error", "Another Error", "Stop!"],
3,
),
(
[
lambda f: _raiser(ValueError("An Error")),
lambda f: None,
lambda f: _raiser(ValueError("Another Error")),
lambda f: _raiser(StopValidation),
lambda f: _raiser(ValueError("This Won't Show.")),
],
["An Error", "Another Error"],
3,
),
],
)
def test_form_level_validation_meta_fails(
self, validator_funcs, errors, stop_after
):
validator_funcs = [pretend.call_recorder(v) for v in validator_funcs]

class TestForm(Form):
class Meta:
validators = validator_funcs

form = TestForm()

assert not form.validate()
assert form.errors == {"__all__": errors}
for i, v in enumerate(validator_funcs):
assert v.calls == [pretend.call(form)]
if i >= stop_after:
break


class TestSetLocaleForm:
def test_validate(self):
form = SetLocaleForm(MultiDict({"locale_id": "es"}))
Expand Down
14 changes: 7 additions & 7 deletions warehouse/accounts/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ class HoneypotMixin:
confirm_form = wtforms.StringField()


class UsernameSearchForm(forms.Form):
class UsernameSearchForm(wtforms.Form):
username = wtforms.StringField(
validators=[
wtforms.validators.InputRequired(),
Expand All @@ -401,7 +401,7 @@ class RegistrationForm( # type: ignore[misc]
NewEmailMixin,
NewPasswordMixin,
HoneypotMixin,
forms.Form,
wtforms.Form,
):
full_name = wtforms.StringField(
validators=[
Expand Down Expand Up @@ -439,7 +439,7 @@ def validate_g_recaptcha_response(self, field):
raise wtforms.validators.ValidationError("Recaptcha error.")


class LoginForm(PasswordMixin, UsernameMixin, forms.Form):
class LoginForm(PasswordMixin, UsernameMixin, wtforms.Form):
def __init__(self, *args, user_service, breach_service, **kwargs):
super().__init__(*args, **kwargs)
self.user_service = user_service
Expand Down Expand Up @@ -480,7 +480,7 @@ def validate_password(self, field):
)


class _TwoFactorAuthenticationForm(forms.Form):
class _TwoFactorAuthenticationForm(wtforms.Form):
def __init__(self, *args, request, user_id, user_service, **kwargs):
super().__init__(*args, **kwargs)
self.request = request
Expand Down Expand Up @@ -544,7 +544,7 @@ def validate_credential(self, field):
self.validated_credential = validated_credential


class ReAuthenticateForm(PasswordMixin, forms.Form):
class ReAuthenticateForm(PasswordMixin, wtforms.Form):
__params__ = [
"username",
"password",
Expand Down Expand Up @@ -602,7 +602,7 @@ def validate_recovery_code_value(self, field):
)


class RequestPasswordResetForm(forms.Form):
class RequestPasswordResetForm(wtforms.Form):
username_or_email = wtforms.StringField(
validators=[
wtforms.validators.InputRequired(),
Expand Down Expand Up @@ -632,5 +632,5 @@ def validate_username_or_email(self, field):
)


class ResetPasswordForm(NewPasswordMixin, forms.Form):
class ResetPasswordForm(NewPasswordMixin, wtforms.Form):
pass
4 changes: 2 additions & 2 deletions warehouse/admin/templates/admin/banners/edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ <h3 class="card-title">Create banner</h3>
<form class="form-horizontal" method="POST" action="{{ request.current_route_path() }}">
<input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">

{% if form.errors.__all__ %}
{% for error in form.errors.__all__ %}
{% if form.form_errors %}
{% for error in form.form_errors %}
<div class="alert alert-danger"><i class="icon fa fa-ban"></i> {{ error }}</div>
{% endfor %}
{% endif %}
Expand Down
4 changes: 2 additions & 2 deletions warehouse/admin/templates/admin/sponsors/edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ <h3 class="card-title">Create sponsor</h3>
<form class="form-horizontal" method="POST" action="{{ request.current_route_path() }}" enctype="multipart/form-data">
<input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">

{% if form.errors.__all__ %}
{% for error in form.errors.__all__ %}
{% if form.form_errors %}
{% for error in form.form_errors %}
<div class="alert alert-danger"><i class="icon fa fa-ban"></i> {{ error }}</div>
{% endfor %}
{% endif %}
Expand Down
12 changes: 6 additions & 6 deletions warehouse/admin/templates/admin/users/detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,8 @@ <h3 class="card-title">Edit User</h3>
<form class="form-horizontal" method="POST" action="{{ request.current_route_path() }}">
<input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">

{% if form.errors.__all__ %}
{% for error in form.errors.__all__ %}
{% if form.form_errors %}
{% for error in form.form_errors %}
<div class="alert alert-danger"><i class="icon fa fa-ban"></i> {{ error }}</div>
{% endfor %}
{% endif %}
Expand Down Expand Up @@ -509,8 +509,8 @@ <h3 class="card-title">Emails</h3>
<form class="form-horizontal" method="POST" action="{{ request.route_path('admin.user.submit_email', username=user.username) }}">
<input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">

{% if form.errors.__all__ %}
{% for error in form.errors.__all__ %}
{% if form.form_errors %}
{% for error in form.form_errors %}
<div class="alert alert-danger"><i class="icon fa fa-ban"></i> {{ error }}</div>
{% endfor %}
{% endif %}
Expand Down Expand Up @@ -570,8 +570,8 @@ <h3 class="card-title">Add a new email</h3>
<input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">

<div class="card-body">
{% if add_email_form.errors.__all__ %}
{% for error in add_email_form.errors.__all__ %}
{% if add_email_form.form_errors %}
{% for error in add_email_form.form_errors %}
<div class="alert alert-danger"><i class="icon fa fa-ban"></i> {{ error }}</div>
{% endfor %}
{% endif %}
Expand Down
4 changes: 2 additions & 2 deletions warehouse/admin/views/banners.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from warehouse.authnz import Permissions
from warehouse.banners.models import Banner
from warehouse.forms import Form, URIValidator
from warehouse.forms import URIValidator


@view_config(
Expand Down Expand Up @@ -145,7 +145,7 @@ def preview_banner(request):
raise HTTPNotFound


class BannerForm(Form):
class BannerForm(wtforms.Form):
name = wtforms.fields.StringField(
validators=[
wtforms.validators.Length(max=100),
Expand Down
4 changes: 2 additions & 2 deletions warehouse/admin/views/sponsors.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@

from warehouse.admin.interfaces import ISponsorLogoStorage
from warehouse.authnz import Permissions
from warehouse.forms import Form, URIValidator
from warehouse.forms import URIValidator
from warehouse.sponsors.models import Sponsor


class SponsorForm(Form):
class SponsorForm(wtforms.Form):
name = wtforms.fields.StringField(
validators=[
wtforms.validators.Length(max=100),
Expand Down
Loading

0 comments on commit 80c5fad

Please sign in to comment.