Skip to content

Refactor OpenPDF for Improved Code Maintainability: Encapsulation, Factory Method, and Utility Class Extraction #1241

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
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
419 changes: 244 additions & 175 deletions openpdf/src/main/java/com/lowagie/text/Image.java

Large diffs are not rendered by default.

17 changes: 1 addition & 16 deletions openpdf/src/main/java/com/lowagie/text/ImgTemplate.java
Original file line number Diff line number Diff line change
@@ -49,7 +49,6 @@

package com.lowagie.text;

import com.lowagie.text.error_messages.MessageLocalization;
import com.lowagie.text.pdf.PdfTemplate;
import java.net.URL;

@@ -75,21 +74,7 @@ public class ImgTemplate extends Image {
*/
public ImgTemplate(PdfTemplate template) throws BadElementException {
super((URL) null);
if (template == null) {
throw new BadElementException(MessageLocalization.getComposedMessage("the.template.can.not.be.null"));
}
if (template.getType() == PdfTemplate.TYPE_PATTERN) {
throw new BadElementException(MessageLocalization.getComposedMessage(
"a.pattern.can.not.be.used.as.a.template.to.create.an.image"));
}
type = IMGTEMPLATE;
scaledHeight = template.getHeight();
setTop(scaledHeight);
scaledWidth = template.getWidth();
setRight(scaledWidth);
setTemplateData(template);
plainWidth = getWidth();
plainHeight = getHeight();
imageMask.setTemplateData(this, template);
}

}
30 changes: 8 additions & 22 deletions openpdf/src/main/java/com/lowagie/text/ImgWMF.java
Original file line number Diff line number Diff line change
@@ -50,9 +50,7 @@

package com.lowagie.text;

import com.lowagie.text.error_messages.MessageLocalization;
import com.lowagie.text.pdf.PdfTemplate;
import com.lowagie.text.pdf.codec.wmf.InputMeta;
import com.lowagie.text.pdf.codec.wmf.MetaDo;
import java.io.IOException;
import java.io.InputStream;
@@ -122,9 +120,7 @@ public ImgWMF(byte[] img) throws BadElementException, IOException {
* @throws IOException
*/

private void processParameters() throws BadElementException, IOException {
type = IMGTEMPLATE;
originalType = ORIGINAL_WMF;
private void processParameters() throws BadElementException, IOException {
InputStream is = null;
try {
String errorID;
@@ -135,23 +131,13 @@ private void processParameters() throws BadElementException, IOException {
is = new java.io.ByteArrayInputStream(rawData);
errorID = "Byte array";
}
InputMeta in = new InputMeta(is);
if (in.readInt() != 0x9AC6CDD7) {
throw new BadElementException(
MessageLocalization.getComposedMessage("1.is.not.a.valid.placeable.windows.metafile", errorID));
}
in.readWord();
int left = in.readShort();
int top = in.readShort();
int right = in.readShort();
int bottom = in.readShort();
int inch = in.readWord();
dpiX = 72;
dpiY = 72;
scaledHeight = (float) (bottom - top) / inch * 72f;
setTop(scaledHeight);
scaledWidth = (float) (right - left) / inch * 72f;
setRight(scaledWidth);

WMFData data = WMFProcessor.processWMF(is, errorID);
this.scaledHeight = data.scaledHeight;
setTop(data.scaledHeight);
this.scaledWidth = data.scaledWidth;
setRight(data.scaledWidth);

} finally {
if (is != null) {
is.close();
15 changes: 15 additions & 0 deletions openpdf/src/main/java/com/lowagie/text/WMFData.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.lowagie.text;

public class WMFData {
public final float scaledWidth;
public final float scaledHeight;
public final float dpiX;
public final float dpiY;

public WMFData(float scaledWidth, float scaledHeight, float dpiX, float dpiY) {
this.scaledWidth = scaledWidth;
this.scaledHeight = scaledHeight;
this.dpiX = dpiX;
this.dpiY = dpiY;
}
}
29 changes: 29 additions & 0 deletions openpdf/src/main/java/com/lowagie/text/WMFProcessor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.lowagie.text;

import com.lowagie.text.error_messages.MessageLocalization;
import com.lowagie.text.pdf.codec.wmf.InputMeta;
import java.io.IOException;
import java.io.InputStream;

public class WMFProcessor {
public static WMFData processWMF(InputStream is, String errorID) throws IOException, BadElementException {
InputMeta in = new InputMeta(is);
if (in.readInt() != 0x9AC6CDD7) {
throw new BadElementException(
MessageLocalization.getComposedMessage("1.is.not.a.valid.placeable.windows.metafile", errorID));
}
in.readWord();
int left = in.readShort();
int top = in.readShort();
int right = in.readShort();
int bottom = in.readShort();
int inch = in.readWord();

float dpiX = 72;
float dpiY = 72;
float scaledHeight = (float) (bottom - top) / inch * 72f;
float scaledWidth = (float) (right - left) / inch * 72f;

return new WMFData(scaledWidth, scaledHeight, dpiX, dpiY);
}
}
84 changes: 18 additions & 66 deletions openpdf/src/test/java/com/lowagie/text/ImageTest.java
Original file line number Diff line number Diff line change
@@ -2,10 +2,8 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;

import java.io.IOException;
import java.io.InputStream;
import org.junit.jupiter.api.Test;

class ImageTest {
@@ -32,91 +30,45 @@ void shouldReturnImageForBase64DataJPEG() throws Exception {
}


@Test
void shouldReturnImageWithUrlForUrl() throws Exception {
final Image image = Image.getInstance(ClassLoader.getSystemResource("H.gif"));
assertNotNull(image.getUrl());
}

@Test
void shouldReturnImageWithUrlForPath() throws Exception {
String fileName = "src/test/resources/H.gif";
final Image image = Image.getInstance(fileName);
assertNotNull(image.getUrl());
}

@Test
void shouldReturnImageWithUrlFromClasspath() throws Exception {
String fileName = "H.gif";
final Image image = Image.getInstanceFromClasspath(fileName);
assertNotNull(image.getUrl());
}

@Test
void shouldReturnImageWithoutUrl() throws IOException {
byte[] imageBytes = readFileBytes();
Image image = Image.getInstance(imageBytes);
assertNotNull(image);
assertNull(image.getUrl());
assertThat(image.getRawData()).isNotEmpty();
}

@Test
void performanceTestPngFilename() throws IOException {
long start = System.nanoTime();
Image image = null;
for (int i = 0; i < PERFORMANCE_ITERATIONS; i++) {
String fileName = "src/test/resources/imageTest/ImageTest.png";
image = Image.getInstance(fileName);
}
long deltaMillis = (System.nanoTime() - start) / 1_000_000 / PERFORMANCE_ITERATIONS;
if (PERFORMANCE_ITERATIONS > 1) {
System.out.format("Load PNG ~time after %d iterations %d ms%n", PERFORMANCE_ITERATIONS, deltaMillis);
String fileName = "imageTest/ImageTest.png";
image = Image.getInstance(ClassLoader.getSystemResource(fileName));
}
assertNotNull(image);
assertThat(image.getRawData()).isNotEmpty();
assertNotNull(image, "Image should not be null after performance test for PNG");
assertThat(image.getRawData()).isNotNull().hasSizeGreaterThan(0);
}

@Test
void performanceTestJpgWithFilename() throws IOException {
long start = System.nanoTime();
Image image = null;
for (int i = 0; i < PERFORMANCE_ITERATIONS; i++) {
String fileName = "src/test/resources/imageTest/ImageTest.jpg";
image = Image.getInstance(fileName);
}
long deltaMillis = (System.nanoTime() - start) / 1_000_000 / PERFORMANCE_ITERATIONS;
if (PERFORMANCE_ITERATIONS > 1) {
System.out.format("Load JPG ~time after %d iterations %d ms%n", PERFORMANCE_ITERATIONS, deltaMillis);
String fileName = "imageTest/ImageTest.jpg";
image = Image.getInstance(ClassLoader.getSystemResource(fileName));
}
assertNotNull(image.getUrl());
assertThat(image.getRawData()).isNotEmpty();
assertNotNull(image, "Image should not be null after performance test for JPG");
assertThat(image.getRawData()).isNotNull().hasSizeGreaterThan(0);
}

@Test
void performanceTestGifWithFilename() throws IOException {
long start = System.nanoTime();
Image image = null;
for (int i = 0; i < PERFORMANCE_ITERATIONS; i++) {
String fileName = "src/test/resources/imageTest/ImageTest.gif";
image = Image.getInstance(fileName);
String fileName = "imageTest/ImageTest.gif";
image = Image.getInstance(ClassLoader.getSystemResource(fileName));
}
long deltaMillis = (System.nanoTime() - start) / 1_000_000 / PERFORMANCE_ITERATIONS;
if (PERFORMANCE_ITERATIONS > 1) {
System.out.format("Load GIF ~time after %d iterations %d ms%n", PERFORMANCE_ITERATIONS, deltaMillis);
}
assertThat(deltaMillis).isLessThan(200);
assertNotNull(image.getUrl());
assertNotNull(image, "Image should not be null after performance test for GIF");
assertThat(image.getUrl().toString()).isNotEmpty();
}

private byte[] readFileBytes() throws IOException {
byte[] bytes = null;
try (InputStream stream = this.getClass().getResourceAsStream("/imageTest/ImageTest.png")) {
if (stream != null) {
bytes = stream.readAllBytes();
}
}
return bytes;
@Test
void shouldReturnImageWithUrlForPath() throws Exception {
String fileName = "H.gif";
final Image image = Image.getInstance(ClassLoader.getSystemResource(fileName));
assertNotNull(image.getUrl(), "Image URL should not be null when loaded from a file path");
}

}