javrasya / django-river

Django workflow library that supports on the fly changes ⛵
BSD 3-Clause "New" or "Revised" License
742 stars 105 forks source link

RiverException: There can be only one state field in a model class #38

Closed psychok7 closed 7 years ago

psychok7 commented 7 years ago

@javrasya When running my own unit tests i started getting this river exception river.utils.exceptions.RiverException: There can be only one state field in a model class, but the thing is i am not using more than 1 statefield in my model classes.

I have status = StateField(editable=False, verbose_name=_('status')) in 2 different Models.

Looking at the your code, i found:

        if id(cls) in classes:
            raise RiverException(ErrorCode.MULTIPLE_STATE_FIELDS, "There can be only one state field in a model class.")

I commented this and my tests started working again, but i am not sure if its a good fix. Why does it think i have more than 1 state field??

Here is the full traceback:

root@5ef5a8a65ed5:/code# python3 manage.py test --no-input --settings=xx.settings.test
Creating test database for alias 'default'...
Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 367, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 359, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/test.py", line 29, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 294, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/test.py", line 72, in handle
    failures = test_runner.run_tests(test_labels)
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 549, in run_tests
    old_config = self.setup_databases()
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 499, in setup_databases
    self.parallel, **kwargs
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 743, in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/base/creation.py", line 70, in create_test_db
    run_syncdb=True,
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 130, in call_command
    return command.execute(*args, **defaults)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/migrate.py", line 204, in handle
    fake_initial=fake_initial,
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/executor.py", line 115, in migrate
    state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/executor.py", line 145, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/executor.py", line 244, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/migration.py", line 119, in apply
    operation.state_forwards(self.app_label, project_state)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/operations/fields.py", line 194, in state_forwards
    state.reload_model(app_label, self.model_name_lower)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/state.py", line 162, in reload_model
    self.apps.render_multiple(states_to_be_rendered)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/state.py", line 277, in render_multiple
    model.render(self)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/state.py", line 559, in render
    body,
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/base.py", line 157, in __new__
    new_class.add_to_class(obj_name, obj)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/base.py", line 316, in add_to_class
    value.contribute_to_class(cls, name)
  File "/code/river/models/fields/state.py", line 95, in contribute_to_class
    raise RiverException(ErrorCode.MULTIPLE_STATE_FIELDS, "There can be only one state field in a model class.")
river.utils.exceptions.RiverException: There can be only one state field in a model class.
psychok7 commented 7 years ago

@javrasya do you think the line i mentioned might just be in the wrong place??

javrasya commented 7 years ago

@psychok7 I couldn't check that yet. Will look at it soon.

javrasya commented 7 years ago

It might somehow contribute your model more than once if you are sure there is only one state field in your model.

Instead of commenting it out, we should check that, is that class is registered before WITH MULTIPLE DIFFERENT FIELDS. It should raise an exception if there are more than one state field with the different name.

Could you debug it, does it hit that line for same fields? I suspect that django sometimes does that which is triggering contribute_to_class method of model fields twice.

psychok7 commented 7 years ago

@javrasya i don't think i understood what line you want me to debug, but i added :

        if id(cls) in classes:
            # import ipdb;ipdb.set_trace()
            raise RiverException(ErrorCode.MULTIPLE_STATE_FIELDS, "There can be only one state field in a model class.")

        print("(cls): ", cls)
        classes.append(id(cls))

I got <class '__fake__.QRequest'> multiple times in the print with different ids(cls) right up until where it raised the error (the id must have been equal at that point).

If you want me to debug something else let me know the exact line. thanks

javrasya commented 7 years ago

@psychok7 it probably hits that line more than once;

if id(cls) in classes:

Can you just look at self.name variable which is actual field name at that line. I wonder that, does it hit that line for the same field more than once.

This line of code tries to guarantee not having multiple state fields in a model. We shouldn't remove it. We should fix it somehow.

If it hit that line for the same field(which you can detect by looking at the field name variable) for more than once, I think I can fix it.

psychok7 commented 7 years ago

@javrasya yes it hits the line multiple times but with None:

        if id(cls) in classes:
            print("self.name: ", self.name)
            print("There can be only one state field in a model class.")
            # raise RiverException(ErrorCode.MULTIPLE_STATE_FIELDS, "There can be only one state field in a model class.")

i get :

Destroying old test database for alias 'default'...
self.name:  None
There can be only one state field in a model class.
self.name:  None
There can be only one state field in a model class.
self.name:  None
There can be only one state field in a model class.
self.name:  None
There can be only one state field in a model class.
javrasya commented 7 years ago

Can you try with name instead of self.name, sorry to bother you.

psychok7 commented 7 years ago

@javrasya no problem, with name i get:

Destroying old test database for alias 'default'...
name:  status
There can be only one state field in a model class.
name:  status
There can be only one state field in a model class.
name:  status
There can be only one state field in a model class.
javrasya commented 7 years ago

@psychok7 OK, this is exactly, what I thought. Django triggers that whole function for more than one for the same field. I will try to fix this.

psychok7 commented 7 years ago

@javrasya ok, let me know when you finish and i will test it again ;)

javrasya commented 7 years ago

@psychok7 can you test it with master branch once again.

psychok7 commented 7 years ago

@javrasya i tested it but i now get another error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/options.py", line 617, in get_field
    return self.fields_map[field_name]
KeyError: 'status'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 367, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 359, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/test.py", line 29, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 294, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/test.py", line 72, in handle
    failures = test_runner.run_tests(test_labels)
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 549, in run_tests
    old_config = self.setup_databases()
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 499, in setup_databases
    self.parallel, **kwargs
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 743, in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/base/creation.py", line 70, in create_test_db
    run_syncdb=True,
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 130, in call_command
    return command.execute(*args, **defaults)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/migrate.py", line 204, in handle
    fake_initial=fake_initial,
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/executor.py", line 115, in migrate
    state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/executor.py", line 145, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/executor.py", line 244, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/migration.py", line 129, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/usr/local/lib/python3.5/dist-packages/django/db/migrations/operations/fields.py", line 147, in database_forwards
    schema_editor.remove_field(from_model, from_model._meta.get_field(self.name))
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/options.py", line 619, in get_field
    raise FieldDoesNotExist('%s has no field named %r' % (self.object_name, field_name))
django.core.exceptions.FieldDoesNotExist: Qualif has no field named 'status'

My model does have the status field.

javrasya commented 7 years ago

When you are getting this error, can you share your test case?

psychok7 commented 7 years ago

@javrasya am not sure. i ran only one test with python3 manage.py test entities.test_api --noinput. The thing is this model doesn't even have django-river in it. It look like something happens at the migration stage idk. Anways here is my testcase with a model without river:

class EntityAPITests(TestCase):
    def setUp(self):
        self.client = APIClient(enforce_csrf_checks=True)
        self.user = UserFactory()
        self.token = Token.objects.get(user=self.user)

    def test_200_separator(self):
        self.client.credentials(HTTP_AUTHORIZATION='Token ' + self.token.key)
        response = self.client.get(reverse('entitystaff-list'))
        self.assertEqual(response.status_code, status.HTTP_200_OK)
        self.assertTrue(response.data)
psychok7 commented 7 years ago

@javrasya if i revert to the previous commit i get the other error msg so it must be something with the change you made in MASTER i think. if i remove the RAISE with the old code, then my tests run just fine.

javrasya commented 7 years ago

@psychok7 I don't face that issue. Can you replace contribute_to_class method with this and let me know does that work;

    def contribute_to_class(self, cls, name, virtual_only=False):
        def is_workflow_completed(workflow_object):
            return ObjectService.is_workflow_completed(workflow_object)

        def proceed(self, user, *args, **kwargs):
            TransitionService.proceed(self, user, *args, **kwargs)

        @property
        def on_initial_state(self):
            from river.services.state import StateService

            return StateService.get_initial_state(ContentType.objects.get_for_model(self)) == self.get_state()

        @property
        def on_final_state(self):
            from river.services.state import StateService

            return self.get_state() in StateService.get_final_states(ContentType.objects.get_for_model(self))

        def get_initial_state(self):
            from river.services.state import StateService

            return StateService.get_initial_state(ContentType.objects.get_for_model(self))

        def get_available_proceedings(self, *args, **kwargs):
            from river.services.proceeding import ProceedingService

            return ProceedingService.get_available_proceedings(self, [self.get_state()], *args, **kwargs)

        @property
        def initial_proceedings(self):
            from river.services.proceeding import ProceedingService

            return self.get_state() in ProceedingService.get_initial_proceedings(ContentType.objects.get_for_model(self))

        @property
        def final_proceedings(self):
            from river.services.proceeding import ProceedingService

            return self.get_state() in ProceedingService.get_final_proceedings(ContentType.objects.get_for_model(self))

        @property
        def next_proceedings(self):
            from river.services.proceeding import ProceedingService

            return self.get_state() in ProceedingService.get_next_proceedings(ContentType.objects.get_for_model(self))

        @property
        def proceeding(self):
            try:
                return self.proceedings.filter(transaction_date__isnull=False).latest('transaction_date')
            except Proceeding.DoesNotExist:
                return None

        def _get_state(self):
            return getattr(self, name)

        def _set_state(self, state):
            setattr(self, name, state)

        field_identifiers = [_get_identifier(c, f) for c, f in class_field_rl.items()]
        cls_identifier = _get_cls_identifier(cls)
        field_identifier = _get_identifier(cls_identifier, name)
        if field_identifier not in field_identifiers:

            if cls_identifier in class_field_rl.keys():
                raise RiverException(ErrorCode.MULTIPLE_STATE_FIELDS,
                                     "There can be only one state field in a model class. Class:%s - Field:%s." % (cls.__name__, name))

            class_field_rl[cls_identifier] = name

        self.model = cls
        self.__add_to_class(cls, "proceedings", GenericRelation('%s.%s' % (Proceeding._meta.app_label, Proceeding._meta.object_name)))
        self.__add_to_class(cls, "proceeding", proceeding)

        self.__add_to_class(cls, "is_workflow_completed", is_workflow_completed)
        self.__add_to_class(cls, "proceed", proceed)

        self.__add_to_class(cls, "on_initial_state", on_initial_state)
        self.__add_to_class(cls, "on_final_state", on_final_state)

        self.__add_to_class(cls, "get_initial_state", get_initial_state)
        self.__add_to_class(cls, "get_available_proceedings", get_available_proceedings)

        self.__add_to_class(cls, "initial_proceedings", initial_proceedings)
        self.__add_to_class(cls, "final_proceedings", final_proceedings)
        self.__add_to_class(cls, "next_proceedings", next_proceedings)

        self.__add_to_class(cls, "get_state", _get_state)
        self.__add_to_class(cls, "set_state", _set_state)

        super(StateField, self).contribute_to_class(cls, name, virtual_only=virtual_only)

        post_save.connect(_post_save, self.model, False, dispatch_uid='%s_%s_riverstatefield_post' % (self.model, name))
psychok7 commented 7 years ago

@javrasya there is now a different error lol:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
psycopg2.ProgrammingError: relation "river_testmodel" does not exist
LINE 1: ...test_field", "river_testmodel"."my_field_id" FROM "river_tes...
                                                             ^

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 367, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/__init__.py", line 359, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/test.py", line 29, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 294, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/management/commands/test.py", line 72, in handle
    failures = test_runner.run_tests(test_labels)
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 549, in run_tests
    old_config = self.setup_databases()
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 499, in setup_databases
    self.parallel, **kwargs
  File "/usr/local/lib/python3.5/dist-packages/django/test/runner.py", line 743, in setup_databases
    serialize=connection.settings_dict.get("TEST", {}).get("SERIALIZE", True),
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/base/creation.py", line 78, in create_test_db
    self.connection._test_serialized_contents = self.serialize_db_to_string()
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/base/creation.py", line 122, in serialize_db_to_string
    serializers.serialize("json", get_objects(), indent=None, stream=out)
  File "/usr/local/lib/python3.5/dist-packages/django/core/serializers/__init__.py", line 129, in serialize
    s.serialize(queryset, **options)
  File "/usr/local/lib/python3.5/dist-packages/django/core/serializers/base.py", line 79, in serialize
    for count, obj in enumerate(queryset, start=1):
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/base/creation.py", line 118, in get_objects
    for obj in queryset.iterator():
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/query.py", line 54, in __iter__
    results = compiler.execute_sql()
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/sql/compiler.py", line 835, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python3.5/dist-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/usr/local/lib/python3.5/dist-packages/django/utils/six.py", line 685, in reraise
    raise value.with_traceback(tb)
  File "/usr/local/lib/python3.5/dist-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: relation "river_testmodel" does not exist
LINE 1: ...test_field", "river_testmodel"."my_field_id" FROM "river_tes...
psychok7 commented 7 years ago

@javrasya any ideas on this "new" error?

psychok7 commented 7 years ago

@javrasya i figured it out. the issue was now on my end because i had the river tests folder inside my project. i removed it and everything now works with the new piece of code you added here. I will create a PR with that code so that you don't have to