Models customization works bad

[nnseva]

Describe the bug

Customized models lead to errors at different stages of migration.

The complex case with the customized Application model and another model referencing the AccessToken model produces different errors while the migration process, depending on the application settings (direct swapping of the AccessToken model is also unavailable, see f.e. #1017).

Without the settings.OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL the makemigrations command fails with the following:

AttributeError: 'Settings' object has no attribute 'OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL'

Having the settings.OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL, the makemigrations command is successful but the following initial migration fails with another error:

ValueError: Related model 'myoauth.oauth2app' cannot be resolved

To Reproduce

1. Install last Django, last django-oauth-toolkit package, and clone the repository https://github.com/nnseva/django-oauth-bad-case
2. Try python manage.py makemigrations myoauth, it should work properly
3. Try python manage.py makemigrations badcase, it produces the first error
4. Try to fix the error using modified settings python manage.py makemigrations --settings djoauth.settings_case2 (use diff to see a difference)
5. Try to process migrations python manage.py migrate --settings djoauth.settings_case2, see the second error
6. You can see the first error again if trying to process migrations with the default settings python manage.py migrate
7. See the workaround, processing migrations for different applications separately:

• python manage.py migrate myoauth --settings djoauth.settings_case2
• python manage.py migrate badcase --settings djoauth.settings_case2

Expected behavior

Either step 3, or step 5 is expected to be errorless.

Version

(py3.8.test.djoauth) seva@SEVA-MOBILE:~/py3.8.test.djoauth/test/djoauth$ pip freeze
asgiref==3.5.2
backports.zoneinfo==0.2.1
certifi==2022.9.24
cffi==1.15.1
charset-normalizer==2.1.1
cryptography==38.0.3
Deprecated==1.2.13
Django==4.1.3
django-oauth-toolkit==2.2.0
idna==3.4
jwcrypto==1.4.2
oauthlib==3.2.2
pycparser==2.21
requests==2.28.1
sqlparse==0.4.3
urllib3==1.26.12
wrapt==1.14.1

The error looks like appears for all versions up to django-oauth-toolkit==2.2.0

I really have met the problem for the first time with django-oauth-toolkit==1.7.1 and Django==2.2.5, but checked against a last available stable Django version, and latest versions of the django-oauth-toolkit package.

• I have tested with the latest published release and it's still a problem.
• I have tested with the master branch and it's still a problem.

Additional context

No any

[SinaZK]

Any Idea how to resolve this issue?

[nnseva]

@SinaZK I've noticed that problematic references interfere with the definition of ForeignKey(oauth2_settings.*) instead of ForeignKey(settings.*) in the package. I am not sure anyway.

From my point of view, it should be investigated separately, how to make customizable foreign key fields to avoid such inconsistencies. Probably, the package's non-initial migrations should also be fixed.

The other way is to make base AccessKey etc. models instantiated and inherit them by the customer if necessary. It avoids such problems but may degrade performance a bit (using database joins).

[nnseva]

I am thinking about one, probably not-backward-compatible solution.

Let us create two Django apps instead of one.

One is a fully-abstract solution, which introduces all the functional code, and abstract models related to each other.

The second one introduces only instantiation classes, without functional code except the one necessary to instantiate.

A programmer using the package, has two options, either:

• include both, abstract application and instantiating application, into the project code and use the package as such
• include only abstract application into the project, and instantiate classes in the project himself, customizing models by necessity

In the documentation, we should explicitly declare the mix of the base instantiating application with the own instantiating application as a bad practice.

A programmer using the package should have the possibility to migrate from the package-provided instantiating application to the one created by himself, using some kind of provided by the package template of the instantiating application. We can use an app_label replacement, or something of such kind, to avoid data migration.

In order to provide maximum backward compatibility, we should separate abstract code in the application, different from the oauth2_provider, f.e. oauth2_abstract. Then the oauth2_provider will be the instantiating application only. The swappable attribute may be left in instantiation classes only for compatibility purposes.

[chaitanya4288]

Is there any fix for this issue?

[nnseva]

@chaitanya4288 No any :\

[charettes]

For anyone running this problem I might have a cheat code to get you unblocked.

As y'all might know swappable models were never truly supported as a core feature for anything but contrib.auth.User and thus a few locations in Django's migration core hardcode AUTH_USER_MODEL.

The one that interests us is the code that raises the ValueError: Related model 'myoauth.oauth2app' cannot be resolved when calling the migrate command mentioned by @nnseva.

If you edit this file in your local Django install to include make_model_tuple(settings.OAUTH2_PROVIDER_ACCESS_TOKEN_MODEL) and other swappable models provided by django-oauth-toolkit it should allow makemigrations to succeed and generate a migration file that doesn't need the local edit to succeed on other installs.

In other words, the hack described above is only really needed to generate the proper migration files.

Hope it helps!

[nnseva]

it looks like a great common solution. Would it be possible to you to make a PR for the Django project? This kind of the patch should probably be appied to all models declared as swappable

[charettes]

This kind of the patch should probably be appied to all models declared as swappable

If Django truly supported third party swappable models yet which AFAIK it never truly did.

If the issue of swappable third-party models is one you care about and can commit to seeing the patch through I suggest you give a shot at it yourself. It's not something I can commit to.