CsvBulkLoader::processRecord() - Code Metrics - Inspection of "Fix additional missing items" - i-lateral/silverstripe-slightly-better-bulkloader - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — 1.0 ( 2d5e19...3aa83c )

by Morven

created 2019-02-08 19:01 UTC

CsvBulkLoader::processRecord() B

↳ Parent: CsvBulkLoader

Complexity

Conditions	9
Paths	18

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	44
rs	7.6604
c	0
b	0
f	0
cc	9
nc	18
nop	4

<?php

namespace ilateral\SilverStripe\SlightlyBetterBulkLoader;

use SilverStripe\Control\Director;
use SilverStripe\Dev\CsvBulkLoader as SS_CsvBulkLoader;

/**
 * Custom CSV importer that removes/de-duplicates blank header columns and also
 * tracks errors while importing.
 */
class CsvBulkLoader extends SS_CsvBulkLoader
{
    /**
     * @param string $filepath
     * @param boolean $preview
     *
     * @return null|BulkLoader_Result
     */
    protected function processAll($filepath, $preview = false)
    {
        $previousDetectLE = ini_get('auto_detect_line_endings');
        ini_set('auto_detect_line_endings', true);

        $result = BulkLoader_Result::create();

        try {
            $filepath = Director::getAbsFile($filepath);
            $csvReader = CustomReader::createFromPath($filepath, 'r');

            $tabExtractor = function ($row, $rowOffset, $iterator) {

                foreach ($row as &$item) {
                    // [SS-2017-007] Ensure all cells with leading tab and then [@=+] have the tab removed on import
                    if (preg_match("/^\t[\-@=\+]+.*/", $item)) {
                        $item = ltrim($item, "\t");
                    }
                }
                return $row;
            };

            if (isset($this->columnMap) && count($this->columnMap)) {
                $headerMap = $this->getNormalisedColumnMap();
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
                $remapper = function ($row, $rowOffset, $iterator) use ($headerMap, $tabExtractor) {
                    $row = $tabExtractor($row, $rowOffset, $iterator);
                    foreach ($headerMap as $column => $renamedColumn) {
                        if ($column == $renamedColumn) {
                            continue;
                        }
                        if (array_key_exists($column, $row)) {
                            if (strpos($renamedColumn, '_ignore_') !== 0) {
                                $row[$renamedColumn] = $row[$column];
                            }
                            unset($row[$column]);
                        }
                    }
                    return $row;
                };
            } else {
                $remapper = $tabExtractor;
            }

            $rows = null;

            if ($this->hasHeaderRow) {
                $rows = $csvReader->fetchAssoc(0, $remapper);
            } elseif ($this->columnMap) {
                $rows = $csvReader->fetchAssoc($headerMap, $remapper);
function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}
            }

            if (!empty($rows)) {
                foreach ($rows as $row) {
                    $this->processRecord($row, $this->columnMap, $result, $preview);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}
                }
            }
        } catch (\Exception $e) {
            $failedMessage = sprintf("Failed to parse %s", $filepath);
            if (Director::isDev()) {
                $failedMessage = sprintf($failedMessage . " because %s", $e->getMessage());
            }
            $result->addError($failedMessage);
        } finally {
            ini_set('auto_detect_line_endings', $previousDetectLE);
        }

        return $result;
    }

    /**
     * Process a single record
     *
     * @todo Better messages for relation checks and duplicate detection
     * Note that columnMap isn't used.
     *
     * @param array $record
     * @param array $columnMap
     * @param BulkLoader_Result $results
     * @param boolean $preview
     *
     * @return int

     */
    protected function processRecord($record, $columnMap, &$results, $preview = false)
    {
        $required = $this->getRequiredFields();
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
        $current_row = $results->getTotal() + 1;
        $obj = singleton($this->objectClass);
        $missing = [];

        foreach ($required as $field) {
            $valid = false;
            $label = $obj->fieldLabel($field);

            // Is the field label used instead of the field name and is it set?
            if (isset($record[$label]) && !empty($record[$label])) {
                $valid = true;
            }

            // Is required data missing? If so track an error
            if (!$valid && (isset($record[$field]) && !empty($record[$field]))) {
                $valid = true;
            }

            if (!$valid) {
                $missing[] = $label . "/" . $field;
            }
        }

        // If we have missing data, add an error
        if (count($missing) > 0) {
            $results->addError(
                _t(
                    __CLASS__ . '.Required',
                    'Required fields "{fields}" not set on row "{row}"',
                    [
                        'fields' => implode(", ", $missing),
                        'row' => $current_row
                    ]
                )
            );
            return null;
        }

        // If validation passed, process as usual
        return parent::processRecord($record, $columnMap, $results, $preview);
    }
}


1			<?php
2
3			namespace ilateral\SilverStripe\SlightlyBetterBulkLoader;
4
5			use SilverStripe\Control\Director;
6			use SilverStripe\Dev\CsvBulkLoader as SS_CsvBulkLoader;
7
8			/**
9			* Custom CSV importer that removes/de-duplicates blank header columns and also
10			* tracks errors while importing.
11			*/
12			class CsvBulkLoader extends SS_CsvBulkLoader
13			{
14			/**
15			* @param string $filepath
16			* @param boolean $preview
17			*
18			* @return null\|BulkLoader_Result
19			*/
20			protected function processAll($filepath, $preview = false)
21			{
22			$previousDetectLE = ini_get('auto_detect_line_endings');
23			ini_set('auto_detect_line_endings', true);
24
25			$result = BulkLoader_Result::create();
26
27			try {
28			$filepath = Director::getAbsFile($filepath);
29			$csvReader = CustomReader::createFromPath($filepath, 'r');
30
31			$tabExtractor = function ($row, $rowOffset, $iterator) {
			0 ignored issues – show Unused Code introduced 2019-02-08 12:23 UTC by Report Bug Copy Issue Report The parameter `$rowOffset` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history... Unused Code introduced 2019-02-08 12:23 UTC by Report Bug Copy Issue Report The parameter `$iterator` is not used and could be removed. This check looks from parameters that have been defined for a function or method, but which are not used in the method body. Loading history...
32			foreach ($row as &$item) {
33			// [SS-2017-007] Ensure all cells with leading tab and then [@=+] have the tab removed on import
34			if (preg_match("/^\t[\-@=\+]+.*/", $item)) {
35			$item = ltrim($item, "\t");
36			}
37			}
38			return $row;
39			};
40
41			if (isset($this->columnMap) && count($this->columnMap)) {
42			$headerMap = $this->getNormalisedColumnMap();
			0 ignored issues – show Documentation Bug introduced 2019-02-08 12:23 UTC by Report Bug Copy Issue Report The method `getNormalisedColumnMap` does not exist on `object<ilateral\SilverSt...lkLoader\CsvBulkLoader>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
43			$remapper = function ($row, $rowOffset, $iterator) use ($headerMap, $tabExtractor) {
44			$row = $tabExtractor($row, $rowOffset, $iterator);
45			foreach ($headerMap as $column => $renamedColumn) {
46			if ($column == $renamedColumn) {
47			continue;
48			}
49			if (array_key_exists($column, $row)) {
50			if (strpos($renamedColumn, '_ignore_') !== 0) {
51			$row[$renamedColumn] = $row[$column];
52			}
53			unset($row[$column]);
54			}
55			}
56			return $row;
57			};
58			} else {
59			$remapper = $tabExtractor;
60			}
61
62			$rows = null;
63
64			if ($this->hasHeaderRow) {
65			$rows = $csvReader->fetchAssoc(0, $remapper);
66			} elseif ($this->columnMap) {
67			$rows = $csvReader->fetchAssoc($headerMap, $remapper);
			0 ignored issues – show Bug introduced 2019-02-08 12:23 UTC by Report Bug Copy Issue Report The variable `$headerMap` does not seem to be defined for all execution paths leading up to this point. If you define a variable conditionally, it can happen that it is not defined for all execution paths. Let’s take a look at an example: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } // $x is potentially undefined here. echo $x; } In the above example, the variable `$x` is defined if you pass “foo” or “bar” as argument for `$a`. However, since the `switch` statement has no default case statement, if you pass any other value, the variable `$x` would be undefined. Available Fixes Check for existence of the variable explicitly: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } if (isset($x)) { // Make sure it's always set. echo $x; } } Define a default value for the variable: function myFunction($a) { $x = ''; // Set a default which gets overridden for certain paths. switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; } echo $x; } Add a value for the missing path: function myFunction($a) { switch ($a) { case 'foo': $x = 1; break; case 'bar': $x = 2; break; // We add support for the missing case. default: $x = ''; break; } echo $x; } Loading history...
68			}
69
70			if (!empty($rows)) {
71			foreach ($rows as $row) {
72			$this->processRecord($row, $this->columnMap, $result, $preview);
			0 ignored issues – show Bug introduced 2019-02-08 13:24 UTC by Report Bug Copy Issue Report It seems like `$this->columnMap` can also be of type `null`; however, `ilateral\SilverStripe\Sl...Loader::processRecord()` does only seem to accept `array`, maybe add an additional type check? If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /** * @return array\|string */ function returnsDifferentValues($x) { if ($x) { return 'foo'; } return array(); } $x = returnsDifferentValues($y); if (is_array($x)) { // $x is an array. } If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
73			}
74			}
75			} catch (\Exception $e) {
76			$failedMessage = sprintf("Failed to parse %s", $filepath);
77			if (Director::isDev()) {
78			$failedMessage = sprintf($failedMessage . " because %s", $e->getMessage());
79			}
80			$result->addError($failedMessage);
81			} finally {
82			ini_set('auto_detect_line_endings', $previousDetectLE);
83			}
84
85			return $result;
86			}
87
88			/**
89			* Process a single record
90			*
91			* @todo Better messages for relation checks and duplicate detection
92			* Note that columnMap isn't used.
93			*
94			* @param array $record
95			* @param array $columnMap
96			* @param BulkLoader_Result $results
97			* @param boolean $preview
98			*
99			* @return int
			0 ignored issues – show Documentation introduced 2019-02-08 12:23 UTC by Report Bug Copy Issue Report Should the return type not be `null\|integer`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
100			*/
101			protected function processRecord($record, $columnMap, &$results, $preview = false)
102			{
103			$required = $this->getRequiredFields();
			0 ignored issues – show Documentation Bug introduced 2019-02-08 12:23 UTC by Report Bug Copy Issue Report The method `getRequiredFields` does not exist on `object<ilateral\SilverSt...lkLoader\CsvBulkLoader>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
104			$current_row = $results->getTotal() + 1;
105			$obj = singleton($this->objectClass);
106			$missing = [];
107
108			foreach ($required as $field) {
109			$valid = false;
110			$label = $obj->fieldLabel($field);
111
112			// Is the field label used instead of the field name and is it set?
113			if (isset($record[$label]) && !empty($record[$label])) {
114			$valid = true;
115			}
116
117			// Is required data missing? If so track an error
118			if (!$valid && (isset($record[$field]) && !empty($record[$field]))) {
119			$valid = true;
120			}
121
122			if (!$valid) {
123			$missing[] = $label . "/" . $field;
124			}
125			}
126
127			// If we have missing data, add an error
128			if (count($missing) > 0) {
129			$results->addError(
130			_t(
131			__CLASS__ . '.Required',
132			'Required fields "{fields}" not set on row "{row}"',
133			[
134			'fields' => implode(", ", $missing),
135			'row' => $current_row
136			]
137			)
138			);
139			return null;
140			}
141
142			// If validation passed, process as usual
143			return parent::processRecord($record, $columnMap, $results, $preview);
144			}
145			}
146

i-lateral / silverstripe-slightly-better-bulkloader

Push — 1.0 ( 2d5e19...3aa83c )

CsvBulkLoader::processRecord() B

Complexity

Size

Duplication

Importance

Available Fixes

Duplication Side-by-Side

Filter issues like