cocoon-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gkossakow...@apache.org
Subject svn commit: r531020 - in /cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main: java/org/apache/cocoon/servletservice/components/ resources/META-INF/cocoon/spring/
Date Sat, 21 Apr 2007 12:49:16 GMT
Author: gkossakowski
Date: Sat Apr 21 05:49:15 2007
New Revision: 531020

URL: http://svn.apache.org/viewvc?view=rev&rev=531020
Log:
Completely redesigned checking of source validity. Previous approach didn't work in all cases:
http://thread.gmane.org/gmane.text.xml.cocoon.cvs/24099/focus=72801
In new approach, ServletSource stores Last-Modified values for its own use in store. This
way we can always get all needed information to perform conditional get and return appropriate
value in getValidity() method.
Maintaining own store is not the most elegant solution but it works and is quite reliable
(see remark below). If someone comes with better one, I'll be happy to help implement it.
Remark: Keys for store are calculated using value returned by getURI() method which *must*
be unique. However, this is not the case currently and has to be fixed ASAP.

Modified:
    cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java
    cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSourceFactory.java
    cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/resources/META-INF/cocoon/spring/cocoon-servlet-service-servlet-source-factory.xml

Modified: cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java?view=diff&rev=531020&r1=531019&r2=531020
==============================================================================
--- cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java
(original)
+++ cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSource.java
Sat Apr 21 05:49:15 2007
@@ -32,6 +32,7 @@
 import org.apache.excalibur.source.SourceException;
 import org.apache.excalibur.source.SourceValidity;
 import org.apache.excalibur.source.impl.AbstractSource;
+import org.apache.excalibur.store.Store;
 
 /**
  * Implementation of a {@link Source} that gets its content by
@@ -44,29 +45,25 @@
 	private transient Log logger = LogFactory.getLog(getClass());
     
     private ServletConnection servletConnection;
+    
     /**
-     * <p>This validity must be lazy-initialized as there can occur following situations:</p>
-     * <ol>
-     * <li>Source is asked for the validity for the first time so the validity won't
be asked if it's valid and 
-     * <code>getInputStream()</code> will be called to get response body. While
making request there is no information that could 
-     * be used to set <code>ifModifiedSince</code> property but <i>after</i>
fetching response there could be information about 
-     * last modification date that has to be set to the <code>ServletValidity</code>
instance.</li>
-     * <li>It's second (or next) time that Source is asked for the validity object
so caller has old validity object. Old validity 
-     * object will be asked if it's still valid. New validity object has reference to the
{@link ServletConnection} so 
-     * <code>ifModifiedSince</code> property can be set by value pulled from
<b>old</b> validity object.</li>
-     * </ol>
-     * 
-     * <p>This two different situations demand lazy-initialization.</p>
+     * The store is used to store values of Last-Modified header (if it exists). This store
is required because in {@link #getValidity()}
+     * we need value of Last-Modified header of previous response in order to perform conditional
GET.
+     * @see Broken caching of servlet: source in some cases thread 
+     * 		(http://news.gmane.org/find-root.php?group=gmane.text.xml.cocoon.devel&article=72801)

      */
-    private final ServletValidity validity;
+    private Store store;
+    
+    private boolean connected;
     
-    public ServletSource(String location) throws IOException {
+    public ServletSource(String location, Store store) throws IOException {
         // the systemId (returned by getURI()) is by default null
         // using the block uri is a little bit questionable as it only is valid
         // whithin the current block, not globally
+    	this.store = store;
         setSystemId(location);
         this.servletConnection = new ServletConnection(location);
-        this.validity = new ServletValidity(this.servletConnection);
+        connected = false;
     }
 
     /* (non-Javadoc)
@@ -81,24 +78,32 @@
         }
     }
 
-    public long getLastModified() {
-    	try {
+	/* (non-Javadoc)
+	 * @see org.apache.excalibur.source.impl.AbstractSource#getValidity()
+	 */
+	public SourceValidity getValidity() {
+		try {
 			connect();
+			return servletConnection.getLastModified() > 0 ? new ServletValidity(servletConnection.getResponseCode())
: null;
 		} catch (Exception e) {
-			return 0;
+			if (logger.isDebugEnabled())
+				logger.debug("Exception occured while making servlet request", e);
+			return null;
 		}
-    	return servletConnection.getLastModified();
 	}
-
-	public SourceValidity getValidity() {
+	
+	/* (non-Javadoc)
+	 * @see org.apache.excalibur.source.impl.AbstractSource#getLastModified()
+	 */
+	public long getLastModified() {
 		try {
 			connect();
+			return servletConnection.getLastModified() > 0 ? servletConnection.getLastModified()
: 0;
 		} catch (Exception e) {
 			if (logger.isDebugEnabled())
 				logger.debug("Exception occured while making servlet request", e);
-			return null;
+			return 0;
 		}
-		return servletConnection.getLastModified() > 0 ? this.validity : null;
 	}
 
 	/**
@@ -109,11 +114,48 @@
         return true;
     }
     
+	public OutputStream getOutputStream() throws IOException {
+		return servletConnection.getOutputStream();
+	}
+	
     private void connect() throws IOException, ServletException {
+    	if (connected) return;
+    	long lastModified = getStoredLastModified();
+    	if (lastModified > 0)
+    		servletConnection.setIfModifiedSince(lastModified);
     	servletConnection.connect();
-    	//This way it's guaranteed that validity has proper value set
-    	validity.setLastModified(servletConnection.getLastModified());
+    	connected = true;
+    	//If header is present, Last-Modified value will be stored for further use in conditional
gets
+    	setStoredLastModified(servletConnection.getLastModified());
     }
+    
+    /**
+     * Returns Last-Modified value from previous response if present. Otherwise 0 is returned.
+     * @return Last-Modified value from previous response if present. Otherwise 0 is returned.
+     */
+    private long getStoredLastModified() {
+    	Long lastModified = (Long)store.get(calculateInternalKey());
+    	return lastModified != null ? lastModified.longValue() : 0;
+	}
+    
+    /**
+     * Stores value of Last-Modified header in {@link #store}.
+     * @param lastModified value that will be stored in {@link #store}. Only positive values
will be stored.
+     * @throws IOException if exception occured while storing value 
+     */
+    private void setStoredLastModified(long lastModified) throws IOException {
+    	String key = calculateInternalKey();
+    	store.remove(key);
+    	if (lastModified > 0)
+    		store.store(key, new Long(lastModified));
+    }
+    
+	/**
+	 * @return key that will be used to store value of Last-Modified header
+	 */
+	private String calculateInternalKey() {
+		return ServletSource.class.getName() + "$" + getURI();
+	}
 
     private final class ServletValidity implements SourceValidity {
     	
@@ -122,12 +164,10 @@
 		 */
 		private static final long serialVersionUID = 1793646888814956538L;
 		
-		private transient ServletConnection servletConnection;
-		private transient Log logger = LogFactory.getLog(getClass());
-    	private long lastModified;
+    	private int responseCode;
 
-		public ServletValidity(ServletConnection servletConnection) {
-			this.servletConnection = servletConnection;
+		public ServletValidity(int responseCode) {
+			setResponseCode(responseCode);
     	}
 
 		/* (non-Javadoc)
@@ -144,35 +184,22 @@
 			if (newValidity instanceof ServletValidity) {
 				ServletValidity newServletValidity = (ServletValidity)newValidity;
 				
-				newServletValidity.servletConnection.setIfModifiedSince(this.getLastModified());
-				try {
-					newServletValidity.servletConnection.connect();
-					switch (newServletValidity.servletConnection.getResponseCode()) {
-						case HttpServletResponse.SC_NOT_MODIFIED: return 1;
-						case HttpServletResponse.SC_OK: return -1;
-						default: return 0; 
-					}
-				} catch (Exception e) {
-					if (logger.isDebugEnabled())
-						logger.debug("Exception occured while checking for cache entry for servlet source is
still valid", e);
-					return 0;
+				switch (newServletValidity.getResponseCode()) {
+						case HttpServletResponse.SC_NOT_MODIFIED: return SourceValidity.VALID;
+						case HttpServletResponse.SC_OK: return SourceValidity.INVALID;
+						default: return 0;
 				}
 			}
-			return 0;
+			return SourceValidity.UNKNOWN;
 		}
 
-		public long getLastModified() {
-			return lastModified;
+		public int getResponseCode() {
+			return responseCode;
 		}
 
-		public void setLastModified(long lastModified) {
-			this.lastModified = lastModified;
-		}
-    	
+		public void setResponseCode(int responseCode) {
+			this.responseCode = responseCode;
+		}    	
     }
-
-	public OutputStream getOutputStream() throws IOException {
-		return servletConnection.getOutputStream();
-	}
 
 }

Modified: cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSourceFactory.java
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSourceFactory.java?view=diff&rev=531020&r1=531019&r2=531020
==============================================================================
--- cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSourceFactory.java
(original)
+++ cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/java/org/apache/cocoon/servletservice/components/ServletSourceFactory.java
Sat Apr 21 05:49:15 2007
@@ -26,6 +26,7 @@
 import org.apache.excalibur.source.SourceFactory;
 import org.apache.excalibur.source.SourceUtil;
 import org.apache.excalibur.source.URIAbsolutizer;
+import org.apache.excalibur.store.Store;
 
 /**
  * This class implements the servlet: protocol.
@@ -38,6 +39,11 @@
 
     /** By default we use the logger for this class. */
     private Log logger = LogFactory.getLog(getClass());
+    
+    /**
+     * Store that will be used by {@link ServletSource}.
+     */
+    private Store store; 
 
     private Log getLogger() {
         return this.logger;
@@ -55,7 +61,7 @@
             getLogger().debug("Creating source object for " + location);
         }
 
-        return new ServletSource(location);
+        return new ServletSource(location, store);
     }
 
     /*
@@ -80,5 +86,13 @@
     public String absolutize(String baseURI, String location) {
         return SourceUtil.absolutize(baseURI, location, true);
     }
+
+	public Store getStore() {
+		return store;
+	}
+
+	public void setStore(Store store) {
+		this.store = store;
+	}
 
 }

Modified: cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/resources/META-INF/cocoon/spring/cocoon-servlet-service-servlet-source-factory.xml
URL: http://svn.apache.org/viewvc/cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/resources/META-INF/cocoon/spring/cocoon-servlet-service-servlet-source-factory.xml?view=diff&rev=531020&r1=531019&r2=531020
==============================================================================
--- cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/resources/META-INF/cocoon/spring/cocoon-servlet-service-servlet-source-factory.xml
(original)
+++ cocoon/trunk/core/cocoon-servlet-service/cocoon-servlet-service-components/src/main/resources/META-INF/cocoon/spring/cocoon-servlet-service-servlet-source-factory.xml
Sat Apr 21 05:49:15 2007
@@ -25,6 +25,8 @@
       |
       | Each source factory adds a special uri schemes to the system.
       +-->
-  <bean name="org.apache.excalibur.source.SourceFactory/servlet" class="org.apache.cocoon.servletservice.components.ServletSourceFactory"/>
+  <bean name="org.apache.excalibur.source.SourceFactory/servlet" class="org.apache.cocoon.servletservice.components.ServletSourceFactory">
+    <property name="store" ref="org.apache.excalibur.store.Store"/>
+  </bean>
 
 </beans>



Mime
View raw message