CsvBulkLoader::processRecord()   B
last analyzed

Complexity

Conditions 6
Paths 6

Size

Total Lines 32

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 32
rs 8.7857
c 0
b 0
f 0
cc 6
nc 6
nop 4
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 by
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 by
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 by
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 by
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

  1. 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;
        }
    }
    
  2. 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;
    }
    
  3. 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 by
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 by
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 by
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);
0 ignored issues
show
Unused Code introduced by
$obj is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
106
        $missing = [];
107
108
        foreach ($required as $field) {
109
            // Is required data missing? If so track an error
110
            if (!isset($record[$field]) || (isset($record[$field]) && empty($record[$field]))) {
111
                $missing[] = $field;
112
            }
113
        }
114
115
        // If we have missing data, add an error
116
        if (count($missing) > 0) {
117
            $results->addError(
118
                _t(
119
                    __CLASS__ . '.Required',
120
                    'Required fields "{fields}" not set on row "{row}"',
121
                    [
122
                        'fields' => implode(", ", $missing),
123
                        'row' => $current_row
124
                    ]
125
                )
126
            );
127
            return null;
128
        }
129
130
        // If validation passed, process as usual
131
        return parent::processRecord($record, $columnMap, $results, $preview);
132
    }
133
}
134