Skip to content
This repository was archived by the owner on Jan 23, 2024. It is now read-only.
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions example/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
},
)

db = MongoEngine(app)
api = MongoRest(app)
db = MongoEngine()
api = MongoRest()

class UserResource(Resource):
document = documents.User
Expand Down Expand Up @@ -303,6 +303,8 @@ class DictDocView(ResourceView):
resource = DictDocResource
methods = [Fetch, List, Create, Update]

db.init_app(app)
api.init_app(app)

if __name__ == "__main__":
port = int(os.environ.get('PORT', 8000))
Expand Down
81 changes: 73 additions & 8 deletions flask_mongorest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,64 @@
from flask_mongorest.methods import Create, BulkUpdate, List


class _DelayedApp(object):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary newline

'''
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should always use double quotes (as in """ instead of ''') in docstrings

Stores URL rules for later merging with application URL map.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary newline

'''

def __init__(self):
self.url_rules = []

def add_url_rule(self, *args, **kwargs):
self.url_rules.append((args, kwargs))


class MongoRest(object):
def __init__(self, app, **kwargs):
self.app = app

def __init__(self, app=None, **kwargs):
'''
Takes optional Flask application instance. If supplied, `init_app` will be
called to update application url map.

Copy link
Copy Markdown
Member

@wojcikstefan wojcikstefan Jul 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary newline

'''

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary newline

self.url_prefix = kwargs.pop('url_prefix', '')
app.register_blueprint(Blueprint(self.url_prefix, __name__, template_folder='templates'))
self.template_folder = kwargs.pop('template_folder', 'templates')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we pop a template_folder from kwargs here instead of specifying def __init__(self, app=None, template_folder='templates', **kwargs) above?

if app is not None:
self.init_app(app, **kwargs)
else:
self.app = _DelayedApp()

def init_app(self, app):
'''
Provides delayed application instance initialization to support
Flask application factory pattern. For further details on application
factories see:

http://flask.pocoo.org/docs/dev/patterns/appfactories/

and

http://mattupstate.com/python/2013/06/26/how-i-structure-my-flask-applications.html

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary newline

'''
app.register_blueprint(
Blueprint(self.url_prefix,
__name__,
template_folder=self.template_folder))
if hasattr(self, 'app'):
if isinstance(self.app, _DelayedApp):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these two ifs should be combined and the whole block starting with these conditions could use a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have a suggestion for the comment(s)?

for args, kwargs in self.app.url_rules:
app.add_url_rule(*args, **kwargs)
self.app = app
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to always set self.app in __init__ - either to _DelayedApp() or to a real Flask app object? We could then skip some of the ugly hasattr conditions.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be cleaner. I will see what works best.


def register(self, **kwargs):
def decorator(klass):

# Construct a url based on a 'name' kwarg with a fallback to a Mongo document's name
# Construct a url based on a 'name' kwarg with a fallback to a
# Mongo document's name
document_name = klass.resource.document.__name__.lower()
name = kwargs.pop('name', document_name)
url = kwargs.pop('url', '/%s/' % document_name)
Expand All @@ -24,11 +72,28 @@ def decorator(klass):
pk_type = kwargs.pop('pk_type', 'string')
view_func = klass.as_view(name)
if List in klass.methods:
self.app.add_url_rule(url, defaults={'pk': None}, view_func=view_func, methods=[List.method], **kwargs)
self.app.add_url_rule(
url,
defaults={
'pk': None},
view_func=view_func,
methods=[
List.method],
**kwargs)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This formatting looks a bit extreme. Would this work?

self.app.add_url_rule(url, defaults={'pk': None}, view_func=view_func,
                      methods=[List.method], **kwargs)

if Create in klass.methods or BulkUpdate in klass.methods:
self.app.add_url_rule(url, view_func=view_func, methods=[x.method for x in klass.methods if x in (Create, BulkUpdate)], **kwargs)
self.app.add_url_rule('%s<%s:%s>/' % (url, pk_type, 'pk'), view_func=view_func, methods=[x.method for x in klass.methods if x not in (List, BulkUpdate)], **kwargs)
self.app.add_url_rule(
url,
view_func=view_func,
methods=[
x.method for x in klass.methods if x in (
Create,
BulkUpdate)],
**kwargs)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something like this is much more readable:

methods = [x.method for x in klass.methods if x in (Create, BulkUpdate)]
self.app.add_url_rule(url, view_func=view_func, methods=methods, **kwargs)

self.app.add_url_rule(
'%s<%s:%s>/' %
(url, pk_type, 'pk'), view_func=view_func, methods=[
x.method for x in klass.methods if x not in (
List, BulkUpdate)], **kwargs)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto - better formatting could be used

Copy link
Copy Markdown
Author

@frankV frankV Jul 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How's this?

methods = [x.method for x in klass.methods if x not in (List, BulkUpdate)]
self.app.add_url_rule('%s<%s:%s>/' % (url, pk_type, 'pk'),
     view_func=view_func, methods=methods, **kwargs)

return klass

return decorator

2 changes: 2 additions & 0 deletions flask_mongorest/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ def _dispatch_request(self, *args, **kwargs):
return super(ResourceView, self).dispatch_request(*args, **kwargs)
except mongoengine.queryset.DoesNotExist as e:
return {'error': 'Empty query: ' + str(e)}, '404 Not Found'
except mongoengine.ValidationError as e:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this within the scope of this PR?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about this one. I'll see how it got in here.

return {'field-errors': e.errors}, '400 Bad Request'
except ValidationError as e:
return e.message, '400 Bad Request'
except Unauthorized as e:
Expand Down