Conditions | 7 |
Paths | 4 |
Total Lines | 140 |
Lines | 0 |
Ratio | 0 % |
Changes | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
1 | <?php |
||
119 | public function testGetPagesUsing( array $aspects, bool $expect456, bool $expect789 ) { |
||
120 | $globalSiteId = 'wiki'; |
||
121 | $entityIds = [ |
||
122 | // This entity ID is irrelevant to ImplicitUsageLookup, |
||
123 | // but the inner lookup will return a usage for it. |
||
124 | // We will assert that it’s not thrown away. |
||
125 | new PropertyId( 'P951' ), |
||
126 | // For this entity ID, the inner lookup will already return a usage |
||
127 | // equal to the implicit usage, so there will be nothing to do. |
||
128 | new ItemId( 'Q123' ), |
||
129 | // For this entity ID, the inner lookup will return different usage |
||
130 | // from the implicit usage, so the implicit usage would need to be added. |
||
131 | // However, if $expect456 is false, the implicit usage should not be added |
||
132 | // because it’s not covered by the $aspects. |
||
133 | new ItemId( 'Q456' ), |
||
134 | // For this entity ID, the inner lookup will return no usage at all, |
||
135 | // so the ImplicitUsageLookup would have to add it, |
||
136 | // but only if $expect789 is true (otherwise it’s not covered by the $aspects). |
||
137 | new ItemId( 'Q789' ), |
||
138 | // For the title linked to this entity ID according to the SiteLinkLookup, |
||
139 | // the TitleFactory will return a Title with a 0 article ID, |
||
140 | // indicating that the repo thinks a page is linked to the item, |
||
141 | // but on the client it does not exist (maybe it was just deleted). |
||
142 | new ItemId( 'Q1000' ), |
||
143 | ]; |
||
144 | $siteLinkLookup = $this->createMock( SiteLinkLookup::class ); |
||
145 | $siteLinkLookup->method( 'getLinks' ) |
||
146 | ->with( [ 123, 456, 789, 1000 ], [ $globalSiteId ] ) |
||
147 | ->willReturn( [ |
||
148 | [ $globalSiteId, 'Page 123', $entityIds[1]->getNumericId() ], |
||
149 | [ $globalSiteId, 'Page 456', $entityIds[2]->getNumericId() ], |
||
150 | [ $globalSiteId, 'Page 789', $entityIds[3]->getNumericId() ], |
||
151 | [ $globalSiteId, 'Deleted page', $entityIds[4]->getNumericId() ], |
||
152 | ] ); |
||
153 | // The mocked titles will have a page ID matching the page title, |
||
154 | // and page language code lang-x-pageID: |
||
155 | // For example, title Page 123 = page ID 123 = language lang-x-123 |
||
156 | $titleFactory = $this->createMock( TitleFactory::class ); |
||
157 | $titleFactory->method( 'newFromDBkey' ) |
||
158 | ->willReturnCallback( function ( $pageName ) { |
||
159 | if ( strpos( $pageName, 'Page ' ) === 0 ) { |
||
160 | $pageId = (int)substr( $pageName, 5 ); |
||
161 | } else { |
||
162 | $pageId = 0; |
||
163 | } |
||
164 | $title = $this->createMock( Title::class ); |
||
165 | $title->method( 'getArticleID' ) |
||
166 | ->willReturn( $pageId ); |
||
167 | $language = $this->createMock( Language::class ); |
||
168 | $language->method( 'getCode' ) |
||
169 | ->willReturn( "lang-x-$pageId" ); |
||
170 | $title->method( 'getPageLanguage' ) |
||
171 | ->willReturn( $language ); |
||
172 | return $title; |
||
173 | } ); |
||
174 | $linkBatchFactory = $this->createMock( LinkBatchFactory::class ); |
||
175 | $linkBatchFactory->expects( $this->once() ) |
||
176 | ->method( 'newLinkBatch' ) |
||
177 | ->willReturnCallback( function ( array $titles ) { |
||
178 | // assert that it’s called with the right (mocked) titles |
||
179 | $pageIds = []; |
||
180 | foreach ( $titles as $title ) { |
||
181 | $pageIds[] = $title->getArticleID(); |
||
182 | } |
||
183 | $this->assertSame( [ 123, 456, 789, 0 ], $pageIds ); |
||
184 | |||
185 | $linkBatch = $this->createMock( LinkBatch::class ); |
||
186 | $linkBatch->expects( $this->once() ) |
||
187 | ->method( 'execute' ); |
||
188 | return $linkBatch; |
||
189 | } ); |
||
190 | $usageLookup = $this->createMock( UsageLookup::class ); |
||
191 | $pageEntityUsages123 = new PageEntityUsages( 123, [ |
||
192 | new EntityUsage( |
||
193 | $entityIds[1], |
||
194 | EntityUsage::DESCRIPTION_USAGE, |
||
195 | 'lang-x-123' |
||
196 | ), |
||
197 | ] ); |
||
198 | $originalPageEntityUsages123 = clone $pageEntityUsages123; |
||
199 | $pageEntityUsages456 = new PageEntityUsages( 456, [ |
||
200 | new EntityUsage( $entityIds[2], EntityUsage::STATEMENT_USAGE ), |
||
201 | ] ); |
||
202 | $originalPageEntityUsages456 = clone $pageEntityUsages456; |
||
203 | $pageEntityUsages951 = new PageEntityUsages( 951, [ |
||
204 | new EntityUsage( $entityIds[0], EntityUsage::OTHER_USAGE ), |
||
205 | ] ); |
||
206 | $originalPageEntityUsages951 = clone $pageEntityUsages951; |
||
207 | $usageLookup->method( 'getPagesUsing' ) |
||
208 | ->with( $entityIds, $aspects ) |
||
209 | ->willReturn( new ArrayIterator( [ |
||
210 | $pageEntityUsages123, |
||
211 | $pageEntityUsages456, |
||
212 | $pageEntityUsages951, |
||
213 | ] ) ); |
||
214 | |||
215 | $pageEntityUsages = ( new ImplicitDescriptionUsageLookup( |
||
216 | $usageLookup, |
||
217 | $titleFactory, |
||
218 | $linkBatchFactory, |
||
219 | $globalSiteId, |
||
220 | $siteLinkLookup |
||
221 | ) )->getPagesUsing( $entityIds, $aspects ); |
||
222 | |||
223 | $pageEntityUsages = iterator_to_array( $pageEntityUsages ); |
||
224 | $this->assertCount( $expect789 ? 4 : 3, $pageEntityUsages ); |
||
225 | |||
226 | $this->assertSame( $pageEntityUsages123, $pageEntityUsages[0] ); |
||
227 | $this->assertEquals( $originalPageEntityUsages123, $pageEntityUsages123 ); |
||
228 | |||
229 | $this->assertSame( $pageEntityUsages456, $pageEntityUsages[1] ); |
||
230 | $usages456 = array_values( $pageEntityUsages456->getUsages() ); |
||
231 | $originalUsages456 = array_values( $originalPageEntityUsages456->getUsages() ); |
||
232 | $this->assertCount( $expect456 ? 2 : 1, $usages456 ); |
||
233 | // we know the explicit usage comes before the implicit one, |
||
234 | // because PageEntityUsages sorts them and C(laim) < D(escription) |
||
235 | $this->assertSame( $originalUsages456[0], $usages456[0] ); |
||
236 | if ( $expect456 ) { |
||
237 | $implicitUsage = $usages456[1]; |
||
238 | $this->assertEquals( $entityIds[2], $implicitUsage->getEntityId() ); |
||
239 | $this->assertSame( EntityUsage::DESCRIPTION_USAGE, $implicitUsage->getAspect() ); |
||
240 | $this->assertSame( 'lang-x-456', $implicitUsage->getModifier() ); |
||
241 | } |
||
242 | |||
243 | $this->assertSame( $pageEntityUsages951, $pageEntityUsages[2] ); |
||
244 | $this->assertEquals( $originalPageEntityUsages951, $pageEntityUsages951 ); |
||
245 | |||
246 | if ( $expect789 ) { |
||
247 | /** @var PageEntityUsages $pageEntityUsages789 */ |
||
248 | '@phan-var PageEntityUsages $pageEntityUsages789'; |
||
249 | $pageEntityUsages789 = $pageEntityUsages[3]; |
||
250 | $this->assertSame( 789, $pageEntityUsages789->getPageId() ); |
||
251 | $usages789 = array_values( $pageEntityUsages789->getUsages() ); |
||
252 | $this->assertCount( 1, $usages789 ); |
||
253 | $implicitUsage = $usages789[0]; |
||
254 | $this->assertEquals( $entityIds[3], $implicitUsage->getEntityId() ); |
||
255 | $this->assertSame( EntityUsage::DESCRIPTION_USAGE, $implicitUsage->getAspect() ); |
||
256 | $this->assertSame( 'lang-x-789', $implicitUsage->getModifier() ); |
||
257 | } |
||
258 | } |
||
259 | |||
349 |