Skip to content

Commit

Permalink
Merge pull request #699 from dlcs/fix/thumbnailcalculator_sizes
Browse files Browse the repository at this point in the history 
`ThumbnailCalculator` bugfixes
  • Loading branch information
@griffri
griffri authored Jan 10, 2024
2 parents 6812262 + c03fb3d commit e475764
Show file tree
Hide file tree
Showing 3 changed files with 246 additions and 19 deletions.
195 changes: 190 additions & 5 deletions src/protagonist/DLCS.Repository.Tests/Assets/ThumbnailCalculatorTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;
using DLCS.Model.Assets;
using DLCS.Repository.Assets;
using FluentAssertions;
Expand All @@ -12,7 +13,8 @@ public class ThumbnailCalculatorTests
{
private readonly List<Size> landscapeSizes;
private readonly List<Size> portraitSizes;

private readonly List<Size> squareSizes;

public ThumbnailCalculatorTests()
{
portraitSizes = new List<Size>
Expand All @@ -28,20 +30,27 @@ public ThumbnailCalculatorTests()
new(400, 200),
new(200, 100),
};

squareSizes = new List<Size>()
{
new Size(200, 200),
new Size(500, 500)
};
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void GetCandidates_ReturnsCorrectLongestEdge_IfSizeAndHeightProvided(bool resize)
public void GetCandidates_ReturnsCorrectLongestEdge_IfConfinedAndSizeAndHeightProvided(bool resize)
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 110,
Height = 200
Height = 200,
Confined = true
}
};

Expand Down Expand Up @@ -230,4 +239,180 @@ public void GetCandidates_Resize_ReturnsCorrectIdealSize_IfConfinedSquareRequest
result.SmallerSize.Width.Should().Be(200);
result.LargerSize.Should().BeNull();
}

[Fact]
public void GetCandidates_ReturnsTrueForKnownSize_IfConfined_ForLandscapeImage()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 400,
Height = 250,
Confined = true
}
};

// Act
var result = ThumbnailCalculator.GetCandidate(landscapeSizes, imageRequest, false);

// Assert
result.KnownSize.Should().BeTrue();
result.LongestEdge.Should().Be(400);
}

[Fact]
public void GetCandidates_ReturnsTrueForKnownSize_IfConfined_ForPortraitImage()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 250,
Height = 400,
Confined = true
}
};

// Act
var result = ThumbnailCalculator.GetCandidate(portraitSizes, imageRequest, false);

// Assert
result.KnownSize.Should().BeTrue();
result.LongestEdge.Should().Be(400);
}

[Fact]
public void GetCandidates_ReturnsCorrectIdealSize_ForResizedLandscapeImage()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 450,
Height = 250,
}
};

// Act
var result = (ResizableSize)ThumbnailCalculator.GetCandidate(landscapeSizes, imageRequest, true);

// Assert
result.KnownSize.Should().BeFalse();
result.Ideal.Width.Should().Be(450);
result.Ideal.Height.Should().Be(250);
}

[Fact]
public void GetCandidates_ReturnsCorrectIdealSize_ForResizedPortraitImage()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 250,
Height = 450,
}
};

// Act
var result = (ResizableSize)ThumbnailCalculator.GetCandidate(portraitSizes, imageRequest, true);

// Assert
result.KnownSize.Should().BeFalse();
result.Ideal.Width.Should().Be(250);
result.Ideal.Height.Should().Be(450);
}

[Fact]
public void GetCandidates_ReturnsCorrectIdealSize_IfLargerThanAvailableSizes()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 802,
Height = 401,
Confined = true,
}
};

// Act
var result = (ResizableSize)ThumbnailCalculator.GetCandidate(landscapeSizes, imageRequest, true);

// Assert
result.KnownSize.Should().BeFalse();
result.Ideal.Width.Should().Be(802);
result.Ideal.Height.Should().Be(401);
}

[Fact]
public void GetCandidates_ReturnsCorrectLongestEdge_IfSquare()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 200,
Height = 500,
Confined = true,
}
};

// Act
var result = (ResizableSize)ThumbnailCalculator.GetCandidate(squareSizes, imageRequest, true);

// Assert
result.LongestEdge.Should().Be(200);
}

[Fact]
public void GetCandidates_ReturnsEmptyLongestEdge_IfConfined_ForLandscapeImage_IfSmallDimensionTooSmall()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 800,
Height = 350,
Confined = true,
}
};

//Act
var result = ThumbnailCalculator.GetCandidate(landscapeSizes, imageRequest, false);

//Assert
result.KnownSize.Should().BeFalse();
result.LongestEdge.Should().BeNull();
}

[Fact]
public void GetCandidates_ReturnsEmptyLongestEdge_IfConfined_ForPortraitImage_IfSmallDimensionTooSmall()
{
// Arrange
var imageRequest = new ImageRequest
{
Size = new SizeParameter
{
Width = 350,
Height = 800,
Confined = true,
}
};

//Act
var result = ThumbnailCalculator.GetCandidate(portraitSizes, imageRequest, false);

//Assert
result.KnownSize.Should().BeFalse();
result.LongestEdge.Should().BeNull();
}
}
62 changes: 52 additions & 10 deletions src/protagonist/DLCS.Repository/Assets/ThumbnailCalculator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,54 @@ public static SizeCandidate GetCandidate(List<Size> sizes, ImageRequest imageReq

private static SizeCandidate GetLongestEdge(List<Size> sizes, ImageRequest imageRequest)
{
if (imageRequest.Size.Width > 0 && imageRequest.Size.Height > 0)
var requestWidth = imageRequest.Size.Width ?? 0;
var requestHeight = imageRequest.Size.Height ?? 0;

if (requestWidth > 0 && requestHeight > 0)
{
// We don't actually need to check imageRequest.Size.Confined (!w,h) because same logic applies...
var max = Math.Max(imageRequest.Size.Width ?? 0, imageRequest.Size.Height ?? 0);
return sizes.Select(s => s.MaxDimension).Contains(max)
? new SizeCandidate(max)
: new SizeCandidate();
// get the longest dimension of the requested size
var max = Math.Max(requestWidth, requestHeight);
var foundExactSize = sizes.Exists(s =>
s.Width == requestWidth &&
s.Height == requestHeight);

// We found a size that matches the request exactly, so we'll go with that
if (foundExactSize)
{
return new SizeCandidate(max);
}

// If the image is confined, are there any sizes that fit?
if (imageRequest.Size.Confined)
{
// Pick the first thumbnail size as a reference for shape
var shape = sizes.First().GetShape();

// If this is a landscape image, max should match its width
if (shape == ImageShape.Landscape
&& sizes.Exists(s => s.Width == max && requestHeight >= s.Height))
{
return new SizeCandidate(max);
}
// For portrait images, max should match its height
else if (shape == ImageShape.Portrait
&& sizes.Exists(s => s.Height == max && requestWidth >= s.Width))
{
return new SizeCandidate(max);
}
// Lastly, for square images, min should match both dimensions instead
else if (shape == ImageShape.Square)
{
var min = Math.Min(requestWidth, requestHeight);
if (sizes.Exists(s => s.Width == min))
{
return new SizeCandidate(min);
}
}
}

// Otherwise, resize
return new SizeCandidate();
}

if (imageRequest.Size.Max)
Expand All @@ -41,23 +82,23 @@ private static SizeCandidate GetLongestEdge(List<Size> sizes, ImageRequest image

// we need to know the sizes of things...
int? longestEdge = null;
if (imageRequest.Size.Width > 0)
if (requestWidth > 0)
{
foreach (var size in sizes)
{
if (size.Width == imageRequest.Size.Width)
if (size.Width == requestWidth)
{
longestEdge = size.MaxDimension;
break;
}
}
}

if (imageRequest.Size.Height > 0)
if (requestHeight > 0)
{
foreach (var size in sizes)
{
if (size.Height == imageRequest.Size.Height)
if (size.Height == requestHeight)
{
longestEdge = size.MaxDimension;
break;
Expand All @@ -73,6 +114,7 @@ private static ResizableSize GetLongestEdgeAndSize(List<Size> sizes, ImageReques
// TODO - handle there being none "open"?

var sizeCandidate = GetLongestEdge(sizes, imageRequest);

if (sizeCandidate.KnownSize)
{
// We have found a matching size, use that.
Expand Down
8 changes: 4 additions & 4 deletions src/protagonist/Orchestrator.Tests/Features/Images/ImageRequestHandlerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public async Task HandleRequest_ProxiesToThumbs_IfRequiresAuth_AndFullRegionOfKn
{
// Arrange
var context = new DefaultHttpContext();
context.Request.Path = "/iiif-img/2/2/test-image/full/!100,150/0/default.jpg";
context.Request.Path = "/iiif-img/2/2/test-image/full/!150,150/0/default.jpg";

A.CallTo(() => customerRepository.GetCustomerPathElement("2")).Returns(new CustomerPathElement(2, "Test-Cust"));
var assetId = new AssetId(2, 2, "test-image");
Expand All @@ -293,15 +293,15 @@ public async Task HandleRequest_ProxiesToThumbs_IfRequiresAuth_AndFullRegionOfKn

// Assert
result.Target.Should().Be(ProxyDestination.Thumbs);
result.Path.Should().Be("thumbs/2/2/test-image/full/!100,150/0/default.jpg");
result.Path.Should().Be("thumbs/2/2/test-image/full/!150,150/0/default.jpg");
}

[Fact]
public async Task HandleRequest_ProxiesToThumbs_IfFullRegion_AndKnownSize()
{
// Arrange
var context = new DefaultHttpContext();
context.Request.Path = "/iiif-img/2/2/test-image/full/!100,150/0/default.jpg";
context.Request.Path = "/iiif-img/2/2/test-image/full/!150,150/0/default.jpg";

A.CallTo(() => customerRepository.GetCustomerPathElement("2")).Returns(new CustomerPathElement(2, "Test-Cust"));
var assetId = new AssetId(2, 2, "test-image");
Expand All @@ -318,7 +318,7 @@ public async Task HandleRequest_ProxiesToThumbs_IfFullRegion_AndKnownSize()

// Assert
result.Target.Should().Be(ProxyDestination.Thumbs);
result.Path.Should().Be("thumbs/2/2/test-image/full/!100,150/0/default.jpg");
result.Path.Should().Be("thumbs/2/2/test-image/full/!150,150/0/default.jpg");
}

[Theory]
Expand Down

0 comments on commit e475764

Please sign in to comment.