Enhance url-cache

Enhance url-cache

[Julien Danjou]

Hi,

I've recently tried to use url-cache, but found it was very odd.
Here's a little set of patches I propose to enhance the interface and
how it works. It should not be too disruptive I guess.

Feedbacks welcome.

[PATCH 1/5] url-cache.el (url-cache-expired): Handle any type of cached object

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/url/ChangeLog    |    1 +
 lisp/url/url-cache.el |   27 ++++++++++++---------------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/lisp/url/ChangeLog b/lisp/url/ChangeLog
index 7726f6c..a5e5c26 100644
--- a/lisp/url/ChangeLog
+++ b/lisp/url/ChangeLog
@@ -1,6 +1,7 @@
 2010-09-14  Julien Danjou  <julien@danjou.info>
 
 	* url-cache (url-store-in-cache): Make `buff' argument really optional.
+	(url-cache-expired): Handle any type of cached object.
 
 2010-09-14  Glenn Morris  <rgm@gnu.org>
 
diff --git a/lisp/url/url-cache.el b/lisp/url/url-cache.el
index 3a6f00d..015a1ff 100644
--- a/lisp/url/url-cache.el
+++ b/lisp/url/url-cache.el
@@ -180,21 +180,18 @@ Very fast if you have an `md5' primitive function, suitably fast otherwise."
   (insert-file-contents-literally fnam))
 
 ;;;###autoload
-(defun url-cache-expired (url mod)
-  "Return t if a cached file has expired."
-  (let* ((urlobj (if (vectorp url) url (url-generic-parse-url url)))
-	 (type (url-type urlobj)))
-    (cond
-     (url-standalone-mode
-      (not (file-exists-p (url-cache-create-filename url))))
-     ((string= type "http")
-      t)
-     ((member type '("file" "ftp"))
-      (if (or (equal mod '(0 0)) (not mod))
-	  t
-	(or (> (nth 0 mod) (nth 0 (current-time)))
-	    (> (nth 1 mod) (nth 1 (current-time))))))
-     (t nil))))
+(defun url-cache-expired (url expire-time)
+  "Return t if a cached URL is more than EXPIRE-TIME old."
+    (cond (url-standalone-mode
+           (not (file-exists-p (url-cache-create-filename url))))
+          (t (let ((cache-time (url-is-cached url)))
+               (if cache-time
+                   (time-less-p
+                    (time-add
+                     (url-is-cached url)
+                     (seconds-to-time expire-time))
+                    (current-time))
+                 t)))))
 
 (provide 'url-cache)
 
-- 
1.7.1

[PATCH 2/5] url-cache.el: (url-fetch-from-cache): Add new function

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/url/ChangeLog    |    1 +
 lisp/url/url-cache.el |    7 +++++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/lisp/url/ChangeLog b/lisp/url/ChangeLog
index a5e5c26..d4602aa 100644
--- a/lisp/url/ChangeLog
+++ b/lisp/url/ChangeLog
@@ -2,6 +2,7 @@
 
 	* url-cache (url-store-in-cache): Make `buff' argument really optional.
 	(url-cache-expired): Handle any type of cached object.
+	(url-fetch-from-cache): Add new function.
 
 2010-09-14  Glenn Morris  <rgm@gnu.org>
 
diff --git a/lisp/url/url-cache.el b/lisp/url/url-cache.el
index 015a1ff..6ca4c44 100644
--- a/lisp/url/url-cache.el
+++ b/lisp/url/url-cache.el
@@ -69,6 +69,13 @@ FILE can be created or overwritten."
               (write-region (point-min) (point-max) fname nil 5))))))
 
 ;;;###autoload
+(defun url-fetch-from-cache (url)
+  "Fetch URL from cache and return a buffer with the content."
+  (with-current-buffer (generate-new-buffer " *temp*")
+    (url-cache-extract (url-cache-create-filename url))
+    (current-buffer)))
+
+;;;###autoload
 (defun url-is-cached (url)
   "Return non-nil if the URL is cached."
   (let* ((fname (url-cache-create-filename url))
-- 
1.7.1

[PATCH 3/5] url-cache.el: (url-is-cached): Enhance docstring.

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/url/ChangeLog    |    1 +
 lisp/url/url-cache.el |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lisp/url/ChangeLog b/lisp/url/ChangeLog
index d4602aa..5a35894 100644
--- a/lisp/url/ChangeLog
+++ b/lisp/url/ChangeLog
@@ -3,6 +3,7 @@
 	* url-cache (url-store-in-cache): Make `buff' argument really optional.
 	(url-cache-expired): Handle any type of cached object.
 	(url-fetch-from-cache): Add new function.
+	(url-is-cached): Enhance docstring.
 
 2010-09-14  Glenn Morris  <rgm@gnu.org>
 
diff --git a/lisp/url/url-cache.el b/lisp/url/url-cache.el
index 6ca4c44..3c11a85 100644
--- a/lisp/url/url-cache.el
+++ b/lisp/url/url-cache.el
@@ -77,7 +77,7 @@ FILE can be created or overwritten."
 
 ;;;###autoload
 (defun url-is-cached (url)
-  "Return non-nil if the URL is cached."
+  "Return last modification time if the URL is cached."
   (let* ((fname (url-cache-create-filename url))
 	 (attribs (file-attributes fname)))
     (and fname				; got a filename
-- 
1.7.1

[PATCH 4/5] url-vars.el: Remove useless variable `url-cache-expired'.

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/url/ChangeLog   |    1 +
 lisp/url/url-vars.el |    9 ---------
 2 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/lisp/url/ChangeLog b/lisp/url/ChangeLog
index 5a35894..34169a8 100644
--- a/lisp/url/ChangeLog
+++ b/lisp/url/ChangeLog
@@ -4,6 +4,7 @@
 	(url-cache-expired): Handle any type of cached object.
 	(url-fetch-from-cache): Add new function.
 	(url-is-cached): Enhance docstring.
+	* url-vars.el: Remove useless variable `url-cache-expired'.
 
 2010-09-14  Glenn Morris  <rgm@gnu.org>
 
diff --git a/lisp/url/url-vars.el b/lisp/url/url-vars.el
index 7419247..06d3c2d 100644
--- a/lisp/url/url-vars.el
+++ b/lisp/url/url-vars.el
@@ -83,15 +83,6 @@ If non-nil and not t, the user will be asked for each refresh request."
   :type 'boolean
   :group 'url-cache)
 
-;; Fixme: sanitize this.
-(defcustom url-cache-expired
-  (lambda (t1 t2) (>= (- (car t2) (car t1)) 5))
-  "A function determining if a cached item has expired.
-It takes two times (numbers) as its arguments, and returns non-nil if
-the second time is 'too old' when compared to the first time."
-  :type 'function
-  :group 'url-cache)
-
 (defconst url-bug-address "bug-gnu-emacs@gnu.org"
   "Where to send bug reports.")
 
-- 
1.7.1

[PATCH 5/5] url-cache: add url-cache-expire-time

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 lisp/url/ChangeLog    |    3 ++-
 lisp/url/url-cache.el |    7 ++++---
 lisp/url/url-vars.el  |    5 +++++
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/url/ChangeLog b/lisp/url/ChangeLog
index 34169a8..08d9d58 100644
--- a/lisp/url/ChangeLog
+++ b/lisp/url/ChangeLog
@@ -1,7 +1,8 @@
 2010-09-14  Julien Danjou  <julien@danjou.info>
 
 	* url-cache (url-store-in-cache): Make `buff' argument really optional.
-	(url-cache-expired): Handle any type of cached object.
+	(url-cache-expired): Handle any type of cached object and add a
+	global `url-cache-expire-time' variable.
 	(url-fetch-from-cache): Add new function.
 	(url-is-cached): Enhance docstring.
 	* url-vars.el: Remove useless variable `url-cache-expired'.
diff --git a/lisp/url/url-cache.el b/lisp/url/url-cache.el
index 3c11a85..36fbded 100644
--- a/lisp/url/url-cache.el
+++ b/lisp/url/url-cache.el
@@ -187,8 +187,9 @@ Very fast if you have an `md5' primitive function, suitably fast otherwise."
   (insert-file-contents-literally fnam))
 
 ;;;###autoload
-(defun url-cache-expired (url expire-time)
-  "Return t if a cached URL is more than EXPIRE-TIME old."
+(defun url-cache-expired (url &optional expire-time)
+  "Return t if a cached URL is more than EXPIRE-TIME old.
+If EXPIRE-TIME is not set, `url-cache-expire-time' is used instead."
     (cond (url-standalone-mode
            (not (file-exists-p (url-cache-create-filename url))))
           (t (let ((cache-time (url-is-cached url)))
@@ -196,7 +197,7 @@ Very fast if you have an `md5' primitive function, suitably fast otherwise."
                    (time-less-p
                     (time-add
                      (url-is-cached url)
-                     (seconds-to-time expire-time))
+                     (seconds-to-time (or expire-time url-cache-expire-time))
                     (current-time))
                  t)))))
 
diff --git a/lisp/url/url-vars.el b/lisp/url/url-vars.el
index 06d3c2d..2922843 100644
--- a/lisp/url/url-vars.el
+++ b/lisp/url/url-vars.el
@@ -83,6 +83,11 @@ If non-nil and not t, the user will be asked for each refresh request."
   :type 'boolean
   :group 'url-cache)
 
+(defcustom url-cache-expire-time 3600
+  "Maximum time in seconds to keep the documents cached."
+  :type 'integer
+  :group 'url-cache)
+
 (defconst url-bug-address "bug-gnu-emacs@gnu.org"
   "Where to send bug reports.")
 
-- 
1.7.1

Re: [PATCH 3/5] url-cache.el: (url-is-cached): Enhance docstring.

[Glenn Morris]

Applied something similar in emacs-23.

Re: [PATCH 4/5] url-vars.el: Remove useless variable `url-cache-expired'.

[Glenn Morris]

Applied in trunk.

Re: [PATCH 2/5] url-cache.el: (url-fetch-from-cache): Add new function

[Glenn Morris]

Applied to trunk (without the autoload).

Re: Enhance url-cache

[Julien Danjou]

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

On Wed, Sep 15 2010, Julien Danjou wrote:

> Feedbacks welcome.

I'm still waiting for review/merge of patch 1 and 5.

Maybe Glenn again? :)

-- 
Julien Danjou
// ᐰ <julien@danjou.info>   http://julien.danjou.info

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Enhance url-cache

[Glenn Morris]

Yes, I wll install it. Comments:

It changes the behaviour of url-cache-expired a bit, but since the old
version makes no sense and is not used anywhere, why not.

All new defcustoms should have a :version tag (no need for a new patch).

Can you suggest a brief description of the new option and the function
for doc/misc/url.texi?

[PATCH] Fix URL documentation

[Julien Danjou]

Signed-off-by: Julien Danjou <julien@danjou.info>
---
 doc/misc/url.texi |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/doc/misc/url.texi b/doc/misc/url.texi
index a6bbf0b..dcb8f5c 100644
--- a/doc/misc/url.texi
+++ b/doc/misc/url.texi
@@ -731,14 +731,6 @@ directory to store the cache files.  It defaults to sub-directory
 @file{cache} of @code{url-configuration-directory}.
 @end defopt
 
-@c Fixme: function v. option, but neither used.
-@c @findex url-cache-expired
-@c @defopt url-cache-expired
-@c This is a function to decide whether or not a cache entry has expired.
-@c It takes two times as it parameters and returns non-@code{nil} if the
-@c second time is ``too old'' when compared with the first time.
-@c @end defopt
-
 @defopt url-cache-creation-function
 The cache relies on a scheme for mapping URLs to files in the cache.
 This variable names a function which sets the type of cache to use.
@@ -748,6 +740,11 @@ corresponding cache file.  The two supplied possibilities are
 @code{url-cache-create-filename-human-readable}.
 @end defopt
 
+@defopt url-cache-expire-time
+This variable defines a default time value to use as expire delay when
+checking if an URL has expired with @code{url-cache-expired}.
+@end defopt
+
 @defun url-cache-create-filename-using-md5 url
 Creates a cache file name from @var{url} using MD5 hashing.
 This is creates entries with very few cache collisions and is fast.
@@ -768,6 +765,19 @@ more likely to conflict with other files.
 @end smallexample
 @end defun
 
+@defun url-cache-expired
+This is a function to decide whether or not a cache entry has expired.
+It takes a an URL as it first parameters and a expiration delay in
+second paramater. It returns non-@code{nil} if URL is not cached or is
+cached for more than the give expiration delay. If the delay is not
+used, @var{url-cache-expire-time} is used instead.
+@end defun
+
+@defun url-fetch-from-cache
+This function takes an URL as its first argument and then returns a
+buffer containing the data cached for the given URL.
+@end defun
+
 @c Fixme: never actually used currently?
 @c @defopt url-standalone-mode
 @c @cindex Relying on cache
-- 
1.7.1

Re: [PATCH] Fix URL documentation

[Glenn Morris]

Thanks.