Skip to content

Using weak tag instead of just last modification date #2435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.Comparator;
import java.util.EnumSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
Expand All @@ -51,8 +52,9 @@
import javax.servlet.ServletRequest;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import org.suigeneris.jrcs.diff.Diff;
import org.suigeneris.jrcs.diff.DifferentiationFailedException;
import javax.servlet.http.HttpServletResponse;
import javax.ws.rs.core.HttpHeaders;
import org.opengrok.indexer.Info;
import org.opengrok.indexer.analysis.AnalyzerGuru;
import org.opengrok.indexer.analysis.ExpandTabsReader;
import org.opengrok.indexer.analysis.FileAnalyzer.Genre;
Expand All @@ -70,6 +72,8 @@
import org.opengrok.indexer.search.QueryBuilder;
import org.opengrok.indexer.util.IOUtils;
import org.opengrok.indexer.web.messages.MessagesContainer.AcceptedMessage;
import org.suigeneris.jrcs.diff.Diff;
import org.suigeneris.jrcs.diff.DifferentiationFailedException;

/**
* A simple container to lazy initialize common vars wrt. a single request. It
Expand Down Expand Up @@ -1619,4 +1623,76 @@ public void checkSourceRootExistence() throws IOException {
throw new IOException(String.format("Source root path \"%s\" is not readable", sourceRootPathFile.getAbsolutePath()));
}
}

/**
* Get all project related messages. These include
* <ol>
* <li>Main messages</li>
* <li>Messages with tag = project name</li>
* <li>Messages with tag = project's groups names</li>
* </ol>
*
* @return the sorted set of messages according to the accept time
* @see org.opengrok.indexer.web.messages.MessagesContainer#MESSAGES_MAIN_PAGE_TAG
*/
private SortedSet<AcceptedMessage> getProjectMessages() {
SortedSet<AcceptedMessage> messages = getMessages();

if (getProject() != null) {
messages.addAll(getMessages(getProject().getName()));
getProject().getGroups().forEach(group -> {
messages.addAll(getMessages(group.getName()));
});
}

return messages;
}

/**
* Decide if this resource has been modified since the header value in the request.
* <p>
* The resource is modified since the weak ETag value in the request, the ETag is
* computed using:
* <ul>
* <li>the source file modification</li>
* <li>project messages</li>
* <li>last timestamp for index</li>
* <li>OpenGrok current deployed version</li>
* </ul>
* <p>
* <p>
* If the resource was modified, appropriate headers in the response are filled.
*
* @param request the http request containing the headers
* @param response the http response for setting the headers
* @return true if resource was not modified; false otherwise
* @see <a href="https://tools.ietf.org/html/rfc7232#section-2.3">HTTP ETag</a>
* @see <a href="https://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html">HTTP Caching</a>
*/
public boolean isNotModified(HttpServletRequest request, HttpServletResponse response) {
String currentEtag = String.format("W/\"%s\"",
Objects.hash(
// last modified time as UTC timestamp in millis
getLastModified(),
// all project related messages which changes the view
getProjectMessages(),
// last timestamp value
getEnv().getDateForLastIndexRun().getTime(),
// OpenGrok version has changed since the last time
Info.getVersion()
)
);

String headerEtag = request.getHeader(HttpHeaders.IF_NONE_MATCH);

if (headerEtag != null && headerEtag.equals(currentEtag)) {
// weak ETag has not changed, return 304 NOT MODIFIED
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
return true;
}

// return 200 OK
response.setHeader(HttpHeaders.ETAG, currentEtag);
return false;
}
}
3 changes: 0 additions & 3 deletions opengrok-web/src/main/webapp/WEB-INF/web.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,8 @@
<filter-mapping>
<filter-name>ExpiresHalfHourFilter</filter-name>
<url-pattern>/history/*</url-pattern>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not history ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

history does not include mast.jsp so this does not affect it anyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that history page will not display any messages ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. It means it will work like now, with sometimes inconsistent view as mentioned in #2405 because of the 30 minutes caching interval. This is also a subject to change if desired, but in different PR.

<url-pattern>/xref/*</url-pattern>
<url-pattern>/raw/*</url-pattern>
<url-pattern>/download/*</url-pattern>
<url-pattern>/diff/*</url-pattern>
<url-pattern>/more/*</url-pattern>
<url-pattern>/rss/*</url-pattern>
<url-pattern>/error</url-pattern>
<url-pattern>/enoent</url-pattern>
Expand Down
8 changes: 3 additions & 5 deletions opengrok-web/src/main/webapp/mast.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,11 @@ org.opengrok.indexer.web.Util"%><%
return;
}

// jel: hmmm - questionable for dynamic content
long flast = cfg.getLastModified();
if (request.getDateHeader("If-Modified-Since") >= flast) {
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
if (cfg.isNotModified(request, response)) {
// the resource was not modified
// the code 304 NOT MODIFIED has been inserted to the response
return;
}
response.setDateHeader("Last-Modified", flast);

// Use UTF-8 if no encoding is specified in the request
if (request.getCharacterEncoding() == null) {
Expand Down