Skip to content

Commit

Permalink
[#128] Make object id in the hook models charfield to avoid type viol…
Browse files Browse the repository at this point in the history
…ation (#130)
  • Loading branch information
javrasya authored Jan 28, 2020
1 parent c191f0e commit e681bd2
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 5 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ dist
.cache
.pytest_cache
.python-version
build
build
venv*
96 changes: 96 additions & 0 deletions river/migrations/0014_auto_20200128_0558.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
from django.db import migrations, models
from django.db.models import F, PositiveIntegerField
from django.db.models.functions import Cast


def backup_values(apps, schema_editor):
def backup_hook(hook_cls):
hook_cls.objects.update(object_id2=F("object_id"))
hook_cls.objects.update(object_id=None)

backup_hook(apps.get_model("river", "OnApprovedHook"))
backup_hook(apps.get_model("river", "OnCompleteHook"))
backup_hook(apps.get_model("river", "OnTransitHook"))


def revert_backup_values(apps, schema_editor):
def revert_backup_hook(hook_cls):
hook_cls.objects.update(object_id=Cast(F("object_id2"), output_field=PositiveIntegerField()))
hook_cls.objects.update(object_id2=None)

revert_backup_hook(apps.get_model("river", "OnApprovedHook"))
revert_backup_hook(apps.get_model("river", "OnCompleteHook"))
revert_backup_hook(apps.get_model("river", "OnTransitHook"))


def restore_values(apps, schema_editor):
def restore_hook(hook_cls):
hook_cls.objects.update(object_id=F("object_id2"))

restore_hook(apps.get_model("river", "OnApprovedHook"))
restore_hook(apps.get_model("river", "OnCompleteHook"))
restore_hook(apps.get_model("river", "OnTransitHook"))


def revert_restore_values(apps, schema_editor):
def revert_restore_hook(hook_cls):
hook_cls.objects.update(object_id2=F("object_id"))

revert_restore_hook(apps.get_model("river", "OnApprovedHook"))
revert_restore_hook(apps.get_model("river", "OnCompleteHook"))
revert_restore_hook(apps.get_model("river", "OnTransitHook"))


class Migration(migrations.Migration):
dependencies = [
('river', '0013_auto_20191214_0742'),
]

operations = [
migrations.AddField(
model_name='onapprovedhook',
name='object_id2',
field=models.CharField(blank=True, max_length=200, null=True),
),
migrations.AddField(
model_name='oncompletehook',
name='object_id2',
field=models.CharField(blank=True, max_length=200, null=True),
),
migrations.AddField(
model_name='ontransithook',
name='object_id2',
field=models.CharField(blank=True, max_length=200, null=True),
),

migrations.RunPython(backup_values, reverse_code=revert_backup_values),
migrations.AlterField(
model_name='onapprovedhook',
name='object_id',
field=models.CharField(blank=True, max_length=200, null=True),
),
migrations.AlterField(
model_name='oncompletehook',
name='object_id',
field=models.CharField(blank=True, max_length=200, null=True),
),
migrations.AlterField(
model_name='ontransithook',
name='object_id',
field=models.CharField(blank=True, max_length=200, null=True),
),
migrations.RunPython(restore_values, reverse_code=revert_restore_values),

migrations.RemoveField(
model_name='onapprovedhook',
name='object_id2',
),
migrations.RemoveField(
model_name='oncompletehook',
name='object_id2',
),
migrations.RemoveField(
model_name='ontransithook',
name='object_id2',
),
]
2 changes: 1 addition & 1 deletion river/models/hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class Meta:
workflow = models.ForeignKey(Workflow, verbose_name=_("Workflow"), related_name='%(app_label)s_%(class)s_hooks', on_delete=PROTECT)

content_type = models.ForeignKey(ContentType, blank=True, null=True, on_delete=models.SET_NULL)
object_id = models.PositiveIntegerField(blank=True, null=True)
object_id = models.CharField(max_length=200, blank=True, null=True)
workflow_object = GenericForeignKey('content_type', 'object_id')

hook_type = models.CharField(_('When?'), choices=HOOK_TYPES, max_length=50)
Expand Down
114 changes: 111 additions & 3 deletions river/tests/tmigrations/test__migrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import sys
from datetime import datetime, timedelta
from unittest import skipUnless
from uuid import uuid4

import django
import six
Expand All @@ -10,8 +11,10 @@
from django.test.utils import override_settings
from hamcrest import assert_that, equal_to, has_length, has_item, is_not, less_than

from river.models import TransitionApproval
from river.models.factories import StateObjectFactory, WorkflowFactory, TransitionApprovalMetaFactory, PermissionObjectFactory, UserObjectFactory, TransitionMetaFactory
from river.models import TransitionApproval, OnApprovedHook, Function, OnCompleteHook, OnTransitHook
from river.models.factories import StateObjectFactory, WorkflowFactory, TransitionApprovalMetaFactory, PermissionObjectFactory, UserObjectFactory, \
TransitionMetaFactory
from river.models.hook import BEFORE
from river.tests.models import BasicTestModel
from river.tests.models.factories import BasicTestModelObjectFactory

Expand All @@ -22,7 +25,6 @@

from django.core.management import call_command
from django.test import TestCase
from django.conf import settings

_author_ = 'ahmetdal'

Expand Down Expand Up @@ -672,3 +674,109 @@ def test__shouldCreateTransitionsAndTransitionMetasOutOfApprovalMetaAndApprovals
assert_that(result, has_item(equal_to((meta_2.transition_approvals.first().pk, transition_2.transitions.first().pk))))
assert_that(result, has_item(equal_to((meta_3.transition_approvals.first().pk, transition_2.transitions.first().pk))))
assert_that(result, has_item(equal_to((meta_4.transition_approvals.first().pk, transition_3.transitions.first().pk))))

@skipUnless(*MIGRATION_TEST_ENABLED)
def test__shouldMigrateObjectIdInHooksByCastingItToString(self):
out = StringIO()
sys.stout = out

state1 = StateObjectFactory(label="state1")
state2 = StateObjectFactory(label="state2")

workflow = WorkflowFactory(initial_state=state1, content_type=ContentType.objects.get_for_model(BasicTestModel), field_name="my_field")
transition_1 = TransitionMetaFactory.create(workflow=workflow, source_state=state1, destination_state=state2)

transition_approval_meta_1 = TransitionApprovalMetaFactory.create(
workflow=workflow,
transition_meta=transition_1,
priority=0
)

workflow_object = BasicTestModelObjectFactory()

callback_method = """
from river.tests.hooking.base_hooking_test import callback_output
def handle(context):
print(context)
key = '%s'
callback_output[key] = callback_output.get(key,[]) + [context]
"""

callback_function = Function.objects.create(name=uuid4(), body=callback_method % str(uuid4()))

OnApprovedHook.objects.create(
workflow=workflow,
callback_function=callback_function,
transition_approval_meta=transition_approval_meta_1,
hook_type=BEFORE,
workflow_object=workflow_object.model
)

OnCompleteHook.objects.create(
workflow=workflow,
callback_function=callback_function,
hook_type=BEFORE,
workflow_object=workflow_object.model
)

OnTransitHook.objects.create(
workflow=workflow,
callback_function=callback_function,
transition_meta=transition_1,
hook_type=BEFORE,
workflow_object=workflow_object.model
)

call_command('migrate', 'river', '0013', stdout=out)

with connection.cursor() as cur:
schema = cur.execute("PRAGMA table_info('river_onapprovedhook');").fetchall()
column_type = next(iter([column[2] for column in schema if column[1] == 'object_id']), None)
assert_that(column_type, equal_to("integer unsigned"))

schema = cur.execute("PRAGMA table_info('river_ontransithook');").fetchall()
column_type = next(iter([column[2] for column in schema if column[1] == 'object_id']), None)
assert_that(column_type, equal_to("integer unsigned"))

schema = cur.execute("PRAGMA table_info('river_oncompletehook');").fetchall()
column_type = next(iter([column[2] for column in schema if column[1] == 'object_id']), None)
assert_that(column_type, equal_to("integer unsigned"))

result = cur.execute("select object_id from river_onapprovedhook where object_id='%s';" % workflow_object.model.pk).fetchall()
assert_that(result, has_length(1))
assert_that(result[0][0], equal_to(workflow_object.model.pk))

result = cur.execute("select object_id from river_ontransithook where object_id='%s';" % workflow_object.model.pk).fetchall()
assert_that(result, has_length(1))
assert_that(result[0][0], equal_to(workflow_object.model.pk))

result = cur.execute("select object_id from river_oncompletehook where object_id='%s';" % workflow_object.model.pk).fetchall()
assert_that(result, has_length(1))
assert_that(result[0][0], equal_to(workflow_object.model.pk))

call_command('migrate', 'river', '0014', stdout=out)

with connection.cursor() as cur:
schema = cur.execute("PRAGMA table_info('river_onapprovedhook');").fetchall()
column_type = next(iter([column[2] for column in schema if column[1] == 'object_id']), None)
assert_that(column_type, equal_to("varchar(200)"))

schema = cur.execute("PRAGMA table_info('river_ontransithook');").fetchall()
column_type = next(iter([column[2] for column in schema if column[1] == 'object_id']), None)
assert_that(column_type, equal_to("varchar(200)"))

schema = cur.execute("PRAGMA table_info('river_oncompletehook');").fetchall()
column_type = next(iter([column[2] for column in schema if column[1] == 'object_id']), None)
assert_that(column_type, equal_to("varchar(200)"))

result = cur.execute("select object_id from river_onapprovedhook where object_id='%s';" % workflow_object.model.pk).fetchall()
assert_that(result, has_length(1))
assert_that(result[0][0], equal_to(str(workflow_object.model.pk)))

result = cur.execute("select object_id from river_ontransithook where object_id='%s';" % workflow_object.model.pk).fetchall()
assert_that(result, has_length(1))
assert_that(result[0][0], equal_to(str(workflow_object.model.pk)))

result = cur.execute("select object_id from river_oncompletehook where object_id='%s';" % workflow_object.model.pk).fetchall()
assert_that(result, has_length(1))
assert_that(result[0][0], equal_to(str(workflow_object.model.pk)))

0 comments on commit e681bd2

Please sign in to comment.