Skip to content

Commit

Permalink
refactor: full_validate() => validate()
Browse files Browse the repository at this point in the history
Conform to full-Form-style validation.

Update failing test cases now that the full form is validated, and we
can remove the custom `warehouse.forms.Form` wrapper class.

Signed-off-by: Mike Fiedler <[email protected]>
  • Loading branch information
miketheman committed Nov 1, 2024
1 parent e69c89d commit 67d4fb6
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 21 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))
assert form.full_validate() is None
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
29 changes: 21 additions & 8 deletions warehouse/forklift/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import wtforms
import wtforms.validators

from warehouse import forms
from warehouse.utils.project import PROJECT_NAME_RE

_filetype_extension_mapping = {
Expand All @@ -31,7 +30,7 @@
# Any additional validations (such as duplicate filenames, etc) should
# occur elsewhere so that they can happen after we've authorized the request
# to upload for the given project.
class UploadForm(forms.Form):
class UploadForm(wtforms.Form):
# The name field is duplicated out of the general metadata handling, to be
# part of the upload form as well so that we can use it prior to extracting
# the metadata from the uploaded artifact.
Expand Down Expand Up @@ -87,32 +86,46 @@ class UploadForm(forms.Form):
]
)

def full_validate(self):
def validate(self, _extra_validators=None) -> bool:
"""
Perform validation on combinations of fields.
"""

# Validate all defined fields first.
success = super().validate()
if not success:
return False

# All non source releases *must* have a pyversion
if (
self.filetype.data
and self.filetype.data != "sdist"
and not self.pyversion.data
):
raise wtforms.validators.ValidationError(
assert isinstance(self.pyversion.errors, list)
self.pyversion.errors.append(
"Python version is required for binary distribution uploads."
)
return False

# All source releases *must* have a pyversion of "source"
if self.filetype.data == "sdist":
if not self.pyversion.data:
self.pyversion.data = "source"
elif self.pyversion.data != "source":
raise wtforms.validators.ValidationError(
assert isinstance(self.pyversion.errors, list)
self.pyversion.errors.append(
"Use 'source' as Python version for an sdist."
)
return False

# We *must* have at least one digest to verify against.
if (
not self.md5_digest.data
and not self.sha256_digest.data
and not self.blake2_256_digest.data
):
raise wtforms.validators.ValidationError(
"Include at least one message digest."
)
self.form_errors.append("Include at least one message digest.")
return False

return success

0 comments on commit 67d4fb6

Please sign in to comment.