-
Notifications
You must be signed in to change notification settings - Fork 96
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
Return table start PDPage from drawer #115
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package org.vandeseer.easytable; | ||
|
||
import lombok.AccessLevel; | ||
import lombok.Getter; | ||
import lombok.Setter; | ||
import lombok.experimental.Accessors; | ||
|
@@ -54,6 +55,13 @@ public class TableDrawer { | |
@Accessors(chain = true, fluent = true) | ||
protected boolean compress; | ||
|
||
@Getter | ||
protected PDPage tableStartPage; | ||
|
||
@Getter (AccessLevel.NONE) | ||
protected boolean startTableInNewPage; | ||
|
||
|
||
protected final List<BiConsumer<Drawer, DrawingContext>> drawerList = new LinkedList<>(); | ||
{ | ||
this.drawerList.add((drawer, drawingContext) -> { | ||
|
@@ -96,7 +104,7 @@ protected Queue<PageData> computeRowsOnPagesWithNewPageStartOf(float yOffsetOnNe | |
if (isRowTooHighToBeDrawnOnPage(row, yOffsetOnNewPage)) { | ||
throw new RowIsTooHighException("There is a row that is too high to be drawn on a single page"); | ||
} | ||
|
||
if (isNotDrawableOnPage(y, row)) { | ||
dataForPages.add(new PageData(firstRowOnPage, lastRowOnPage)); | ||
y = yOffsetOnNewPage; | ||
|
@@ -116,16 +124,32 @@ protected Queue<PageData> computeRowsOnPagesWithNewPageStartOf(float yOffsetOnNe | |
private boolean isRowTooHighToBeDrawnOnPage(Row row, float yOffsetOnNewPage) { | ||
return row.getHeight() > (yOffsetOnNewPage - endY); | ||
} | ||
|
||
protected void determinePageToStartTable(float yOffsetOnNewPage) { | ||
if (startY - table.getRows().get(0).getHeight() < endY) { | ||
startY = yOffsetOnNewPage; | ||
startTableInNewPage = true; | ||
} | ||
} | ||
|
||
public void draw(Supplier<PDDocument> documentSupplier, Supplier<PDPage> pageSupplier, float yOffset) throws IOException { | ||
final PDDocument document = documentSupplier.get(); | ||
|
||
// We create one throwaway page to be able to calculate the page data upfront | ||
float startOnNewPage = pageSupplier.get().getMediaBox().getHeight() - yOffset; | ||
determinePageToStartTable(startOnNewPage); | ||
final Queue<PageData> pageDataQueue = computeRowsOnPagesWithNewPageStartOf(startOnNewPage); | ||
|
||
for (int i = 0; !pageDataQueue.isEmpty(); i++) { | ||
final PDPage pageToDrawOn = determinePageToDraw(i, document, pageSupplier); | ||
|
||
if ((i == 0 && startTableInNewPage) || i > 0 || document.getNumberOfPages() == 0) { | ||
startTableInNewPage = false; | ||
} | ||
|
||
if (i == 0) { | ||
tableStartPage = pageToDrawOn; | ||
} | ||
|
||
try (final PDPageContentStream newPageContentStream = new PDPageContentStream(document, pageToDrawOn, APPEND, compress)) { | ||
this.contentStream(newPageContentStream) | ||
|
@@ -139,11 +163,13 @@ public void draw(Supplier<PDDocument> documentSupplier, Supplier<PDPage> pageSup | |
|
||
protected PDPage determinePageToDraw(int index, PDDocument document, Supplier<PDPage> pageSupplier) { | ||
final PDPage pageToDrawOn; | ||
if (index > 0 || document.getNumberOfPages() == 0) { | ||
|
||
if ((index == 0 && startTableInNewPage) || index > 0 || document.getNumberOfPages() == 0) { | ||
pageToDrawOn = pageSupplier.get(); | ||
document.addPage(pageToDrawOn); | ||
} else | ||
pageToDrawOn = document.getPage(document.getNumberOfPages() - 1); | ||
} else { | ||
pageToDrawOn = document.getPage(document.getNumberOfPages() - 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add brackets around the bodies of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. |
||
} | ||
return pageToDrawOn; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
package org.vandeseer.integrationtest; | ||
|
||
import static junit.framework.TestCase.assertTrue; | ||
import static junit.framework.TestCase.assertEquals; | ||
import static org.apache.pdfbox.pdmodel.PDPageContentStream.AppendMode.APPEND; | ||
import static org.vandeseer.TestUtils.getActualPdfFor; | ||
import static org.vandeseer.TestUtils.getExpectedPdfFor; | ||
import static org.apache.pdfbox.pdmodel.font.PDType1Font.HELVETICA; | ||
|
||
import java.io.IOException; | ||
|
||
import org.apache.pdfbox.pdmodel.PDDocument; | ||
import org.apache.pdfbox.pdmodel.PDPage; | ||
import org.apache.pdfbox.pdmodel.PDPageContentStream; | ||
import org.apache.pdfbox.pdmodel.common.PDRectangle; | ||
import org.apache.pdfbox.pdmodel.font.PDType1Font; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.vandeseer.TestUtils; | ||
import org.vandeseer.easytable.TableDrawer; | ||
import org.vandeseer.easytable.structure.Row; | ||
import org.vandeseer.easytable.structure.Table; | ||
import org.vandeseer.easytable.structure.Table.TableBuilder; | ||
import org.vandeseer.easytable.structure.cell.TextCell; | ||
|
||
import de.redsix.pdfcompare.CompareResult; | ||
import de.redsix.pdfcompare.PdfComparator; | ||
|
||
public class StartPageTest { | ||
|
||
private static final String FULL_TABLE_EXISTING_PAGE_FILE_NAME = "fullTableExistingPage.pdf"; | ||
private static final String START_TABLE_EXISTING_PAGE_FILE_NAME = "startTableExistingPage.pdf"; | ||
private static final String START_TABLE_NEW_PAGE_FILE_NAME = "startTableNewPage.pdf"; | ||
|
||
private Table table; | ||
|
||
private PDDocument document; | ||
|
||
private PDPage currentPage; | ||
|
||
private TableDrawer drawer; | ||
|
||
@Before | ||
public void before() throws IOException { | ||
TestUtils.assertRegressionFolderExists(); | ||
|
||
document = new PDDocument(); | ||
currentPage = new PDPage(PDRectangle.A4); | ||
document.addPage(currentPage); | ||
|
||
try (final PDPageContentStream content = new PDPageContentStream(document, currentPage, APPEND, false)) { | ||
content.beginText(); | ||
content.setFont(PDType1Font.TIMES_ROMAN, 15); | ||
content.newLineAtOffset(50, 500); | ||
content.showText("This line is added to ensure table is drawn on new page when space not available."); | ||
content.endText(); | ||
content.close(); | ||
|
||
TableBuilder builder = Table.builder().addColumnsOfWidth(150, 150, 150).fontSize(25).font(HELVETICA) | ||
.padding(5).borderWidth(1) | ||
.addRow(Row.builder().add(TextCell.builder().text("Header").build()) | ||
.add(TextCell.builder().text("of").build()).add(TextCell.builder().text("Table").build()) | ||
.build()); | ||
|
||
for (int i = 1; i < 5; i++) { | ||
builder.addRow(Row.builder().add(TextCell.builder().text("Row " + i).build()) | ||
.add(TextCell.builder().text("of").build()).add(TextCell.builder().text("Table").build()) | ||
.build()).build(); | ||
} | ||
table = builder.build(); | ||
} | ||
} | ||
|
||
private void createTable(float startY, String outputFileName) throws IOException { | ||
|
||
drawer = TableDrawer.builder().table(table).startX(50f).startY(startY).endY(50F).build(); | ||
|
||
drawer.draw(() -> document, () -> new PDPage(PDRectangle.A4), 50f); | ||
|
||
document.save(TestUtils.TARGET_FOLDER + "/" + outputFileName); | ||
document.close(); | ||
|
||
final CompareResult compareResult = new PdfComparator<>(getExpectedPdfFor(outputFileName), | ||
getActualPdfFor(outputFileName)).compare(); | ||
|
||
assertTrue(compareResult.isEqual()); | ||
} | ||
|
||
@Test | ||
public void createCompleteTableInExistingPage() throws IOException { | ||
createTable(200f, FULL_TABLE_EXISTING_PAGE_FILE_NAME); | ||
assertEquals(currentPage, drawer.getTableStartPage()); | ||
} | ||
|
||
@Test | ||
public void createSplitTableStartInExistingPage() throws IOException { | ||
createTable(120f, START_TABLE_EXISTING_PAGE_FILE_NAME); | ||
assertEquals(currentPage, drawer.getTableStartPage()); | ||
} | ||
|
||
@Test | ||
public void createTableStartInNewPage() throws IOException { | ||
createTable(70f, START_TABLE_NEW_PAGE_FILE_NAME); | ||
assertEquals(document.getPage(document.getNumberOfPages() - 1), drawer.getTableStartPage()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the returned value used at all? So either return the value and assign it where it is called (preferably) or just make the method
void
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The returned PDPage is used by the draw(Supplier documentSupplier, Supplier pageSupplier, float yOffset) method. Not sure I got your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my bad. You are right. Still, there is a thing which I don't like about this method and which should be easy to fix: There is a side effect, namely setting the member variable
tableStartPage
within the method. At the same time this value is returned. Since it is called only once one could argue that it would make more sense to set the member variable after the method is called using its returned value.(There is a 2nd side effect nonetheless though which is setting the member variable
startTableInNewPage
– but the fewer side effects the better in my opinion.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Point taken. Have moved both the if conditions into the parent draw method.