You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Michajlo Matijkiw (JIRA)" <ji...@apache.org> on 2010/09/28 23:00:33 UTC

[jira] Created: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

CacheEntryUpdater does not properly update cache entry resource
---------------------------------------------------------------

                 Key: HTTPCLIENT-1001
                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
             Project: HttpComponents HttpClient
          Issue Type: Bug
          Components: Cache
            Reporter: Michajlo Matijkiw


CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Issue Comment Edited: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

Posted by "Michajlo Matijkiw (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916183#action_12916183 ] 

Michajlo Matijkiw edited comment on HTTPCLIENT-1001 at 9/29/10 12:10 PM:
-------------------------------------------------------------------------

This patch should be applied against TestCacheEntryUpdater and it exposes the mentioned bug.  I have started to work on a fix but was hoping for some feedback on my approach.

I am looking to move the logic of CacheEntryUpdater#updateCacheEntry into BasicHttpCache (this is the only place where it is used).  The rest of CacheEntryUpdater's methods can remain as helper methods.  I think this will simplify the logic and help prevent any code duplication.  I believe it might make resolving HTTPCLIENT-1003 easier as well.

Any thoughts?

Thanks,
Michajlo

      was (Author: cim_michajlomatijkiw):
    This patch should be applied against TestCacheEntryUpdater and it exposes the mentioned bug.  I have started to work on a fix but was hoping for some feedback on my approach.

I am looking to move the logic of CacheEntryUpdater#updateCacheEntry into BasicHttpCache (this is the only place where it is used).  The rest of CacheEntryUpdater's methods can remain as helper methods.  I think this will simplify the logic and help prevent any code duplication.  I believe it might make respolving HTTPCLIENT-1003 easier as well.

Any thoughts?

Thanks,
Michajlo
  
> CacheEntryUpdater does not properly update cache entry resource
> ---------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1001
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: Cache
>            Reporter: Michajlo Matijkiw
>         Attachments: cache_entry_updater_bug_test.patch
>
>
> CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Resolved: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski resolved HTTPCLIENT-1001.
-------------------------------------------

    Resolution: Fixed

Patch 'cache_update_fix2.patch' checked in. Many thanks, Michajlo

Oleg

> CacheEntryUpdater does not properly update cache entry resource
> ---------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1001
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: Cache
>            Reporter: Michajlo Matijkiw
>             Fix For: 4.1 Alpha3
>
>         Attachments: cache_entry_updater_bug_test.patch, cache_update_fix.patch, cache_update_fix2.patch
>
>
> CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

Posted by "Michajlo Matijkiw (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCLIENT-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12916331#action_12916331 ] 

Michajlo Matijkiw commented on HTTPCLIENT-1001:
-----------------------------------------------

Please disregard the previous patch, I am currently working on another version that should be a little more elegant and make HTTPCLIENT-1003 easier.

Thanks,
Michajlo

> CacheEntryUpdater does not properly update cache entry resource
> ---------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1001
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: Cache
>            Reporter: Michajlo Matijkiw
>         Attachments: cache_entry_updater_bug_test.patch, cache_update_fix.patch
>
>
> CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Updated: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

Posted by "Michajlo Matijkiw (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michajlo Matijkiw updated HTTPCLIENT-1001:
------------------------------------------

    Attachment: cache_update_fix2.patch

This patch provides a slightly cleaner fix. 

 - Only 304 responses go through updateCacheEntry, and I have updated CacheEntryUpdater#updateCacheEntry() to reinforce this
 - In CachingHttpClient#revalidateCacheEntry() non-304 responses go through handleBackendResponse() to ensure cachability
 - Updated unit tests to reflect this change

I also fixed a random typo in CachedResponseSuitabilityChecker.java

Please let me know if I have missed anything.

This patch is submitted with the permission of my employer.

Thanks,
Michajlo

> CacheEntryUpdater does not properly update cache entry resource
> ---------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1001
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: Cache
>            Reporter: Michajlo Matijkiw
>             Fix For: 4.1 Alpha3
>
>         Attachments: cache_entry_updater_bug_test.patch, cache_update_fix.patch, cache_update_fix2.patch
>
>
> CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Updated: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

Posted by "Michajlo Matijkiw (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michajlo Matijkiw updated HTTPCLIENT-1001:
------------------------------------------

    Attachment: cache_update_fix.patch

This patch fixes functionality so that a stale cache entry's resource is only copied into the new entry on 304 responses (it is still valid), and is replaced by all other responses.  The following changes were made:

 - Move CacheEntryUpdater#updateCacheEntry() logic into BasicHttpCache#updateCacheEntry(), if the response is not 304 then call BasicCacheEntry#cacheAndReturnResponse()
 - Deleted CacheEntryUpdater tests "testUpdateCacheEntryReturnsDifferentEntryInstance()" and "testUpdatedEntryHasLatestRequestAndResponseDates()" - these are trivial and difficult to test with the rearranged code
 - Added "Via" header to backendResponse in CachingHttpClient revalidation method
 - Added tests to TestBasicHttpCache to check proper updating of response body

This patch is submitted with the permission of my employer.

Thanks,
Michajlo

> CacheEntryUpdater does not properly update cache entry resource
> ---------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1001
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: Cache
>            Reporter: Michajlo Matijkiw
>         Attachments: cache_entry_updater_bug_test.patch, cache_update_fix.patch
>
>
> CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Updated: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

Posted by "Michajlo Matijkiw (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michajlo Matijkiw updated HTTPCLIENT-1001:
------------------------------------------

    Attachment: cache_entry_updater_bug_test.patch

This patch should be applied against TestCacheEntryUpdater and it exposes the mentioned bug.  I have started to work on a fix but was hoping for some feedback on my approach.

I am looking to move the logic of CacheEntryUpdater#updateCacheEntry into BasicHttpCache (this is the only place where it is used).  The rest of CacheEntryUpdater's methods can remain as helper methods.  I think this will simplify the logic and help prevent any code duplication.  I believe it might make respolving HTTPCLIENT-1003 easier as well.

Any thoughts?

Thanks,
Michajlo

> CacheEntryUpdater does not properly update cache entry resource
> ---------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1001
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: Cache
>            Reporter: Michajlo Matijkiw
>         Attachments: cache_entry_updater_bug_test.patch
>
>
> CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Updated: (HTTPCLIENT-1001) CacheEntryUpdater does not properly update cache entry resource

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCLIENT-1001?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated HTTPCLIENT-1001:
------------------------------------------

    Fix Version/s: 4.1 Alpha3

> CacheEntryUpdater does not properly update cache entry resource
> ---------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1001
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1001
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>          Components: Cache
>            Reporter: Michajlo Matijkiw
>             Fix For: 4.1 Alpha3
>
>         Attachments: cache_entry_updater_bug_test.patch, cache_update_fix.patch
>
>
> CacheEntryUpdater#updateCacheEntry() copies the old cache entry's resource, though I believe it should only do so if the response is a 304.  Otherwise it should take the response from the server to update the entry.  This method gets called when validating a cache entry and the server returns a 200 or 304.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org