Potential thread-safety fix for Memento from Luda.

[anjackson]

(Partial?) fix for #177 - UKWA can help test this when @PsypherPunk is able to deploy this branch.

Email quoth:

I attached [/waybackcore/src/main/java/org/archive/wayback/memento/] MementoUtils.java with corrected initialization of SimpleDateFormat.
I think it should eliminate bug #177. I used the same approach as can be find in openwayback commons (org.archive.util.ArchiveUtils) for thread safety.
There is also other possibility to use Apache Commons Lang 3.x FastDateFormat, they claim that classes are thread safe and faster then SimpleDateFormat.

There is similar problem in formatting Link based format at cdx server module at org.archive.cdxserver.writer.MementoLinkWriter

[anjackson]

This presumably needs fixing too:

openwayback/wayback-cdx-server/src/main/java/org/archive/cdxserver/writer/MementoLinkWriter.java

Line 102 in 0041e3d

public final static SimpleDateFormat HTTP_LINK_DATE_FORMATTER;

And there may be others: https://github.com/iipc/openwayback/search?utf8=%E2%9C%93&q=SimpleDateFormat&type=Code

Although, I'm pretty sure those Memento JSPs are no longer needed since @ikreymer moved the Memento support into core.

The fix is pretty clunky, so I'd consider switching to FastDateFormat if Commons Lang 3.x is already on the classpath.

[PsypherPunk]

Tested this locally by making multiple, synchronous requests for resources and comparing all the OpenWayback timestamps to the 'datetime' attributes. In every case they matched so this appears to fix the issue.

However, before merging this, do we need to fix the other SimpleDateFormat references as per @anjackson's comment?

[luda171]

I can change the other files if it helps to push the issue. Do we want to go with FastDateFormat (new dependency) or want to keep the Thread Local approach? The memento related *.jsp are not used.

[anjackson]

It seems that FastDateFormat is already on the class path, so we could use it straight away, e.g. via DateFormatUtils.formatUTC with the US Locale.

[luda171]

There is commons-lang-2.5.jar in classpath , but it only has format
functionality (not parse).
So commons-lang3-3.3.2.jar need to be added to be able to parse.
so import is different
import org.apache.commons.lang3.time.FastDateFormat;
but everything else is straight forward:
TimeZone tz = TimeZone.getTimeZone("GMT");
FastDateFormat httpformatter = FastDateFormat.getInstance("E, dd MMM yyyy
HH:mm:ss z",tz,Locale.US);
and format and parse methods stay the same.

On Wed, Nov 5, 2014 at 6:07 AM, Andy Jackson [email protected]
wrote:

It seems that FastDateFormat is already on the class path, so we could use
it straight away, e.g. via DateFormatUtils.formatUTC with the US Locale.

—
Reply to this email directly or view it on GitHub
#180 (comment).

[anjackson]

Then we'll have to stick to the ThreadLocal syntax if we're going to do this as a bug release.

[anjackson]

Lets pull this and close it down, but leave #177 open until we've tested it.