Skip to content

fix: Improve ICO decoding and UI consistency #2

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.

Sign up for GitHub

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

Open
TYK7 wants to merge 1 commit into
base: feat/color-scheme-extractor
Choose a base branch
from
Open

fix: Improve ICO decoding and UI consistency #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions color-extractor-ui/src/app/color-input-display/color-input-display.component.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@ h2 {
margin-bottom: 10px;
}

.color-info p {
.color-details p {
margin: 5px 0;
font-size: 14px;
word-break: break-all;
}

.color-info p strong {
.color-details p strong {
color: #555;
}
5 changes: 5 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@
<artifactId>jsoup</artifactId>
<version>1.15.3</version> <!-- Or a more recent version -->
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-imaging</artifactId>
<version>1.0.0-alpha6</version>
</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
Expand Down
93 changes: 79 additions & 14 deletions src/main/java/com/example/colorschemeidentifier/service/ColorExtractionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,13 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Service;
import org.apache.commons.imaging.Imaging;
import org.apache.commons.imaging.ImageReadException;


import javax.imageio.ImageIO;
import java.awt.image.BufferedImage;
import java.io.ByteArrayInputStream;
// import java.io.ByteArrayOutputStream; // No longer strictly needed after refactor
import java.io.IOException;
import java.io.InputStream;
Expand Down Expand Up @@ -448,30 +452,91 @@ private void extractColorsFromLogoImage(Set<String> logoUrlSet, Map<String, Colo
String sourceIdentifier = logoUrl.startsWith("data:image")
? "logo_data_uri:" + logoUrl.substring(0, Math.min(logoUrl.length(), 50)) + "..."
: "logo:" + logoUrl;

InputStream imageStream = null;
BufferedImage image = null;

try {
InputStream imageStream = logoUrl.startsWith("data:image")
? processDataUriGetStream(logoUrl, sourceIdentifier)
: openConnectionAndGetStream(logoUrl);
boolean isIcoByExtension = logoUrl.toLowerCase().endsWith(".ico");
String dataUriMimeType = null;
if (logoUrl.startsWith("data:image/")) {
int mimeEnd = logoUrl.indexOf(';');
if (mimeEnd > 0) {
dataUriMimeType = logoUrl.substring(5, mimeEnd).toLowerCase();
}
}
boolean isIcoByMime = "image/x-icon".equals(dataUriMimeType) || "image/vnd.microsoft.icon".equals(dataUriMimeType);

imageStream = logoUrl.startsWith("data:image")
? processDataUriGetStream(logoUrl, sourceIdentifier) // This returns ByteArrayInputStream
: openConnectionAndGetStream(logoUrl); // This returns HttpInputStream

if (imageStream == null) {
logger.warn("Could not get input stream for logo URL: {}", logoUrl);
return;
}

try (InputStream in = imageStream) { // Ensure stream is closed
BufferedImage image = ImageIO.read(in);
if (image != null) {
logger.info("Processing logo image: {}", sourceIdentifier);
// Calls the internal method, setting isLogoColor to true
extractDominantColorsFromImageInternal(image, colorFrequencies, sourceIdentifier, true);
} else {
logger.warn("Could not decode logo image from source: {}", sourceIdentifier);
// For ICO, we need to buffer the stream if it's not already a ByteArrayInputStream,
// because Apache Commons Imaging might need to read it multiple times or it might not support mark/reset on HttpInputStream.
// ImageIO.read also benefits from a resettable stream for some formats.
if (!imageStream.markSupported()) {
imageStream = new ByteArrayInputStream(imageStream.readAllBytes());
}
imageStream.mark(Integer.MAX_VALUE); // Mark the beginning of the stream


if (isIcoByExtension || isIcoByMime) {
logger.info("Attempting to decode ICO logo with Apache Commons Imaging: {}", sourceIdentifier);
try {
image = Imaging.getBufferedImage(imageStream);
logger.info("Successfully decoded ICO logo with Apache Commons Imaging: {}", sourceIdentifier);
} catch (ImageReadException | IOException imagingEx) {
logger.warn("Apache Commons Imaging failed to decode logo ({}): {}. Attempting fallback to ImageIO.", sourceIdentifier, imagingEx.getMessage());
imageStream.reset(); // Reset stream for ImageIO
try {
image = ImageIO.read(imageStream);
if (image != null) {
logger.info("Successfully decoded ICO logo with ImageIO fallback: {}", sourceIdentifier);
} else {
logger.warn("ImageIO fallback also failed to decode ICO logo: {}", sourceIdentifier);
}
} catch (IOException imageIoEx) {
logger.warn("ImageIO fallback failed with IOException for ICO logo ({}): {}", sourceIdentifier, imageIoEx.getMessage());
}
}
} else {
logger.info("Attempting to decode non-ICO logo with ImageIO: {}", sourceIdentifier);
try {
image = ImageIO.read(imageStream);
if (image != null) {
logger.info("Successfully decoded non-ICO logo with ImageIO: {}", sourceIdentifier);
} else {
// This case is important: ImageIO.read can return null for unsupported formats without throwing an exception.
logger.warn("ImageIO.read returned null for logo (likely unsupported format or corrupt image): {}", sourceIdentifier);
}
} catch (IOException imageIoEx) {
logger.warn("ImageIO failed to decode non-ICO logo ({}) with IOException: {}", sourceIdentifier, imageIoEx.getMessage());
}
}
} catch (IOException e) {
logger.error("Error reading logo image stream for {}: {}", sourceIdentifier, e.getMessage());
} catch (Exception e) {

if (image != null) {
extractDominantColorsFromImageInternal(image, colorFrequencies, sourceIdentifier, true);
} else {
logger.warn("Could not decode logo image from source (all attempts failed): {}", sourceIdentifier);
}

} catch (IOException e) { // Catches IO errors from stream opening or readAllBytes
logger.error("IOException during logo image processing for {}: {}", sourceIdentifier, e.getMessage());
} catch (Exception e) { // Catches other unexpected errors
logger.error("An unexpected error occurred while processing logo image {}: {}", sourceIdentifier, e.getMessage(), e);
} finally {
if (imageStream != null) {
try {
imageStream.close();
} catch (IOException e) {
logger.error("Failed to close image stream for {}: {}", sourceIdentifier, e.getMessage());
}
}
}
}

Expand Down
146 changes: 145 additions & 1 deletion src/test/java/com/example/colorschemeidentifier/service/ColorExtractionServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ void extractColorsFromUrl_withLogoProcessing() throws IOException {
// Renamed to reflect it tests the internal algorithm if made accessible
@Test
void testExtractDominantColorsDirectlyInternal_logic() throws Exception {
BufferedImage image = new BufferedImage(3, 1, BufferedImage.TYPE_INT_ARGB);
BufferedImage image = new BufferedImage(3, 1, BufferedImage.TYPE_INT_ARGB); // A sample image
image.setRGB(0, 0, new java.awt.Color(255, 0, 0).getRGB()); // Red
image.setRGB(1, 0, new java.awt.Color(0, 255, 0).getRGB()); // Green
image.setRGB(2, 0, new java.awt.Color(255, 0, 0).getRGB()); // Red
Expand Down Expand Up @@ -628,6 +628,150 @@ void testExtractDominantColorsDirectlyInternal_logic() throws Exception {
assertEquals("logo:logo_image.png", colorFrequencies.get("#20E020").getColorInfo().getSource());
}

// Helper method to create a dummy BufferedImage
private BufferedImage createDummyImage(int width, int height, int color) {
BufferedImage img = new BufferedImage(width, height, BufferedImage.TYPE_INT_RGB);
for (int x = 0; x < width; x++) {
for (int y = 0; y < height; y++) {
img.setRGB(x, y, color);
}
}
return img;
}

private void setupMockDocumentForLogoUrl(Document mockDoc, String logoUrlToReturn) {
when(mockDoc.select("link[rel=icon], link[rel~=(?i)shortcut icon]")).thenAnswer(inv -> {
Elements mockIconLinks = new Elements();
Element mockLinkElement = mock(Element.class);
when(mockLinkElement.absUrl("href")).thenReturn(logoUrlToReturn);
mockIconLinks.add(mockLinkElement);
return mockIconLinks;
});
when(mockDoc.baseUri()).thenReturn(TEST_BASE_URL);
// Prevent other extractions by returning empty elements for other selectors
when(mockDoc.select(not(eq("link[rel=icon], link[rel~=(?i)shortcut icon]")))).thenReturn(new Elements());
}

@Test
void icoLogo_DecodedByCommonsImaging_Success() throws IOException {
String logoUrl = TEST_BASE_URL + "logo.ico";
Document mockDoc = mock(Document.class);
setupMockDocumentForLogoUrl(mockDoc, logoUrl);
BufferedImage mockIcoImage = createDummyImage(1,1, java.awt.Color.GREEN.getRGB()); // Green -> #20E020

try (MockedStatic<Jsoup> mockedJsoup = Mockito.mockStatic(Jsoup.class);
MockedStatic<org.apache.commons.imaging.Imaging> mockedImaging = Mockito.mockStatic(org.apache.commons.imaging.Imaging.class);
MockedStatic<ImageIO> mockedImageIO = Mockito.mockStatic(ImageIO.class)) {

Connection mockMainConnection = mock(Connection.class);
mockedJsoup.when(() -> Jsoup.connect(TEST_URL)).thenReturn(mockMainConnection);
when(mockMainConnection.timeout(anyInt())).thenReturn(mockMainConnection);
when(mockMainConnection.get()).thenReturn(mockDoc);

mockedImaging.when(() -> org.apache.commons.imaging.Imaging.getBufferedImage(any(InputStream.class)))
.thenReturn(mockIcoImage);
mockedImageIO.when(() -> ImageIO.read(any(InputStream.class))).thenThrow(new AssertionError("ImageIO.read should not be called"));

List<ColorInfo> result = colorExtractionService.extractColorsFromUrl(TEST_URL);

assertEquals(1, result.size());
assertEquals("#20E020", result.get(0).getHexValue()); // Quantized Green
assertTrue(result.get(0).isLogoColor());
assertEquals("logo:" + logoUrl, result.get(0).getSource());
mockedImaging.verify(() -> org.apache.commons.imaging.Imaging.getBufferedImage(any(InputStream.class)), times(1));
mockedImageIO.verify(() -> ImageIO.read(any(InputStream.class)), never());
}
}

@Test
void icoLogo_CommonsImagingFails_ImageIOSucceeds() throws IOException {
String logoUrl = TEST_BASE_URL + "favicon.ico";
Document mockDoc = mock(Document.class);
setupMockDocumentForLogoUrl(mockDoc, logoUrl);
BufferedImage mockFallbackImage = createDummyImage(1,1, java.awt.Color.RED.getRGB()); // Red -> #E02020


try (MockedStatic<Jsoup> mockedJsoup = Mockito.mockStatic(Jsoup.class);
MockedStatic<org.apache.commons.imaging.Imaging> mockedImaging = Mockito.mockStatic(org.apache.commons.imaging.Imaging.class);
MockedStatic<ImageIO> mockedImageIO = Mockito.mockStatic(ImageIO.class)) {

Connection mockMainConnection = mock(Connection.class);
mockedJsoup.when(() -> Jsoup.connect(TEST_URL)).thenReturn(mockMainConnection);
when(mockMainConnection.timeout(anyInt())).thenReturn(mockMainConnection);
when(mockMainConnection.get()).thenReturn(mockDoc);

mockedImaging.when(() -> org.apache.commons.imaging.Imaging.getBufferedImage(any(InputStream.class)))
.thenThrow(new org.apache.commons.imaging.ImageReadException("Commons Imaging test error"));
mockedImageIO.when(() -> ImageIO.read(any(InputStream.class)))
.thenReturn(mockFallbackImage);

List<ColorInfo> result = colorExtractionService.extractColorsFromUrl(TEST_URL);

assertEquals(1, result.size());
assertEquals("#E02020", result.get(0).getHexValue()); // Quantized Red
assertTrue(result.get(0).isLogoColor());
mockedImaging.verify(() -> org.apache.commons.imaging.Imaging.getBufferedImage(any(InputStream.class)), times(1));
mockedImageIO.verify(() -> ImageIO.read(any(InputStream.class)), times(1));
}
}

@Test
void icoLogo_CommonsImagingFails_ImageIOFails() throws IOException {
String logoUrl = TEST_BASE_URL + "another.ico";
Document mockDoc = mock(Document.class);
setupMockDocumentForLogoUrl(mockDoc, logoUrl);

try (MockedStatic<Jsoup> mockedJsoup = Mockito.mockStatic(Jsoup.class);
MockedStatic<org.apache.commons.imaging.Imaging> mockedImaging = Mockito.mockStatic(org.apache.commons.imaging.Imaging.class);
MockedStatic<ImageIO> mockedImageIO = Mockito.mockStatic(ImageIO.class)) {

Connection mockMainConnection = mock(Connection.class);
mockedJsoup.when(() -> Jsoup.connect(TEST_URL)).thenReturn(mockMainConnection);
when(mockMainConnection.timeout(anyInt())).thenReturn(mockMainConnection);
when(mockMainConnection.get()).thenReturn(mockDoc);

mockedImaging.when(() -> org.apache.commons.imaging.Imaging.getBufferedImage(any(InputStream.class)))
.thenThrow(new org.apache.commons.imaging.ImageReadException("Commons Imaging test error"));
mockedImageIO.when(() -> ImageIO.read(any(InputStream.class)))
.thenReturn(null); // ImageIO.read returns null for failure

List<ColorInfo> result = colorExtractionService.extractColorsFromUrl(TEST_URL);

assertTrue(result.isEmpty(), "No colors should be extracted if logo decoding fails completely.");
mockedImaging.verify(() -> org.apache.commons.imaging.Imaging.getBufferedImage(any(InputStream.class)), times(1));
mockedImageIO.verify(() -> ImageIO.read(any(InputStream.class)), times(1));
}
}

@Test
void pngLogo_DecodedByImageIO_CommonsImagingSkipped() throws IOException {
String logoUrl = TEST_BASE_URL + "logo.png"; // Non-ICO
Document mockDoc = mock(Document.class);
setupMockDocumentForLogoUrl(mockDoc, logoUrl);
BufferedImage mockPngImage = createDummyImage(1,1, java.awt.Color.CYAN.getRGB()); // Cyan -> #20E0E0

try (MockedStatic<Jsoup> mockedJsoup = Mockito.mockStatic(Jsoup.class);
MockedStatic<org.apache.commons.imaging.Imaging> mockedImaging = Mockito.mockStatic(org.apache.commons.imaging.Imaging.class);
MockedStatic<ImageIO> mockedImageIO = Mockito.mockStatic(ImageIO.class)) {

Connection mockMainConnection = mock(Connection.class);
mockedJsoup.when(() -> Jsoup.connect(TEST_URL)).thenReturn(mockMainConnection);
when(mockMainConnection.timeout(anyInt())).thenReturn(mockMainConnection);
when(mockMainConnection.get()).thenReturn(mockDoc);

mockedImageIO.when(() -> ImageIO.read(any(InputStream.class)))
.thenReturn(mockPngImage);

List<ColorInfo> result = colorExtractionService.extractColorsFromUrl(TEST_URL);

assertEquals(1, result.size());
assertEquals("#20E0E0", result.get(0).getHexValue()); // Quantized Cyan
assertTrue(result.get(0).isLogoColor());
// Commons Imaging should not be called for a .png
mockedImaging.verify(() -> org.apache.commons.imaging.Imaging.getBufferedImage(any(InputStream.class)), never());
mockedImageIO.verify(() -> ImageIO.read(any(InputStream.class)), times(1));
}
}

@Test
void testFrequencyCountingAndTopN() throws IOException {
Expand Down