Flask: URL Converters should have access to app context

Created on 31 Jan 2019  路  9Comments  路  Source: pallets/flask

Expected Behavior

The following code should create a new URL converter that converts a specific field to a model (or None).

class ModelConverter(BaseConverter):

    def to_python(self, value):
        return models.Example.query.filter(models.Example.name == value).first()

    def to_url(self, value):
        if isinstance(value, str):
            return value
        return value.name

Actual Behavior

The converters don't have access to the app context so an error is thrown.

Bad Solution

class OrganizationConverter(BaseConverter):
    app = None
    def to_python(self, value):
        with self.app.app_context():
            return repositories.Organization.query.filter(repositories.Organization.name == value).first()

    def to_url(self, value):
        if isinstance(value, str):
            return value
        return value.name

This solution tries to get around it by creating a new app context. The problem is that the context disappears, killing the database session at the same time. This prevents SQLAlchemy features (such as lazy loading).

Environment

  • Python version: 3.6 and 3.7
  • Flask version: 1.0.2
  • SQLAlchemy version: 1.2.17
  • Flask-SQLAlchemy version: 2.3.2
  • Werkzeug version: 0.14.1

Additional Resources

This issue has also come up on stack overflow

Most helpful comment

I've also ran into this issue with converters, but it's might not be straightforward to change at this point.

URL matching is done as part of creating the request context, not pushing the context:

https://github.com/pallets/flask/blob/0b5b4a66ef99c8b91569dd9b9b34911834689d3f/flask/ctx.py#L313

So we'd need to move it into RequestContext.push, after it's actually pushed so that request is populated:

https://github.com/pallets/flask/blob/0b5b4a66ef99c8b91569dd9b9b34911834689d3f/flask/ctx.py#L379

It's a more internal part of Flask, and I haven't really seen any discussion about overriding or using these parts, but I'd have to understand that more before making the change.

All 9 comments

I've also ran into this issue with converters, but it's might not be straightforward to change at this point.

URL matching is done as part of creating the request context, not pushing the context:

https://github.com/pallets/flask/blob/0b5b4a66ef99c8b91569dd9b9b34911834689d3f/flask/ctx.py#L313

So we'd need to move it into RequestContext.push, after it's actually pushed so that request is populated:

https://github.com/pallets/flask/blob/0b5b4a66ef99c8b91569dd9b9b34911834689d3f/flask/ctx.py#L379

It's a more internal part of Flask, and I haven't really seen any discussion about overriding or using these parts, but I'd have to understand that more before making the change.

I managed to accomplish essentially the same thing with decorator functions (and added it on stack overflow), but it would definitely be powerful to be able to swap url parameters with their models.

Let's start with a PR that makes the change I described, and see if it affects any existing tests. I'm happy to review a PR if someone wants to contribute.

@davidism Hi!
I created a PR for this issue and tested it, should I add you as a reviewer?
sorry, it's my PR here

It doesn't look like you've submitted a PR.

ok sorry, a question before I add the PR.
I added the following test and did not change the code yet:

def test_model_converters(app, client):
    from werkzeug.routing import BaseConverter

    app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
    app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:////tmp/test.db'
    db = SQLAlchemy(app)

    class User(db.Model):
        id = db.Column(db.Integer, primary_key=True)
        username = db.Column(db.String(80), unique=True, nullable=False)
        email = db.Column(db.String(120), unique=True, nullable=False)

    db.create_all()
    admin = User(username='admin', email='[email protected]')
    db.session.add(admin)

    class ModelConverter(BaseConverter):
        def to_python(self, value):
            return User.query.filter(User.username == value).first()

        def to_url(self, value):
            if isinstance(value, str):
                return value
            return value.username

    app.url_map.converters['model'] = ModelConverter

    @app.route('/<model:user_name>')
    def index(user_name):
        return user_name.email

    assert client.get('/admin').data == b'[email protected]'

However, I did not get the out of context exception, what am I missing here?

When using db = SQLAlchemy(app) the object is bound to the app and thus works without a context. Use this instead to avoid it:

db = SQLAlchemy()
db.init_app(app)

@davidism this is the PR #3104

Is this well documented yet? I'm here since I read the 1.1.1 release notes and was looking for some examples of how this feature could be used. The test above by @eladm26 clears things up but it took awhile for me to find it here.

Here's the part from the release notes:

URL routing is performed after the request context is pushed. This allows custom URL converters to access current_app and request. This makes it possible to implement converters such as one that queries the database for a model based on the ID in the URL.

Thanks.

Was this page helpful?
0 / 5 - 0 ratings