avstudnitz / AvS_FastSimpleImport

Wrapper for Magento ImportExport functionality, which imports products and customers from arrays
306 stars 146 forks source link

Shell importing products with images does not work with PHP7 #313

Open peterjaap opened 8 years ago

peterjaap commented 8 years ago

When running M1 on PHP7 and using Inchoo_PHP7 extension to fix the bugs, the shell importer breaks when importing products with images.

This is because this extension does not use the Magento getModel command to get the Mage_ImportExport_Model_Import_Uploader class, but references it directly, therefor skipping the Inchoo_PHP7 rewrite to fix the class for PHP7;

https://github.com/avstudnitz/AvS_FastSimpleImport/blob/master/src/app/code/community/AvS/FastSimpleImport/Model/Import/Entity/Product.php#L1349 https://github.com/avstudnitz/AvS_FastSimpleImport/blob/master/src/app/code/community/AvS/FastSimpleImport/Model/Import/Entity/Category.php#L667

paales commented 8 years ago

@peterjaap This is required, it was already attemted (https://github.com/avstudnitz/AvS_FastSimpleImport/commit/f29edebe5c48d2c67848227f049e6fbb37ac886c ), but is reverted: (https://github.com/avstudnitz/AvS_FastSimpleImport/commit/2ab2f535439bc5ac8799e2608d0c37d27f41d2ea)

This is because getModel assumes the argument of the constructor is an array, but that wont work for the mentioned class. Consider it a corebug ;)

peterjaap commented 8 years ago

So maybe rewrite the Mage_ImportExport_Model_Import_Uploader to a class in this extension, add the PHP7 fix and change the _setUploadFile function from;

    protected function _setUploadFile($filePath)
    {
        if (!is_readable($filePath)) {
            Mage::throwException("File '{$filePath}' was not found or has read restriction.");
        }
        $this->_file = $this->_readFileInfo($filePath);

        $this->_validateFile();
    }

to

    protected function _setUploadFile($filePath)
    {
        if($filePath === null) {
            return;
        }
        if (!is_readable($filePath)) {
            Mage::throwException("File '{$filePath}' was not found or has read restriction.");
        }
        $this->_file = $this->_readFileInfo($filePath);

        $this->_validateFile();
    }

?

paales commented 8 years ago

@peterjaap I'd rather not rewrite this file if not necessary, what PHP7 bug are you running into, could you post a stack trace.

peterjaap commented 8 years ago

It's the curly brackets fix on this line; https://github.com/Inchoo/Inchoo_PHP7/blob/master/app/code/local/Inchoo/PHP7/Model/Import/Uploader.php#L25

hostep commented 8 years ago

@paales: if you pass null as the second param, would that fix the constructor problem?

$this->_fileUploader = Mage::getModel("importexport/import_uploader", null);

They also fixed it like that in the Inchoo_PHP7 module, so I assume it works: https://github.com/Inchoo/Inchoo_PHP7/blob/82cfec98fb1461c16069261fef98c4edf1d821db/app/code/local/Inchoo/PHP7/Model/Import/Entity/Product.php#L16 (this was done a couple of months after @peterjaap opened this issue btw)