SpecialNewItem::__construct()   A
last analyzed

Complexity

Conditions 1
Paths 1

Size

Total Lines 23

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 23
rs 9.552
c 0
b 0
f 0
cc 1
nc 1
nop 8

How to fix   Many Parameters   

Many Parameters

Methods with many parameters are not only hard to understand, but their parameters also often become inconsistent when you need more, or different data.

There are several approaches to avoid long parameter lists:

1
<?php
2
3
namespace Wikibase\Repo\Specials;
4
5
use Message;
6
use OutputPage;
7
use SiteLookup;
8
use Status;
9
use Wikibase\DataModel\Entity\EntityDocument;
10
use Wikibase\DataModel\Entity\Item;
11
use Wikibase\DataModel\Term\Term;
12
use Wikibase\Lib\Store\EntityNamespaceLookup;
13
use Wikibase\Lib\Store\EntityTitleLookup;
14
use Wikibase\Lib\Summary;
15
use Wikibase\Repo\CopyrightMessageBuilder;
16
use Wikibase\Repo\EditEntity\MediawikiEditEntityFactory;
17
use Wikibase\Repo\Specials\HTMLForm\HTMLAliasesField;
18
use Wikibase\Repo\Specials\HTMLForm\HTMLContentLanguageField;
19
use Wikibase\Repo\Specials\HTMLForm\HTMLTrimmedTextField;
20
use Wikibase\Repo\Store\TermsCollisionDetector;
21
use Wikibase\Repo\SummaryFormatter;
22
use Wikibase\Repo\Validators\TermValidatorFactory;
23
use Wikibase\Repo\WikibaseRepo;
24
25
/**
26
 * Page for creating new Wikibase items.
27
 *
28
 * @license GPL-2.0-or-later
29
 * @author John Erling Blad < [email protected] >
30
 */
31
class SpecialNewItem extends SpecialNewEntity {
32
33
	const FIELD_LANG = 'lang';
34
	const FIELD_LABEL = 'label';
35
	const FIELD_DESCRIPTION = 'description';
36
	const FIELD_ALIASES = 'aliases';
37
	const FIELD_SITE = 'site';
38
	const FIELD_PAGE = 'page';
39
40
	/**
41
	 * @var SiteLookup
42
	 */
43
	private $siteLookup;
44
45
	/**
46
	 * @var TermValidatorFactory
47
	 */
48
	private $termValidatorFactory;
49
50
	/**
51
	 * @var TermsCollisionDetector
52
	 */
53
	private $termsCollisionDetector;
54
55
	public function __construct(
56
		SpecialPageCopyrightView $copyrightView,
57
		EntityNamespaceLookup $entityNamespaceLookup,
58
		SummaryFormatter $summaryFormatter,
59
		EntityTitleLookup $entityTitleLookup,
60
		MediawikiEditEntityFactory $editEntityFactory,
61
		SiteLookup $siteLookup,
62
		TermValidatorFactory $termValidatorFactory,
63
		TermsCollisionDetector $termsCollisionDetector
64
	) {
65
		parent::__construct(
66
			'NewItem',
67
			'createpage',
68
			$copyrightView,
69
			$entityNamespaceLookup,
70
			$summaryFormatter,
71
			$entityTitleLookup,
72
			$editEntityFactory
73
		);
74
		$this->siteLookup = $siteLookup;
75
		$this->termValidatorFactory = $termValidatorFactory;
76
		$this->termsCollisionDetector = $termsCollisionDetector;
77
	}
78
79
	public static function factory(): self {
80
		$wikibaseRepo = WikibaseRepo::getDefaultInstance();
81
82
		$settings = $wikibaseRepo->getSettings();
83
		$copyrightView = new SpecialPageCopyrightView(
84
			new CopyrightMessageBuilder(),
85
			$settings->getSetting( 'dataRightsUrl' ),
86
			$settings->getSetting( 'dataRightsText' )
87
		);
88
89
		return new self(
90
			$copyrightView,
91
			$wikibaseRepo->getEntityNamespaceLookup(),
92
			$wikibaseRepo->getSummaryFormatter(),
93
			$wikibaseRepo->getEntityTitleLookup(),
94
			$wikibaseRepo->newEditEntityFactory(),
95
			$wikibaseRepo->getSiteLookup(),
96
			$wikibaseRepo->getTermValidatorFactory(),
97
			$wikibaseRepo->getItemTermsCollisionDetector()
98
		);
99
	}
100
101
	/**
102
	 * @see SpecialNewEntity::doesWrites
103
	 *
104
	 * @return bool
105
	 */
106
	public function doesWrites() {
107
		return true;
108
	}
109
110
	/**
111
	 * @see SpecialNewEntity::createEntityFromFormData
112
	 *
113
	 * @param array $formData
114
	 *
115
	 * @return Item
116
	 */
117
	protected function createEntityFromFormData( array $formData ) {
118
		$languageCode = $formData[ self::FIELD_LANG ];
119
120
		$item = new Item();
121
		$item->setLabel( $languageCode, $formData[ self::FIELD_LABEL ] );
122
		$item->setDescription( $languageCode, $formData[ self::FIELD_DESCRIPTION ] );
123
124
		$item->setAliases( $languageCode, $formData[ self::FIELD_ALIASES ] );
125
126
		if ( isset( $formData[ self::FIELD_SITE ] ) ) {
127
			$site = $this->siteLookup->getSite( $formData[ self::FIELD_SITE ] );
128
			$normalizedPageName = $site->normalizePageName( $formData[ self::FIELD_PAGE ] );
129
130
			$item->getSiteLinkList()->addNewSiteLink( $site->getGlobalId(), $normalizedPageName );
131
		}
132
133
		return $item;
134
	}
135
136
	/**
137
	 * @return array[]
138
	 */
139
	protected function getFormFields() {
140
		$formFields = [
141
			self::FIELD_LANG => [
142
				'name' => self::FIELD_LANG,
143
				'class' => HTMLContentLanguageField::class,
144
				'id' => 'wb-newentity-language',
145
			],
146
			self::FIELD_LABEL => [
147
				'name' => self::FIELD_LABEL,
148
				'default' => $this->parts[0] ?? '',
149
				'class' => HTMLTrimmedTextField::class,
150
				'id' => 'wb-newentity-label',
151
				'placeholder-message' => 'wikibase-label-edit-placeholder',
152
				'label-message' => 'wikibase-newentity-label',
153
			],
154
			self::FIELD_DESCRIPTION => [
155
				'name' => self::FIELD_DESCRIPTION,
156
				'default' => $this->parts[1] ?? '',
157
				'class' => HTMLTrimmedTextField::class,
158
				'id' => 'wb-newentity-description',
159
				'placeholder-message' => 'wikibase-description-edit-placeholder',
160
				'label-message' => 'wikibase-newentity-description',
161
			],
162
			self::FIELD_ALIASES => [
163
				'name' => self::FIELD_ALIASES,
164
				'class' => HTMLAliasesField::class,
165
				'id' => 'wb-newentity-aliases',
166
			],
167
		];
168
169
		$request = $this->getRequest();
170
		if ( $request->getCheck( self::FIELD_SITE ) && $request->getCheck( self::FIELD_PAGE ) ) {
171
			$formFields[ self::FIELD_SITE ] = [
172
				'name' => self::FIELD_SITE,
173
				'default' => $request->getVal( self::FIELD_SITE ),
174
				'type' => 'text',
175
				'id' => 'wb-newitem-site',
176
				'readonly' => 'readonly',
177
				'validation-callback' => function ( $siteId, $formData ) {
0 ignored issues
show
Unused Code introduced by
The parameter $formData 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...
178
					$site = $this->siteLookup->getSite( $siteId );
179
180
					if ( $site === null ) {
181
						return [ $this->msg( 'wikibase-newitem-not-recognized-siteid' )->text() ];
182
					}
183
184
					return true;
185
				},
186
				'label-message' => 'wikibase-newitem-site'
187
			];
188
189
			$formFields[ self::FIELD_PAGE ] = [
190
				'name' => self::FIELD_PAGE,
191
				'default' => $request->getVal( self::FIELD_PAGE ),
192
				'type' => 'text',
193
				'id' => 'wb-newitem-page',
194
				'readonly' => 'readonly',
195
				'validation-callback' => function ( $pageName, $formData ) {
196
					$siteId = $formData['site'];
197
					$site = $this->siteLookup->getSite( $siteId );
198
					if ( $site === null ) {
199
						return true;
200
					}
201
202
					$normalizedPageName = $site->normalizePageName( $pageName );
203
					if ( $normalizedPageName === false ) {
204
						return [
205
							$this->msg(
206
								'wikibase-newitem-no-external-page',
207
								$siteId,
208
								$pageName
209
							)->text(),
210
						];
211
					}
212
213
					return true;
214
				},
215
				'label-message' => 'wikibase-newitem-page'
216
			];
217
		}
218
219
		return $formFields;
0 ignored issues
show
Bug Best Practice introduced by
The return type of return $formFields; (array<string,array<string,string>>) is incompatible with the return type declared by the abstract method Wikibase\Repo\Specials\S...ewEntity::getFormFields of type array[].

If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.

Let’s take a look at an example:

class Author {
    private $name;

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

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}

Our function my_function expects a Post object, and outputs the author of the post. The base class Post returns a simple string and outputting a simple string will work just fine. However, the child class BlogPost which is a sub-type of Post instead decided to return an object, and is therefore violating the SOLID principles. If a BlogPost were passed to my_function, PHP would not complain, but ultimately fail when executing the strtoupper call in its body.

Loading history...
220
	}
221
222
	/**
223
	 * @see SpecialNewEntity::getLegend
224
	 *
225
	 * @return string|Message $msg Message key or Message object
226
	 */
227
	protected function getLegend() {
228
		return $this->msg( 'wikibase-newitem-fieldset' );
229
	}
230
231
	/**
232
	 * @see SpecialNewEntity::getWarnings
233
	 *
234
	 * @return string[]
235
	 */
236
	protected function getWarnings() {
237
		if ( $this->getUser()->isAnon() ) {
238
			return [
239
				$this->msg( 'wikibase-anonymouseditwarning', $this->msg( 'wikibase-entity-item' ) )->parse(),
240
			];
241
		}
242
243
		return [];
244
	}
245
246
	/**
247
	 * @param array $formData
248
	 *
249
	 * @return Status
250
	 */
251
	protected function validateFormData( array $formData ) {
252
		if ( $formData[ self::FIELD_LABEL ] == ''
253
			 && $formData[ self::FIELD_DESCRIPTION ] == ''
254
			 && $formData[ self::FIELD_ALIASES ] === []
255
		) {
256
			return Status::newFatal( 'wikibase-newitem-insufficient-data' );
257
		}
258
259
		// Disallow the same label and description, but ignore if both are empty T100933
260
		if ( $formData[ self::FIELD_LABEL ] !== '' &&
261
			$formData[ self::FIELD_LABEL ] === $formData[ self::FIELD_DESCRIPTION ]
262
		) {
263
			return Status::newFatal( 'wikibase-newitem-same-label-and-description' );
264
		}
265
266
		if ( $formData[self::FIELD_LABEL] != '' ) {
267
			$validator = $this->termValidatorFactory->getLabelValidator( $this->getEntityType() );
268
			$result = $validator->validate( $formData[self::FIELD_LABEL] );
269
			if ( !$result->isValid() ) {
270
				return $this->createStatusFromValidatorError( $result->getErrors()[0] );
271
			}
272
		}
273
274
		if ( $formData[self::FIELD_DESCRIPTION] != '' ) {
275
			$validator = $this->termValidatorFactory->getDescriptionValidator();
276
			$result = $validator->validate( $formData[self::FIELD_DESCRIPTION] );
277
			if ( !$result->isValid() ) {
278
				return $this->createStatusFromValidatorError( $result->getErrors()[0] );
279
			}
280
		}
281
282
		if ( $formData[self::FIELD_ALIASES] !== [] ) {
283
			$validator = $this->termValidatorFactory->getAliasValidator();
284
			foreach ( $formData[self::FIELD_ALIASES] as $alias ) {
285
				$result = $validator->validate( $alias );
286
				if ( !$result->isValid() ) {
287
					return $this->createStatusFromValidatorError( $result->getErrors()[0] );
288
				}
289
			}
290
291
			$result = $validator->validate( implode( '|', $formData[self::FIELD_ALIASES] ) );
292
			if ( !$result->isValid() ) {
293
				return $this->createStatusFromValidatorError( $result->getErrors()[0] );
294
			}
295
		}
296
297
		$collidingItemId = $this->termsCollisionDetector->detectLabelAndDescriptionCollision(
298
			$formData[ self::FIELD_LANG ],
299
			$formData[ self::FIELD_LABEL ],
300
			$formData[ self::FIELD_DESCRIPTION ]
301
		);
302
		if ( $collidingItemId !== null ) {
303
			return Status::newFatal(
304
				'wikibase-validator-label-with-description-conflict',
305
				$formData[ self::FIELD_LABEL ],
306
				$formData[ self::FIELD_LANG ],
307
				$collidingItemId
308
			);
309
		}
310
311
		return Status::newGood();
312
	}
313
314
	private function createStatusFromValidatorError( $error ) {
315
		$params = array_merge( [ 'wikibase-validator-' . $error->getCode() ],  $error->getParameters() );
316
		return Status::newFatal( ...$params );
317
	}
318
319
	/**
320
	 * @param Item $item
321
	 *
322
	 * @return Summary
323
	 * @suppress PhanParamSignatureMismatch Uses intersection types
324
	 */
325
	protected function createSummary( EntityDocument $item ) {
326
		$uiLanguageCode = $this->getLanguage()->getCode();
327
328
		$summary = new Summary( 'wbeditentity', 'create' );
329
		$summary->setLanguage( $uiLanguageCode );
330
		/** @var Term|null $labelTerm */
331
		$labelTerm = $item->getLabels()->getIterator()->current();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Wikibase\DataModel\Entity\EntityDocument as the method getLabels() does only exist in the following implementations of said interface: Wikibase\DataModel\Entity\Item, Wikibase\DataModel\Entity\Property.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
332
		/** @var Term|null $descriptionTerm */
333
		$descriptionTerm = $item->getDescriptions()->getIterator()->current();
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Wikibase\DataModel\Entity\EntityDocument as the method getDescriptions() does only exist in the following implementations of said interface: Wikibase\DataModel\Entity\Item, Wikibase\DataModel\Entity\Property.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
334
		$summary->addAutoSummaryArgs(
335
			$labelTerm ? $labelTerm->getText() : '',
336
			$descriptionTerm ? $descriptionTerm->getText() : ''
337
		);
338
339
		return $summary;
340
	}
341
342
	protected function displayBeforeForm( OutputPage $output ) {
343
		parent::displayBeforeForm( $output );
344
		$output->addModules( 'wikibase.special.languageLabelDescriptionAliases' );
345
	}
346
347
	/**
348
	 * @inheritDoc
349
	 */
350
	protected function getEntityType() {
351
		return Item::ENTITY_TYPE;
352
	}
353
354
}
355