Skip to content

Commit

Permalink
chore: Clean up release-related constants (#13604)
Browse files Browse the repository at this point in the history
Clean up in prep for fixing a releases endpoint bug:

- `VERSION_LENGTH` -> `MAX_VERSION_LENGTH` for clarity and consistency with other max constants
- new `MAX_COMMIT_LENGTH`
- move `BAD_RELEASE_CHARS` and `COMMIT_RANGE_DELIMITER` to `constants.py`
  • Loading branch information
lobsterkatie authored Jun 10, 2019
1 parent 880d00b commit 13af8f5
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 34 deletions.
14 changes: 7 additions & 7 deletions src/sentry/api/serializers/rest_framework/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,31 @@

from sentry.api.serializers.rest_framework import CommitSerializer, ListField
from sentry.api.fields.user import UserField
from sentry.constants import VERSION_LENGTH
from sentry.constants import MAX_COMMIT_LENGTH, MAX_VERSION_LENGTH
from sentry.models import Release


class ReleaseHeadCommitSerializerDeprecated(serializers.Serializer):
currentId = serializers.CharField(max_length=64)
currentId = serializers.CharField(max_length=MAX_COMMIT_LENGTH)
repository = serializers.CharField(max_length=64)
previousId = serializers.CharField(max_length=64, required=False)
previousId = serializers.CharField(max_length=MAX_COMMIT_LENGTH, required=False)


class ReleaseHeadCommitSerializer(serializers.Serializer):
commit = serializers.CharField(max_length=64)
commit = serializers.CharField(max_length=MAX_COMMIT_LENGTH)
repository = serializers.CharField(max_length=200)
previousCommit = serializers.CharField(max_length=64, required=False)
previousCommit = serializers.CharField(max_length=MAX_COMMIT_LENGTH, required=False)


class ReleaseSerializer(serializers.Serializer):
ref = serializers.CharField(max_length=VERSION_LENGTH, required=False)
ref = serializers.CharField(max_length=MAX_VERSION_LENGTH, required=False)
url = serializers.URLField(required=False)
dateReleased = serializers.DateTimeField(required=False)
commits = ListField(child=CommitSerializer(), required=False, allow_null=False)


class ReleaseWithVersionSerializer(ReleaseSerializer):
version = serializers.CharField(max_length=VERSION_LENGTH, required=True)
version = serializers.CharField(max_length=MAX_VERSION_LENGTH, required=True)
owner = UserField(required=False)

def validate_version(self, attrs, source):
Expand Down
5 changes: 4 additions & 1 deletion src/sentry/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ def get_all_languages():
MODULE_ROOT = os.path.dirname(__import__('sentry').__file__)
DATA_ROOT = os.path.join(MODULE_ROOT, 'data')

VERSION_LENGTH = 200
BAD_RELEASE_CHARS = '\n\f\t/'
MAX_VERSION_LENGTH = 200
MAX_COMMIT_LENGTH = 64
COMMIT_RANGE_DELIMITER = '..'

SORT_OPTIONS = OrderedDict(
(
Expand Down
3 changes: 1 addition & 2 deletions src/sentry/models/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
JSONField, Model, sane_repr
)

from sentry.constants import BAD_RELEASE_CHARS, COMMIT_RANGE_DELIMITER
from sentry.models import CommitFileChange
from sentry.signals import issue_resolved

Expand All @@ -35,9 +36,7 @@

_sha1_re = re.compile(r'^[a-f0-9]{40}$')
_dotted_path_prefix_re = re.compile(r'^([a-zA-Z][a-zA-Z0-9-]+)(\.[a-zA-Z][a-zA-Z0-9-]+)+-')
BAD_RELEASE_CHARS = '\n\f\t/'
DB_VERSION_LENGTH = 250
COMMIT_RANGE_DELIMITER = '..'


class ReleaseProject(Model):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from datetime import datetime
from django.core.urlresolvers import reverse

from sentry.constants import VERSION_LENGTH
from sentry.constants import MAX_VERSION_LENGTH
from sentry.models import (
Activity, Environment, File, Release, ReleaseCommit, ReleaseFile, ReleaseProject, ReleaseProjectEnvironment, Repository
)
Expand Down Expand Up @@ -760,10 +760,10 @@ def test_do_not_allow_null_refs(self):

def test_ref_limited_by_max_version_length(self):
serializer = OrganizationReleaseSerializer(data={
'ref': 'a' * VERSION_LENGTH,
'ref': 'a' * MAX_VERSION_LENGTH,
})
assert serializer.is_valid()
serializer = OrganizationReleaseSerializer(data={
'ref': 'a' * (VERSION_LENGTH + 1),
'ref': 'a' * (MAX_VERSION_LENGTH + 1),
})
assert not serializer.is_valid()
11 changes: 5 additions & 6 deletions tests/sentry/api/endpoints/test_organization_releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@
from exam import fixture

from sentry.api.endpoints.organization_releases import ReleaseSerializerWithProjects
from sentry.constants import VERSION_LENGTH
from sentry.constants import BAD_RELEASE_CHARS, MAX_VERSION_LENGTH
from sentry.models import (
Activity,
ApiKey,
ApiToken,
BAD_RELEASE_CHARS,
Commit,
CommitAuthor,
CommitFileChange,
Expand Down Expand Up @@ -1447,24 +1446,24 @@ def test_ref_limited_by_max_version_length(self):
serializer = ReleaseSerializerWithProjects(data={
'version': self.version,
'projects': self.projects,
'ref': 'a' * VERSION_LENGTH,
'ref': 'a' * MAX_VERSION_LENGTH,
})
assert serializer.is_valid()
serializer = ReleaseSerializerWithProjects(data={
'version': self.version,
'projects': self.projects,
'ref': 'a' * (VERSION_LENGTH + 1),
'ref': 'a' * (MAX_VERSION_LENGTH + 1),
})
assert not serializer.is_valid()

def test_version_limited_by_max_version_length(self):
serializer = ReleaseSerializerWithProjects(data={
'version': 'a' * VERSION_LENGTH,
'version': 'a' * MAX_VERSION_LENGTH,
'projects': self.projects,
})
assert serializer.is_valid()
serializer = ReleaseSerializerWithProjects(data={
'version': 'a' * (VERSION_LENGTH + 1),
'version': 'a' * (MAX_VERSION_LENGTH + 1),
'projects': self.projects,
})
assert not serializer.is_valid()
Expand Down
6 changes: 3 additions & 3 deletions tests/sentry/api/endpoints/test_project_release_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from django.core.urlresolvers import reverse

from sentry.api.endpoints.project_release_details import ReleaseSerializer
from sentry.constants import VERSION_LENGTH
from sentry.constants import MAX_VERSION_LENGTH
from sentry.models import (Activity, File, Release, ReleaseCommit, ReleaseFile, ReleaseProject)
from sentry.testutils import APITestCase

Expand Down Expand Up @@ -295,10 +295,10 @@ def test_do_not_allow_null_commits(self):

def test_ref_limited_by_max_version_length(self):
serializer = ReleaseSerializer(data={
'ref': 'a' * VERSION_LENGTH,
'ref': 'a' * MAX_VERSION_LENGTH,
})
assert serializer.is_valid()
serializer = ReleaseSerializer(data={
'ref': 'a' * (VERSION_LENGTH + 1),
'ref': 'a' * (MAX_VERSION_LENGTH + 1),
})
assert not serializer.is_valid()
11 changes: 5 additions & 6 deletions tests/sentry/api/endpoints/test_project_releases.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
from exam import fixture

from sentry.api.endpoints.project_releases import ReleaseWithVersionSerializer
from sentry.constants import VERSION_LENGTH
from sentry.constants import BAD_RELEASE_CHARS, MAX_VERSION_LENGTH
from sentry.models import (
BAD_RELEASE_CHARS,
CommitAuthor,
CommitFileChange,
Environment,
Expand Down Expand Up @@ -743,22 +742,22 @@ def test_do_not_allow_null_commits(self):
def test_ref_limited_by_max_version_length(self):
serializer = ReleaseWithVersionSerializer(data={
'version': self.version,
'ref': 'a' * VERSION_LENGTH,
'ref': 'a' * MAX_VERSION_LENGTH,
})
assert serializer.is_valid()
serializer = ReleaseWithVersionSerializer(data={
'version': self.version,
'ref': 'a' * (VERSION_LENGTH + 1),
'ref': 'a' * (MAX_VERSION_LENGTH + 1),
})
assert not serializer.is_valid()

def test_version_limited_by_max_version_length(self):
serializer = ReleaseWithVersionSerializer(data={
'version': 'a' * VERSION_LENGTH,
'version': 'a' * MAX_VERSION_LENGTH,
})
assert serializer.is_valid()
serializer = ReleaseWithVersionSerializer(data={
'version': 'a' * (VERSION_LENGTH + 1),
'version': 'a' * (MAX_VERSION_LENGTH + 1),
})
assert not serializer.is_valid()

Expand Down
4 changes: 2 additions & 2 deletions tests/sentry/event_manager/test_event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from time import time

from sentry.app import tsdb
from sentry.constants import VERSION_LENGTH
from sentry.constants import MAX_VERSION_LENGTH
from sentry.event_manager import HashDiscarded, EventManager, EventUser
from sentry.grouping.utils import hash_from_values
from sentry.models import (
Expand Down Expand Up @@ -688,7 +688,7 @@ def test_release_project_slug(self):

def test_release_project_slug_long(self):
project = self.create_project(name='foo')
partial_version_len = VERSION_LENGTH - 4
partial_version_len = MAX_VERSION_LENGTH - 4
release = Release.objects.create(
version='foo-%s' % ('a' * partial_version_len, ), organization=project.organization
)
Expand Down
8 changes: 4 additions & 4 deletions tests/sentry/event_manager/test_validate_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from datetime import datetime, timedelta

from sentry.constants import VERSION_LENGTH, MAX_CULPRIT_LENGTH
from sentry.constants import MAX_VERSION_LENGTH, MAX_CULPRIT_LENGTH
from sentry.event_manager import EventManager


Expand Down Expand Up @@ -174,7 +174,7 @@ def test_extra_as_string():

def test_release_tag_max_len():
release_key = u"sentry:release"
release_value = "a" * VERSION_LENGTH
release_value = "a" * MAX_VERSION_LENGTH
data = validate_and_normalize(
{"message": "foo", "tags": [[release_key, release_value]]}
)
Expand All @@ -197,8 +197,8 @@ def test_site_too_long():


def test_release_too_long():
data = validate_and_normalize({"release": "a" * (VERSION_LENGTH + 1)})
assert len(data.get("release")) == VERSION_LENGTH
data = validate_and_normalize({"release": "a" * (MAX_VERSION_LENGTH + 1)})
assert len(data.get("release")) == MAX_VERSION_LENGTH


def test_release_as_non_string():
Expand Down

0 comments on commit 13af8f5

Please sign in to comment.