Closes #7: Implement OPTIONS method #29

khorolets · 2016-09-26T13:56:57Z

Create Resource patched in flask_restplus_patched that inherits restplus Resource and define the options method there.
Patch some of decorators to be possible to be impemented with classes. Patch some decorators to get access to permission decorators to perform permission checks in option.

coveralls · 2016-09-26T14:02:46Z

Coverage increased (+0.07%) to 91.21% when pulling 539ae85 on khorolets:implement_options into 7dc29bc on frol:master.

coveralls · 2016-09-26T14:02:46Z

Coverage increased (+0.07%) to 91.21% when pulling 539ae85 on khorolets:implement_options into 7dc29bc on frol:master.

frol · 2016-09-26T14:02:46Z

flask_restplus_patched/namespace.py

            def wrapper(*args, **kwargs):
                kwargs[object_arg_name] = resolver(kwargs)
                return func(*args, **kwargs)
-            return wrapper
+            if isinstance(func, type):


Please, swap the if block with the wrapper function as if it is a class, there is no need in the function at all.

frol · 2016-09-26T14:03:57Z

flask_restplus_patched/namespace.py

@@ -135,6 +139,18 @@ def dump_wrapper(*args, **kwargs):
                return model.dump(response).data
            return dump_wrapper

+        def dump_response_with_model_decorator(func):


I guess, it is a duplicate chunk of code.

frol · 2016-09-26T14:07:43Z

app/extensions/api/namespace.py

@@ -153,6 +157,7 @@ def decorator(func):
            # Avoid circilar dependency
            from app.modules.users import permissions

+


This looks like a redundant extra new line (1 blank line is enough).

frol · 2016-09-26T14:15:07Z

app/extensions/api/namespace.py

@@ -170,6 +175,9 @@ def wrapper(*args, **kwargs):

                protected_func = _permission_decorator(func)

+            if not hasattr(protected_func, '_decorators'):


Please, create a separate function to avoid such XXX code duplication. I'm still in doubt if I advised a right way of doing this (I mean using _decorators attribute on the function itself).

frol · 2016-09-26T14:57:39Z

flask_restplus_patched/namespace.py

@@ -46,6 +46,12 @@ def resolve_object(self, object_arg_name, resolver):
        ...        # user is a User instance here
        """
        def decorator(func):
+            if isinstance(func, type):


Sorry, I have just realized that we need to rename func to func_or_class here since we support both use-cases.

coveralls · 2016-09-26T15:00:48Z

Coverage increased (+0.07%) to 91.21% when pulling 47415a7 on khorolets:implement_options into 7dc29bc on frol:master.

frol · 2016-09-26T15:01:43Z

app/extensions/api/namespace.py

@@ -83,6 +83,8 @@ def decorator(func):
            from app.extensions import oauth2
            from app.modules.users import permissions

+            self.register_decorator_in_function(func, decorator)


Please, use oauth2.require_oauth(*oauth_scopes) as a decorator here (and move it closer to the oauth_protected_func = ... line) since otherwise this will call self.doc and other unnecessary stuff.

self.register_decorator_in_function(func, oauth2.require_oauth(*oauth_scopes))

frol · 2016-09-26T15:05:06Z

app/extensions/api/namespace.py

@@ -170,6 +172,7 @@ def wrapper(*args, **kwargs):

                protected_func = _permission_decorator(func)

+            self.register_decorator_in_function(protected_func, decorator)


In fact, we only need to register _permission_decorator.

coveralls · 2016-09-26T15:08:42Z

Coverage increased (+0.07%) to 91.21% when pulling 011d393 on khorolets:implement_options into 7dc29bc on frol:master.

coveralls · 2016-09-26T15:08:42Z

Coverage increased (+0.07%) to 91.21% when pulling 011d393 on khorolets:implement_options into 7dc29bc on frol:master.

frol · 2016-09-26T15:14:01Z

app/extensions/api/namespace.py

@@ -199,6 +202,15 @@ def wrapper(*args, **kwargs):

        return decorator

+    def register_decorator_in_function(self, func, decorator_to_register):


Let's start the name with underscore since it is for internal use only.

I think since we only register access-restriction-related decorators, we may indicate this in the function name, e.g. _register_access_restriction_decorator.

Also, rename _decorators to _access_restriction_decorators.

coveralls · 2016-09-26T15:25:52Z

Coverage increased (+0.08%) to 91.221% when pulling bbb397a on khorolets:implement_options into 7dc29bc on frol:master.

frol · 2016-09-26T15:35:21Z

flask_restplus_patched/namespace.py

+        def wrapper(cls):
+            if 'OPTIONS' in cls.methods:
+                cls.options = self.response(code=204)(cls.options)
+            doc = kwargs.pop('doc', None)


You don't need to copy this, just nest the decorators:

def route(...): base_wrapper = super().route(self, *args, **kwargs) def wrapper(cls): if 'OPTIONS'... ... return base_wrapper(cls) return wrapper

Oh, thanks!

frol · 2016-09-26T15:36:18Z

app/extensions/api/namespace.py

@@ -83,6 +83,7 @@ def decorator(func):
            from app.extensions import oauth2
            from app.modules.users import permissions

+


No need for this empty line either.

Create Resource patched in flask_restplus_patched that inherits restplus Resource and define the `options` method there. Patch some of decorators to be possible to be impemented with classes. Patch some decorators to get access to permission decorators to perform permission checks in option.

coveralls · 2016-09-26T15:54:05Z

Coverage increased (+0.08%) to 91.221% when pulling fc2722b on khorolets:implement_options into 7dc29bc on frol:master.

frol requested changes Sep 26, 2016

View reviewed changes

khorolets force-pushed the implement_options branch from 539ae85 to 47415a7 Compare September 26, 2016 14:55

frol requested changes Sep 26, 2016

View reviewed changes

khorolets force-pushed the implement_options branch from 47415a7 to 011d393 Compare September 26, 2016 15:03

frol requested changes Sep 26, 2016

View reviewed changes

khorolets force-pushed the implement_options branch from 011d393 to bbb397a Compare September 26, 2016 15:21

frol requested changes Sep 26, 2016

View reviewed changes

frol reviewed Sep 26, 2016

View reviewed changes

khorolets force-pushed the implement_options branch from bbb397a to fc2722b Compare September 26, 2016 15:49

frol approved these changes Sep 26, 2016

View reviewed changes

frol merged commit e17bde5 into frol:master Sep 26, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Closes #7: Implement OPTIONS method #29

Closes #7: Implement OPTIONS method #29

khorolets commented Sep 26, 2016

coveralls commented Sep 26, 2016 •

edited

Loading

coveralls commented Sep 26, 2016

frol Sep 26, 2016

frol Sep 26, 2016

frol Sep 26, 2016

frol Sep 26, 2016

frol Sep 26, 2016

coveralls commented Sep 26, 2016 •

edited

Loading

frol Sep 26, 2016 •

edited

Loading

frol Sep 26, 2016

frol Sep 26, 2016

coveralls commented Sep 26, 2016 •

edited

Loading

coveralls commented Sep 26, 2016 •

edited

Loading

frol Sep 26, 2016

coveralls commented Sep 26, 2016 •

edited

Loading

frol Sep 26, 2016

khorolets Sep 26, 2016

frol Sep 26, 2016

coveralls commented Sep 26, 2016 •

edited

Loading

		@@ -153,6 +157,7 @@ def decorator(func):
		# Avoid circilar dependency
		from app.modules.users import permissions

		@@ -170,6 +175,9 @@ def wrapper(args, *kwargs):

		protected_func = _permission_decorator(func)

		if not hasattr(protected_func, '_decorators'):

		@@ -170,6 +172,7 @@ def wrapper(args, *kwargs):

		protected_func = _permission_decorator(func)

		self.register_decorator_in_function(protected_func, decorator)

		@@ -199,6 +202,15 @@ def wrapper(args, *kwargs):

		return decorator

		def register_decorator_in_function(self, func, decorator_to_register):

		@@ -83,6 +83,7 @@ def decorator(func):
		from app.extensions import oauth2
		from app.modules.users import permissions

Closes #7: Implement OPTIONS method #29

Closes #7: Implement OPTIONS method #29

Conversation

khorolets commented Sep 26, 2016

coveralls commented Sep 26, 2016 • edited Loading

coveralls commented Sep 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 26, 2016 • edited Loading

frol Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 26, 2016 • edited Loading

coveralls commented Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

coveralls commented Sep 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Sep 26, 2016 • edited Loading

coveralls commented Sep 26, 2016 •

edited

Loading

coveralls commented Sep 26, 2016 •

edited

Loading

frol Sep 26, 2016 •

edited

Loading

coveralls commented Sep 26, 2016 •

edited

Loading

coveralls commented Sep 26, 2016 •

edited

Loading

coveralls commented Sep 26, 2016 •

edited

Loading

coveralls commented Sep 26, 2016 •

edited

Loading