FileField - Code Metrics - Inspection of "API Refactor UploadField, FileField and AssetField..." - silverstripe/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#6325)

by Damian

created 2016-11-29 03:19 UTC

FileField A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	144
Duplicated Lines	0 %

Coupling/Cohesion

Components	1
Dependencies	8

Importance

Changes

Metric	Value
dl	0
loc	144
rs	10
c	0
b	0
f	0
wmc	21
lcom	1
cbo	8

8 Methods

Rating	Name	Size	Complexity
A	__construct()	5	1
A	Field()	8	1
A	getAttributes()	7	1
D	saveInto()	43	9
A	Value()	4	2
B	validate()	21	5
A	setRelationAutoSetting()	5	1
A	getRelationAutoSetting()	4	1

<?php

namespace SilverStripe\Forms;

use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DataObjectInterface;
use SilverStripe\Assets\File;
use SilverStripe\Core\Object;

/**
 * Represents a file type which can be added to a form.
 * Automatically tries to save has_one-relations on the saved
 * record.
 *
 * Please set a validator on the form-object to get feedback
 * about imposed filesize/extension restrictions.
 *
 * See {@link UploadField} For a more full-featured field
 * (incl. ajax-friendly uploads, previews and relationship management).
 *
 * <b>Usage</p>
 *
 * If you want to implement a FileField into a form element, you need to pass it an array of source data.
 *
 * <code>
 * class ExampleForm_Controller extends Page_Controller {
 *
 *  function Form() {
 *      $fields = new FieldList(
 *          new TextField('MyName'),
 *          new FileField('MyFile')
 *      );
 *      $actions = new FieldList(
 *          new FormAction('doUpload', 'Upload file')
 *      );
 *    $validator = new RequiredFields(array('MyName', 'MyFile'));
 *
 *      return new Form($this, 'Form', $fields, $actions, $validator);
 *  }
 *
 *  function doUpload($data, $form) {
 *      $file = $data['MyFile'];
 *      $content = file_get_contents($file['tmp_name']);
 *      // ... process content
 *  }
 * }
 * </code>
 */
class FileField extends FormField
{
    use UploadReceiver;

    /**
     * Flag to automatically determine and save a has_one-relationship
     * on the saved record (e.g. a "Player" has_one "PlayerImage" would
     * trigger saving the ID of newly created file into "PlayerImageID"
     * on the record).
     *
     * @var boolean
     */
    protected $relationAutoSetting = true;

    /**
     * Create a new file field.
     *
     * @param string $name The internal field name, passed to forms.
     * @param string $title The field label.
     * @param int $value The value of the field.
     */
    public function __construct($name, $title = null, $value = null)
    {
        $this->constructUploadReceiver();
        parent::__construct($name, $title, $value);
    }

    /**
     * @param array $properties
     * @return string
     */
    public function Field($properties = array())
    {
        $properties = array_merge($properties, array(
            'MaxFileSize' => $this->getValidator()->getAllowedMaxFileSize()
        ));

        return parent::Field($properties);
    }

    public function getAttributes()
    {
        return array_merge(
            parent::getAttributes(),
            array('type' => 'file')
        );
    }

    /**
     * @param DataObject|DataObjectInterface $record
     */
    public function saveInto(DataObjectInterface $record)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        if (!isset($_FILES[$this->name])) {
            return;
        }

        $fileClass = File::get_class_for_file_extension(
            File::get_file_extension($_FILES[$this->name]['name'])
        );

        /** @var File $file */
        if ($this->relationAutoSetting) {
            // assume that the file is connected via a has-one
            $objectClass = DataObject::getSchema()->hasOneComponent(get_class($record), $this->name);
            if ($objectClass === File::class || empty($objectClass)) {
                // Create object of the appropriate file class
                $file = Object::create($fileClass);
            } else {
                // try to create a file matching the relation
                $file = Object::create($objectClass);
            }
        } elseif ($record instanceof File) {
            $file = $record;
        } else {
            $file = Object::create($fileClass);
        }

        $this->upload->loadIntoFile($_FILES[$this->name], $file, $this->getFolderName());
/**
 * @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->upload->isError()) {
            return;
        }

        if ($this->relationAutoSetting) {
            if (empty($objectClass)) {
                return;
            }

            $file = $this->upload->getFile();

            $record->{$this->name . 'ID'} = $file->ID;
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}
        }
    }

    public function Value()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        return isset($_FILES[$this->getName()]) ? $_FILES[$this->getName()] : null;
    }

    public function validate($validator)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        if (!isset($_FILES[$this->name])) {
            return true;
        }

        $tmpFile = $_FILES[$this->name];

        $valid = $this->upload->validate($tmpFile);
        if (!$valid) {
            $errors = $this->upload->getErrors();
            if ($errors) {

                foreach ($errors as $error) {
                    $validator->validationError($this->name, $error, "validation");
                }
            }
            return false;
        }

        return true;
    }

    /**
     * Set if relation can be automatically assigned to the underlying dataobject
     *
     * @param bool $auto
     * @return $this
     */
    public function setRelationAutoSetting($auto)
    {
        $this->relationAutoSetting = $auto;
        return $this;
    }

    /**
     * Check if relation can be automatically assigned to the underlying dataobject
     *
     * @return bool
     */
    public function getRelationAutoSetting()
    {
        return $this->relationAutoSetting;
    }
}


1			<?php
2
3			namespace SilverStripe\Forms;
4
5			use SilverStripe\ORM\DataObject;
6			use SilverStripe\ORM\DataObjectInterface;
7			use SilverStripe\Assets\File;
8			use SilverStripe\Core\Object;
9
10			/**
11			* Represents a file type which can be added to a form.
12			* Automatically tries to save has_one-relations on the saved
13			* record.
14			*
15			* Please set a validator on the form-object to get feedback
16			* about imposed filesize/extension restrictions.
17			*
18			* See {@link UploadField} For a more full-featured field
19			* (incl. ajax-friendly uploads, previews and relationship management).
20			*
21			* <b>Usage</p>
22			*
23			* If you want to implement a FileField into a form element, you need to pass it an array of source data.
24			*
25			* <code>
26			* class ExampleForm_Controller extends Page_Controller {
27			*
28			* function Form() {
29			* $fields = new FieldList(
30			* new TextField('MyName'),
31			* new FileField('MyFile')
32			* );
33			* $actions = new FieldList(
34			* new FormAction('doUpload', 'Upload file')
35			* );
36			* $validator = new RequiredFields(array('MyName', 'MyFile'));
37			*
38			* return new Form($this, 'Form', $fields, $actions, $validator);
39			* }
40			*
41			* function doUpload($data, $form) {
42			* $file = $data['MyFile'];
43			* $content = file_get_contents($file['tmp_name']);
44			* // ... process content
45			* }
46			* }
47			* </code>
48			*/
49			class FileField extends FormField
50			{
51			use UploadReceiver;
52
53			/**
54			* Flag to automatically determine and save a has_one-relationship
55			* on the saved record (e.g. a "Player" has_one "PlayerImage" would
56			* trigger saving the ID of newly created file into "PlayerImageID"
57			* on the record).
58			*
59			* @var boolean
60			*/
61			protected $relationAutoSetting = true;
62
63			/**
64			* Create a new file field.
65			*
66			* @param string $name The internal field name, passed to forms.
67			* @param string $title The field label.
68			* @param int $value The value of the field.
69			*/
70			public function __construct($name, $title = null, $value = null)
71			{
72			$this->constructUploadReceiver();
73			parent::__construct($name, $title, $value);
74			}
75
76			/**
77			* @param array $properties
78			* @return string
79			*/
80			public function Field($properties = array())
81			{
82			$properties = array_merge($properties, array(
83			'MaxFileSize' => $this->getValidator()->getAllowedMaxFileSize()
84			));
85
86			return parent::Field($properties);
87			}
88
89			public function getAttributes()
90			{
91			return array_merge(
92			parent::getAttributes(),
93			array('type' => 'file')
94			);
95			}
96
97			/**
98			* @param DataObject\|DataObjectInterface $record
99			*/
100			public function saveInto(DataObjectInterface $record)
			0 ignored issues – show Coding Style introduced 2015-11-25 21:50 UTC by Report Bug Copy Issue Report `saveInto` uses the super-global variable `$_FILES` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
101			{
102			if (!isset($_FILES[$this->name])) {
103			return;
104			}
105
106			$fileClass = File::get_class_for_file_extension(
107			File::get_file_extension($_FILES[$this->name]['name'])
108			);
109
110			/** @var File $file */
111			if ($this->relationAutoSetting) {
112			// assume that the file is connected via a has-one
113			$objectClass = DataObject::getSchema()->hasOneComponent(get_class($record), $this->name);
114			if ($objectClass === File::class \|\| empty($objectClass)) {
115			// Create object of the appropriate file class
116			$file = Object::create($fileClass);
117			} else {
118			// try to create a file matching the relation
119			$file = Object::create($objectClass);
120			}
121			} elseif ($record instanceof File) {
122			$file = $record;
123			} else {
124			$file = Object::create($fileClass);
125			}
126
127			$this->upload->loadIntoFile($_FILES[$this->name], $file, $this->getFolderName());
			0 ignored issues – show Bug introduced 2016-11-01 00:55 UTC by Report Bug Copy Issue Report It seems like `$file` can also be of type `this<SilverStripe\Forms\FileField>`; however, `SilverStripe\Assets\Upload::loadIntoFile()` does only seem to accept `object<SilverStripe\Asse...ge\AssetContainer>\|null`, 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...
128
129			if ($this->upload->isError()) {
130			return;
131			}
132
133			if ($this->relationAutoSetting) {
134			if (empty($objectClass)) {
135			return;
136			}
137
138			$file = $this->upload->getFile();
139
140			$record->{$this->name . 'ID'} = $file->ID;
			0 ignored issues – show Bug introduced 2016-11-01 00:55 UTC by Report Bug Copy Issue Report Accessing `ID` on the interface `SilverStripe\Assets\Storage\AssetContainer` suggest that you code against a concrete implementation. How about adding an `instanceof` check? If you access a property on an interface, you most likely code against a concrete implementation of the interface. Available Fixes Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } Changing the type hint: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeClass $object) { $a = $object->a; } Loading history...
141			}
142			}
143
144			public function Value()
			0 ignored issues – show Coding Style introduced 2015-11-25 21:50 UTC by Report Bug Copy Issue Report `Value` uses the super-global variable `$_FILES` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
145			{
146			return isset($_FILES[$this->getName()]) ? $_FILES[$this->getName()] : null;
147			}
148
149			public function validate($validator)
			0 ignored issues – show Coding Style introduced 2015-11-25 21:50 UTC by Report Bug Copy Issue Report `validate` uses the super-global variable `$_FILES` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
150			{
151			if (!isset($_FILES[$this->name])) {
152			return true;
153			}
154
155			$tmpFile = $_FILES[$this->name];
156
157			$valid = $this->upload->validate($tmpFile);
158			if (!$valid) {
159			$errors = $this->upload->getErrors();
160			if ($errors) {
			0 ignored issues – show Bug Best Practice introduced 2016-11-28 05:56 UTC by Report Bug Copy Issue Report The expression `$errors` of type `array` is implicitly converted to a boolean; are you sure this is intended? If so, consider using `! empty($expr)` instead to make it clear that you intend to check for an array without elements. This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent. Consider making the comparison explicit by using `empty(..)` or `! empty(...)` instead. Loading history...
161			foreach ($errors as $error) {
162			$validator->validationError($this->name, $error, "validation");
163			}
164			}
165			return false;
166			}
167
168			return true;
169			}
170
171			/**
172			* Set if relation can be automatically assigned to the underlying dataobject
173			*
174			* @param bool $auto
175			* @return $this
176			*/
177			public function setRelationAutoSetting($auto)
178			{
179			$this->relationAutoSetting = $auto;
180			return $this;
181			}
182
183			/**
184			* Check if relation can be automatically assigned to the underlying dataobject
185			*
186			* @return bool
187			*/
188			public function getRelationAutoSetting()
189			{
190			return $this->relationAutoSetting;
191			}
192			}
193

silverstripe / silverstripe-framework

Pull Request — master (#6325)

FileField A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

8 Methods

Available Fixes

Duplication Side-by-Side

Filter issues like