New method for reversing django's urls in angular

[jkosir]

PR to discuss the new method.

Basically builds a url with arguments for reverse call, DjangularUrlResolverView parses them, calls the actual view and returns the result.

• Doesn't require loading url patterns to angular app anymore.
• Maintains the same interface for djangoUrl service, completely backwards compatible
• Tests already present, will add documentation shortly.

Some other points to consider:

• Currently the code still contains urls_by_namespace function, which was already deprecated with load_djng_urls approach, I suggest we remove that when moving to the new method.

[jrief]

That looks awesome!

I've not yet fully understood how to inject Django URL's into Angular, without adding a code snippet rendered by a Django template.

[jkosir]

Just have a look at DjangularUrlResolverView, should be pretty straightforward.

It doesn't inject anything actually, just passes the arguments for reverse() function to django.

[jkosir]

Added the documentation, preview build available here.

I think this is ready to be merged now.
Also I suggest the new version is 0.8.0, according to semantic versioning guidelines minor version should be updated for backwards compatible added functionality.

[papasax]

This is a good idea. It's definitely not safe to have all django registered address in the javascript source.

[jkosir]

Small change:
I realized the resolving could could be put directly into the middleware, omitting the need for a dedicated view and urlconf entry. (Django runs middlewares's process_request methods before resolving path to view, so a response can be returned from there)

[papasax]

If in the middleware, how do you control who can access this function? What if i want to serve different url for different users? I like the view approach better. Maybe this could be a choice?

[jkosir]

Which function?

[papasax]

I mean the DjangularUrlResolverView.

[jkosir]

When calling your view, it's decorated with all middlewares (session, csrf protection etc. ).
So for access control it's the same as if you reversed url's "manually" and have user make request to that, you control it in your actual view.

The function by itself doesn't really "do" anything, it just starts the same process as if the request came for already reversed url.

I don't exactly see the point of reversing the url to different views for different users, this is supposed to mimic the reverse() function.

[jrief]

@dasf
Just tested your reverse URL stuff. This is really awesome and shall indeed replace the current functionality.

However, I'm not really convinced that we shall do this in the middleware. How about adding a urlpattern, which shall be included into the main projects urls.py file? I believe you thought about that in the first place, and had good reason not to implement it in the first place.

Another approach would be to add a simple Ajax getter, which converts the Django URL namespace on the server. The new problem we now have, is that Django's resolve function does not work as expected:

from django.core.urlresovlers import resolve

resolve('/djangular/url/?djng_url_name=<javascript-named-url>')

does not return any ResolverMatch object.

Another note on the middleware settings.

I would rename djangular.middlewares to djangular.middleware because every other middleware module is named in singular.

Are you sure, that if you use django.middleware.cache.UpdateCacheMiddleware, djangular.middlewares.DjangularUrlMiddleware still has to be first?

[jkosir]

Yes, initially this was done with a view. The only reason I changed it to middleware is because it's easier to configure, no need for djangular urlconf entry. I don't see any real difference between the approaches, so for now I'm for middleware one.

Not exactly sure what you mean with ajax getter?

Yes, it should be renamed to middleware, didn't think of that, will push a commit to fix it.

And yes, this should always be the first middleware. Whatever UpdateCacheMIddleware does should be done when processing request for the real view.

[jrief]

Because urls.py is where customers expect to have a URL parsed. Otherwise its some kind of magic, and thats bad. Even Django's /admin and /static URLs have to be added to urls.py and not through a magic middleware.

Therefore, if its not for a technical reason, I would prefer to add it to urls.py. This way a customer can even configure the prefix.

[jkosir]

urlconf routes request to the view, while a middleware does some sort of request (or response) processing. The url resolving essentially modifies the request path so it's routed to the correct view, which makes sense to be done before (and without) resolving the /djangular/url path. If done with a view it would go through the urlconf twice.

admin adds new pages and urls, so it makes sense to add a urlconf entry, while staticfiles doesn't require a urlconf entry for some reason (although it should imo).

Adding an option to change prefix makes sense, but I don't think that's a desired functionality in most cases. Also it would require changing the prefix in django-angular.js.

[jrief]

Just added an alternative implementation using urlpatterns. It would work like this:

Add to your urls.py:

urlpatterns = patterns('',
    ....
    url(r'^djangular/url/$', include('djangular.urls')),
)

This is based on your excellent work done for the middleware class.

Of course we'd have to add a configuration option to the Javascript part. Personally I'd like to get rid of the term "djangular". This was a bad choice, how about naming this special URL /angular-reverse?

I really dislike any magic things happening in a middleware class before any URL routing. Zen of Python (PEP 20) states:

• Special cases aren't special enough to break the rules.*

and

• Explicit is better than implicit.*

so lets not break these rules. What do you think?

[jkosir]

Well ok then, let's do it with a view. /angular-reverse sounds good.

I see you added a commit with view approach, but haven't changed the middleware. This will break csrf and some other middlewares in some cases.
5ff839d is the view approach version, so we can base it on that.

[jrief]

This was just for demonstration purpose. If you agree, we shall remove your middleware class and focus on the explicit angular_reverse function. Can you please peer review that, maybe I forgot some edge cases.

[jkosir]

You can't remove the middleware class. It has to break middleware processing and that can't be done in a view. Without middleware you can either have the csrf verifications fail (possibly also some other middleware stuff) or bypass authentication and security checks for every view.

5ff839d has the view + middleware approach, use that if you want to do url resolving in a view. Then all it needs is a djangoUrl service update to add an option to change reverser url.

[jkosir]

Have you decided which implementation we should use?
Since this is in the official docs already it would be best to make 0.8.0 available on PyPi soon.

[jrief]

They currently work both. I'd like to do some more tests. If my router would allow to bypass permission settings, then of course this is a no-go. On the other side, I don't like magic...

But I agree, that it was a bad idea to merge that PR too early. I turned the docs back to tag 0.7.10, so at least we don't confuse anybody.

Sorry for the delays.

[jkosir]

No problem with the delays :)

Well whatever you do, you have to break middleware processing (and that can only be done with a custom middleware class), otherwise process_view() method of every middleware will be called with the wrong view. Shouldn't be a problem in most cases, but the solution has to be generic, even if it's somewhat confusing or "magicky".

[jrief]

At the moment I am using this middleware in my current project. Unfortunately I'm having some trouble with URL reversing in combination with ViewSets from DRF.

Therefore I'd like to make another implementation proposal, which for sure will not have the problems with process_view():

To django-angular, we add a special view which does nothing else than converting the named URL patterns into a real URL and return it to the client. This real URL then is used by our Angular $http.get(), $http.post(), $http.put() etc. requests.
Since this result is idempotent, I'll add long-term caching headers so that it can be cached by the client and/or intermediate HTTP servers/proxies. Hence the additional delay should be rather small.

What do you think about? Do you have any security concern, if everybody can reverse() our named Django URLs?

[jkosir]

What problems? I also used this with a ViewSet, haven't had any problems yet.

That implementation has an extra delay (and an extra request) albeit a small one, which is completely unnecessary. I don't agree with using that.

Don't have any security concerns.

[jrief]

@dasf now, I finally made it.

The main changes to your PR is that I moved the reversing path from /djangular/url/ to /angular/reverse. I also fixed this in your test files and the docs.

I also reorganized reverse-urls.rst a little bit. Please annotate.

Then I added my alternative implementation which uses a HTTP permanent redirect onto the real URL. It is compatible with yours and so one can switch between them seamlessly. I still don't feel comfortable with the middleware approach, but I must admit that presumably it is the best solution.

Please have a try. I'd like to release it as 0.7.11 so that our user base gets two intermediate versions before removing the old resolver.

[jkosir]

Looks good to me. 0.7.11 also makes sense.

[papasax]

Hello,

I'm still testing the new reverse function. I'm using the middleware
version.
I'm doing a api request from my controller this way:

my_function = function(){
    var urlapi=djangoUrl.reverse("my_api_endpoint",{})+'?param1='+$scope.param1;
    $http.get(urlapi).success(function(data){
    }).error(function(data, status, headers, config) {
        swal("Server error");
    });
}

The api is made with django rest framework:

router = routers.SimpleRouter()
router.register(r'endpoint', views.MyViewset, base_name='my_api_endpoint')
url(r'^api/', include(router.urls)),

i'm getting this error on the server:

Internal Server Error: /angular/reverse/
Traceback (most recent call last):
File
"/Users/Exlivris3/.virtualenvs/bibliocratie/lib/python2.7/site-packages/django/core/handlers/base.py",
line 87, in get_response
response = middleware_method(request)
File
"/Users/Exlivris3/.virtualenvs/bibliocratie/lib/python2.7/site-packages/djangular/middleware.py",
line 52, in process_request
url = reverse(url_name, args=url_args, kwargs=url_kwargs,
urlconf=self.urlconf)
File
"/Users/Exlivris3/.virtualenvs/bibliocratie/lib/python2.7/site-packages/django/core/urlresolvers.py",
line 551, in reverse
return iri_to_uri(resolver._reverse_with_prefix(view, prefix, _args,
*_kwargs))
File
"/Users/Exlivris3/.virtualenvs/bibliocratie/lib/python2.7/site-packages/django/core/urlresolvers.py",
line 468, in _reverse_with_prefix
(lookup_view_s, args, kwargs, len(patterns), patterns))
NoReverseMatch: Reverse for 'my_api_endpoint-list?param1=VALUE' with arguments
'()' and keyword arguments '{}' not found. 0 pattern(s) tried: []

This method used to work with 0.7.10. How do i make api request with parameters with the new djangoUrl.reverse function?

Thanks,

[jrief]

@jkosir
Hi Jakob,
I never liked that the angular reverser had to run through all the middlewares when calling an actual view. This added a call for each configured middleware on the stack and this somehow seemed to be too complicate for me.

I therefore looked for a simpler solution. Please have a look at this alternative implementation:
https://github.com/jrief/django-angular/blob/simpler-middleware/djangular/middleware.py

It works perfectly in my environment, but it might have other drawback, I haven't thought about.

@papasax would be great if you also could test this. I even believe that angular-reverse-requests should be a little bit faster.

[jkosir]

This looks much better, nice. I'll give it a try later today.

request.environ['QUERY_STRING'] = ''

This removes the query parameters right? While djng_url... ones can be removed we should probably keep others.

[papasax]

I just tried on my project, and i have a problem with my api call.
Here is the call in JS:
$http({
url: djangoUrl.reverse("api-livres-list"),
method: "GET",
params: {phase:
$scope.phase,a_la_une:'True',sort_by:"date_souscription:desc"}
}).success(function(data){
$scope.livres_a_la_une=data;
}).error(function(data, status, headers, config) {
swal("Erreur de connexion reseau. Le serveur est injoignable.");
});

with in python gives this object <QueryDict: {u'djng_url_name':
[u'api-livres-list'], u'phase': [u'GETMONEY'], u'a_la_une': [u'True'],
u'sort_by': [u'date_souscription:desc']}>

process_request in middleware.py doesn't detect the parameters because it
looks for "djng_url_kwarg_" prefix which is not there and the call to
reverse fails.

2015-05-11 9:53 GMT+02:00 Jakob Košir notifications@github.com:

This looks much better, nice. I'll give it a try later today.

request.environ['QUERY_STRING'] = ''

This removes the query parameters right? While djng_url... ones can be
removed we should probably keep others.

—
Reply to this email directly or view it on GitHub
#144 (comment).

[jrief]

@papasax
this should be fixed now

[papasax]

Yes it's working now. It looks good!

2015-05-11 22:11 GMT+02:00 Jacob Rief notifications@github.com:

@papasax https://github.com/papasax
this should be fixed now

—
Reply to this email directly or view it on GitHub
#144 (comment).

[jrief]

@jkosir
If you don't have any objections, I would merge this.

[jkosir]

Looks good to me. Will add some tests to check if it keeps arguments when I have time, hopefully tomorrow.

[jrief]

OK, "simpler middleware" now is part of master branch, but I'll wait for your tests before releasing the next version. Thanks!

[jkosir]

Updated the tests for new approach. As far as I'm concerned this can be released now.