SixLabors/ImageSharp.Web

Clarify ImageMetaData Last Modified Time / etag value

deanmarcussen opened this issue · 2 comments

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp.Web
  • I have verified if the problem exist in both DEBUG and RELEASE mode
  • I have searched open and closed issues to ensure it has not already been reported

Description

Cheers for all the fixes over the weekend @JimBobSquarePants .

I'm just doing some integration work with ImageSharp and OrchardCore at the moment and wanted to clarify the behaviour of the ImageMetaData LastModifiedDate, which is used to generate the etag.

The comments all say that it's based on the date the source file was modified

/// <param name="lastWriteTimeUtc">The date and time in coordinated universal time (UTC) since the source file was last modified.</param>

but in ImageSharpMiddleware when creating the cached image metadata it uses DateTime.UtcNow, rather than the last modified date of the source image.

cachedImageMetadata = new ImageMetaData(DateTime.UtcNow, image.Format.DefaultMimeType, maxAge);

Unless I've missed something somewhere else?

I can't decide if I prefer it this way, or not, but figured I should clarify if it's intended / unintended?

Hi @deanmarcussen

Cheers for all the fixes over the weekend @JimBobSquarePants .

No worries! 😄

Regarding your question. Yeah, I can see why this is ambiguous. The source in this sense does not mean the image we are processing rather it means the metadata source. We reuse the same class to represent both source and cached image metadata.

So, there are 3 potential values for the property:

When representing an image to be processed.

var metadata = new ImageMetaData(fileInfo.LastModified.UtcDateTime);

When representing an image that has previously been processed and cached.

DateTime.TryParse(lastWriteTimeUtcString, null, DateTimeStyles.RoundtripKind, out DateTime lastWriteTimeUtc);

When representing a newly processed image to be cached.

cachedImageMetadata = new ImageMetaData(DateTime.UtcNow, image.Format.DefaultMimeType, maxAge);

I hope this makes sense?

Awesome, yes that clears up any ambiguity, thanks.