Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
-+-
   Reporter:  Maniac |Owner:  adrian
   |Milestone:
 Status:  reopened   |  Version:
  Component:  HTTP   | Keywords:  http middleware
  handling   |  conditional get if-modified-since
 Resolution: |Has patch:  1
   Triage Stage:  Accepted   |  Needs tests:  0
Needs documentation:  0  |
Patch needs improvement:  0  |
-+-

Comment (by lukeplant):

 Ah, you're right - I think I was wrongly assuming that the decorators
 '`condition`' and others were created using `decorator_from_middleware`,
 in which case they would be implicitly testing `ConditionalGetMiddleware`.
 In fact, only '`conditional_page`' is created that way, which isn't tested
 in `tests/regressiontests/conditional_processing/models.py`.  Thanks for
 the explanation and updates.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
---+
   Reporter:  Maniac | Owner: 
 adrian   
 Status:  reopened | Milestone: 
  
  Component:  HTTP handling|   Version: 
  
 Resolution:   |  Keywords: 
 http middleware conditional get if-modified-since
   Triage Stage:  Accepted | Has patch: 
 1
Needs documentation:  0|   Needs tests: 
 0
Patch needs improvement:  0|  
---+
Changes (by aaugustin):

  * needs_better_patch:  1 => 0


Comment:

 Existing tests in `tests/regressiontests/conditional_processing/models.py`
 are for the view decorators `condition`, `last_modified` and `etag`. The
 tests I added are for `ConditionnalGetMiddleware`. These are two different
 mechanisms to achieve the same goal, the former per-view, the latter
 globally. Obviously tests are similar, but they test different modules.

 Currently there are no tests for `ConditionnalGetMiddleware`. It seems to
 me that `tests/regressiontests/middleware/tests.py` is the proper place to
 test middleware, so I put the tests there.

 I have updated the patch to cope with the refactoring of
 `django.views.static` and `django.contrib.staticfiles`.
 I'm now using `django.utils.unittest.TestCase` when `django.test.TestCase`
 is not needed.

 New patch attached. Thanks for your review, lukeplant!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
---+
   Reporter:  Maniac | Owner: 
 adrian   
 Status:  reopened | Milestone: 
  
  Component:  HTTP handling|   Version: 
  
 Resolution:   |  Keywords: 
 http middleware conditional get if-modified-since
   Triage Stage:  Accepted | Has patch: 
 1
Needs documentation:  0|   Needs tests: 
 0
Patch needs improvement:  1|  
---+
Changes (by lukeplant):

  * needs_better_patch:  0 => 1


Comment:

 This looks like a very good patch in general, but with some problems:

 * Why have tests for `ConditionalGetMiddleware` been added to
 tests/regressiontests/middleware/tests.py, instead of to
 tests/regressiontests/conditional_processing/models.py with the others?
 And why is there such overlap between the new tests (e.g. in 'ETag'
 processing) and the existing ones? This duplication is confusing and
 unhelpful.
 * The `HttpDateProcessing` class should inherit from `unittest.TestCase`
 (specifically, `django.utils.unittest.TestCase`), and not
 `django.test.TestCase`.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
---+
   Reporter:  Maniac | Owner: 
 adrian   
 Status:  reopened | Milestone: 
  
  Component:  HTTP handling|   Version: 
  
 Resolution:   |  Keywords: 
 http middleware conditional get if-modified-since
   Triage Stage:  Accepted | Has patch: 
 1
Needs documentation:  0|   Needs tests: 
 0
Patch needs improvement:  0|  
---+
Changes (by aaugustin):

 * cc: aaugustin (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
---+
   Reporter:  Maniac | Owner: 
 adrian   
 Status:  reopened | Milestone: 
  
  Component:  HTTP handling|   Version: 
  
 Resolution:   |  Keywords: 
 http middleware conditional get if-modified-since
   Triage Stage:  Accepted | Has patch: 
 1
Needs documentation:  0|   Needs tests: 
 0
Patch needs improvement:  0|  
---+
Changes (by aaugustin):

  * needs_better_patch:  1 => 0
  * needs_tests:  1 => 0


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
---+
   Reporter:  Maniac | Owner: 
 adrian   
 Status:  reopened | Milestone: 
  
  Component:  HTTP handling|   Version: 
  
 Resolution:   |  Keywords: 
 http middleware conditional get if-modified-since
   Triage Stage:  Accepted | Has patch: 
 1
Needs documentation:  0|   Needs tests: 
 1
Patch needs improvement:  1|  
---+

Comment (by aaugustin):

 New patch:
 - builds upon julianb's last patch, but does much less changes to
 django/views/decorators/http.py,
 - adds a `parse_http_date` function in `django.utils.http` that implements
 a RFC-compliant parsing of HTTP dates, and uses it wherever ad-hoc parsing
 was implemented; that function may be simplified with
 email.Utils.parsedate if RFC compliance is not required;
 - adds tests for !ConditionalGetMiddleware (fairly similar to the tests in
 conditional_processing, but those test the decorators and not the
 middleware),
 - fixes a related test that used UTC instead of GMT.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
--+-
  Reporter:  Maniac   | Owner:  
adrian   
Status:  reopened | Milestone:  
 
 Component:  HTTP handling|   Version:  
 
Resolution:   |  Keywords:  
http middleware conditional get if-modified-since
 Stage:  Accepted | Has_patch:  1   
 
Needs_docs:  0|   Needs_tests:  1   
 
Needs_better_patch:  1|  
--+-
Comment (by julianb):

 I improved the patch for the middleware by using parsedate from
 email.utils which might parse more dates.

 The HTTP decorater also has the same issue and I managed to add some tests
 for it. Tests for the ConditionalGetMiddleware are still needed, as I am
 not really into tests.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
--+-
  Reporter:  Maniac   | Owner:  
adrian   
Status:  reopened | Milestone:  
 
 Component:  HTTP handling|   Version:  
 
Resolution:   |  Keywords:  
http middleware conditional get if-modified-since
 Stage:  Accepted | Has_patch:  1   
 
Needs_docs:  0|   Needs_tests:  1   
 
Needs_better_patch:  1|  
--+-
Changes (by lukeplant):

  * status:  closed => reopened
  * needs_better_patch:  0 => 1
  * resolution:  wontfix =>
  * stage:  Unreviewed => Accepted

Comment:

 Accepted after discussion on mailing list, and a valid use case based on
 headers that Google sends.

 The patch needs tests, including what happens with valid dates before and
 after, and what happens with invalid dates. Please can we also have it
 tested with someone who knows exactly what Google is sending.

 Thanks!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
--+-
  Reporter:  Maniac   | Owner:  
adrian   
Status:  closed   | Milestone:  
 
 Component:  HTTP handling|   Version:  
 
Resolution:  wontfix  |  Keywords:  
http middleware conditional get if-modified-since
 Stage:  Unreviewed   | Has_patch:  1   
 
Needs_docs:  0|   Needs_tests:  1   
 
Needs_better_patch:  0|  
--+-
Changes (by lrekucki):

  * status:  reopened => closed
  * resolution:  => wontfix

Comment:

 Django's policy is that you shouldn't reopen tickets resolved as "won't
 fix" without prior discussion on django-developers mailing list. Please
 post there describing why this ticket should be accepted. It's also a good
 idea to describe problem with current behavior in more detail.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #717: If-Modified-Since checked for exact match

[Django]

#717: If-Modified-Since checked for exact match
--+-
  Reporter:  Maniac   | Owner:  
adrian   
Status:  reopened | Milestone:  
 
 Component:  HTTP handling|   Version:  
 
Resolution:   |  Keywords:  
http middleware conditional get if-modified-since
 Stage:  Unreviewed   | Has_patch:  1   
 
Needs_docs:  0|   Needs_tests:  1   
 
Needs_better_patch:  0|  
--+-
Changes (by julianb):

  * status:  closed => reopened
  * component:  django.contrib.admin => HTTP handling
  * needs_tests:  0 => 1
  * keywords:  => http middleware conditional get if-modified-since
  * has_patch:  0 => 1
  * resolution:  wontfix =>

Comment:

 After 5 years, I propose we tackle this. Currently it's not in line with
 the RFC (http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html).

 I have observed Google to often check with a newer Last-Modified date, and
 Django replying with "Oh, sure it got modified, but long time before what
 you ask, here's the resource anyways: 200 OK"

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.