Skip to content

Commit

Permalink
getPages() should only ever return elements of type 'Page' (#350)
Browse files Browse the repository at this point in the history
* getPages() should only ever return elements of type 'Pages' or 'Page'

* added @todo to getPages()

* added sample file and test case
  • Loading branch information
Connum authored Oct 12, 2020
1 parent 72a8778 commit c9c2bed
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
Binary file added samples/bugs/Issue331.pdf
Binary file not shown.
5 changes: 4 additions & 1 deletion src/Smalot/PdfParser/Pages.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ class Pages extends PDFObject
/**
* @param bool $deep
*
* @todo Objects other than Pages or Page might need to be treated specifically in order to get Page objects out of them,
* see https://github.com/smalot/pdfparser/issues/331
*
* @return array
*/
public function getPages($deep = false)
Expand All @@ -56,7 +59,7 @@ public function getPages($deep = false)
foreach ($kids as $kid) {
if ($kid instanceof self) {
$pages = array_merge($pages, $kid->getPages(true));
} else {
} elseif ($kid instanceof Page) {
$pages[] = $kid;
}
}
Expand Down
26 changes: 26 additions & 0 deletions tests/Integration/PageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,32 @@ public function testGetDataTmIssue336()
$this->assertEquals('Lorem', $item[1]);
}

/**
* Tests that getPages() only returns Page objects
*
* @see https://github.com/smalot/pdfparser/issues/331
*
* Sample pdf file provided by @Reqrefusion, see
* https://github.com/smalot/pdfparser/pull/350#issuecomment-703195220
*/
public function testGetPages()
{
$filename = $this->rootDir.'/samples/bugs/Issue331.pdf';
$document = $this->getParserInstance()->parseFile($filename);
$pages = $document->getPages();

// This should actually be 3 pages, but as long as the cause for issue #331
// has not been found and the issue is not fixed, we'll settle for 2 here.
// We still test for the count, so in case the bug should be fixed
// unknowingly, we don't forget to resolve the issue as well and make sure
// this assertion is present.
$this->assertCount(2, $pages);

foreach ($pages as $page) {
$this->assertTrue($page instanceof Page);
}
}

public function testGetTextXY()
{
// Document with text.
Expand Down

0 comments on commit c9c2bed

Please sign in to comment.