Skip to content
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

IOFactory::identify() returns wrong value when there's a custom reader registered #4357

Open
marrch-caat opened this issue Feb 12, 2025 · 1 comment · May be fixed by #4361
Open

IOFactory::identify() returns wrong value when there's a custom reader registered #4357

marrch-caat opened this issue Feb 12, 2025 · 1 comment · May be fixed by #4361

Comments

@marrch-caat
Copy link

When I create a custom reader class, say EchXlsxReader, and register it with registerReader() method, identify method behaves incorrect, returning the class name instead of the type for which it was registered. According to official docs, https://phpspreadsheet.readthedocs.io/en/latest/topics/reading-files/, the following code should work:

/**  Identify the type of $inputFileName  **/for 
$inputFileType = \PhpOffice\PhpSpreadsheet\IOFactory::identify($inputFileName);
/**  Create a new Reader of the type that has been identified  **/
$reader = \PhpOffice\PhpSpreadsheet\IOFactory::createReader($inputFileType);
/**  Load $inputFileName to a Spreadsheet Object  **/
$spreadsheet = $reader->load($inputFileName);

But that really doesn't work:

// I register a custom reader to use for XLSX files innstead of standard one.
IOFactory::registerReader(IOFactory::READER_XLSX, EchXlsxReader::class);
...
// The following code will work fine:
$objPHPExcel = IOFactory::load($inputFileName);

// But the code from the mentioned sample fails:
$inputFileType = IOFactory::identify($inputFileName); //Successfully creates the reader and returns "EchXlsxReader"
$objReader = IOFactory::createReader($inputFileType); //Throws error, "No reader found for type EchXlsxReader"

According to the docs the behavior is incorrect, it works for built-in readers just because their (non-qualified) names are equal to the names of the types they read.

@oleibman
Copy link
Collaborator

The problem seems to be that identify just returns the last node of the class name rather than the full name. I think a change to this behavior would be a breaking change. However, I can add a parameter to identify allowing it to return the full name, and then change the documentation to use the new parameter. Look for a change along this line in the next day or so.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Feb 13, 2025
Fix PHPOffice#4357.`identify` returns a type, not a class name, and this result is not always usable for createReader. The docs say that it is usable in that manner. Changing the behavior of `identify` would be a breaking change, but adding an optional parameter (defaulting to current behavior) allowing it to return a class name rather than a type would not. This PR adds that parameter and updates the documentation to suggest its use for the code in question.

To complete this change, `createReader` now accepts either a type (current behavior) or class name (new). Although it is not particularly identified as a possible problem by the issue, `createWriter` is similarly changed for consistency's sake.
@oleibman oleibman linked a pull request Feb 13, 2025 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants