Skip to content

Commit ef62b69

Browse files
froemkendkd-kaehm
authored andcommitted
[TASK] Refactor and optimize Classification handling
- Set `public: true` and add `autowire: true` for Classification service. - Refactor tests to use `IntegrationTestBase` and improve readability: - Add type annotations, reformat arrays, and streamline mock setup. - Replace inline `DataProvider` references with `methodName` annotation. - Simplify `Classification` and `ClassificationService` classes: - Implement constructor property promotion. - Remove unused setter methods. - Optimize instance creation by replacing `GeneralUtility::makeInstance` with direct instantiation.
1 parent 6fd3523 commit ef62b69

6 files changed

Lines changed: 95 additions & 91 deletions

File tree

Classes/ContentObject/Classification.php

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ class Classification extends AbstractContentObject
4141
{
4242
public const CONTENT_OBJECT_NAME = 'SOLR_CLASSIFICATION';
4343

44+
public function __construct(
45+
protected ClassificationService $classificationService,
46+
) {}
47+
4448
/**
4549
* Executes the SOLR_CLASSIFICATION content object.
4650
*
@@ -70,10 +74,8 @@ public function render($conf = [])
7074
$data = $this->cObj->stdWrap($data, $conf);
7175
}
7276
$classifications = $this->buildClassificationsFromConfiguration($configuredMappedClasses);
73-
/** @var ClassificationService $classificationService */
74-
$classificationService = GeneralUtility::makeInstance(ClassificationService::class);
7577

76-
return serialize($classificationService->getMatchingClassNames((string)$data, $classifications));
78+
return serialize($this->classificationService->getMatchingClassNames((string)$data, $classifications));
7779
}
7880

7981
/**
@@ -84,6 +86,7 @@ public function render($conf = [])
8486
protected function buildClassificationsFromConfiguration(array $configuredMappedClasses): array
8587
{
8688
$classifications = [];
89+
8790
foreach ($configuredMappedClasses as $class) {
8891
if ((empty($class['patterns']) && empty($class['matchPatterns'])) || empty($class['class'])) {
8992
throw new InvalidArgumentException(
@@ -99,12 +102,7 @@ protected function buildClassificationsFromConfiguration(array $configuredMapped
99102
$unMatchPatters = empty($class['unmatchPatterns']) ? [] : GeneralUtility::trimExplode(',', $class['unmatchPatterns']);
100103

101104
$className = $class['class'];
102-
$classifications[] = GeneralUtility::makeInstance(
103-
ClassificationItem::class,
104-
$matchPatterns,
105-
$unMatchPatters,
106-
$className,
107-
);
105+
$classifications[] = new ClassificationItem($matchPatterns, $unMatchPatters, $className);
108106
}
109107

110108
return $classifications;

Classes/Domain/Index/Classification/Classification.php

Lines changed: 4 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -17,63 +17,26 @@
1717

1818
namespace ApacheSolrForTypo3\Solr\Domain\Index\Classification;
1919

20-
/**
21-
* Class Classification
22-
*/
2320
class Classification
2421
{
25-
/**
26-
* Array of regular expressions
27-
*/
28-
protected array $matchPatterns = [];
29-
30-
/**
31-
* Array of regular expressions
32-
*/
33-
protected array $unMatchPatterns = [];
34-
35-
protected string $mappedClass;
36-
37-
/**
38-
* Classification constructor.
39-
*/
4022
public function __construct(
41-
array $matchPatterns = [],
42-
array $unMatchPatterns = [],
43-
string $mappedClass = '',
44-
) {
45-
$this->matchPatterns = $matchPatterns;
46-
$this->unMatchPatterns = $unMatchPatterns;
47-
$this->mappedClass = $mappedClass;
48-
}
23+
protected array $matchPatterns = [],
24+
protected array $unMatchPatterns = [],
25+
protected string $mappedClass = '',
26+
) {}
4927

5028
public function getUnMatchPatterns(): array
5129
{
5230
return $this->unMatchPatterns;
5331
}
5432

55-
public function setUnMatchPatterns(array $unMatchPatterns): void
56-
{
57-
$this->unMatchPatterns = $unMatchPatterns;
58-
}
59-
6033
public function getMatchPatterns(): array
6134
{
6235
return $this->matchPatterns;
6336
}
6437

65-
public function setMatchPatterns(array $matchPatterns): void
66-
{
67-
$this->matchPatterns = $matchPatterns;
68-
}
69-
7038
public function getMappedClass(): string
7139
{
7240
return $this->mappedClass;
7341
}
74-
75-
public function setMappedClass(string $mappedClass): void
76-
{
77-
$this->mappedClass = $mappedClass;
78-
}
7942
}

Classes/Domain/Index/Classification/ClassificationService.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717

1818
namespace ApacheSolrForTypo3\Solr\Domain\Index\Classification;
1919

20-
/**
21-
* Class ClassificationService
22-
*/
2320
class ClassificationService
2421
{
2522
/**

Configuration/Services.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@ services:
372372

373373
### EXT:solr content objects
374374
ApacheSolrForTypo3\Solr\ContentObject\Classification:
375-
shared: false
375+
public: true
376+
autowire: true
376377
tags:
377378
- name: frontend.contentobject
378379
identifier: 'SOLR_CLASSIFICATION'

Tests/Unit/ContentObject/ClassificationTest.php renamed to Tests/Integration/ContentObject/ClassificationTest.php

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,34 @@
1515
* The TYPO3 project - inspiring people to share!
1616
*/
1717

18-
namespace ApacheSolrForTypo3\Solr\Tests\Unit\ContentObject;
18+
namespace ApacheSolrForTypo3\Solr\Tests\Integration\ContentObject;
1919

2020
use ApacheSolrForTypo3\Solr\ContentObject\Classification;
21+
use ApacheSolrForTypo3\Solr\Tests\Integration\IntegrationTestBase;
2122
use PHPUnit\Framework\Attributes\DataProvider;
2223
use PHPUnit\Framework\Attributes\Test;
2324
use Traversable;
25+
use TYPO3\CMS\Core\Core\SystemEnvironmentBuilder;
26+
use TYPO3\CMS\Core\Http\ServerRequest;
27+
use TYPO3\CMS\Core\Utility\GeneralUtility;
28+
use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
2429

2530
/**
2631
* Tests for the SOLR_CLASSIFICATION cObj.
2732
*/
28-
class ClassificationTest extends SetUpContentObject
33+
class ClassificationTest extends IntegrationTestBase
2934
{
30-
protected function getTestableContentObjectClassName(): string
35+
protected ContentObjectRenderer $contentObjectRenderer;
36+
37+
protected function setUp(): void
3138
{
32-
return Classification::class;
39+
parent::setUp();
40+
41+
$request = (new ServerRequest('https://example.com'))
42+
->withAttribute('applicationType', SystemEnvironmentBuilder::REQUESTTYPE_FE);
43+
44+
$this->contentObjectRenderer = GeneralUtility::makeInstance(ContentObjectRenderer::class);
45+
$this->contentObjectRenderer->setRequest($request);
3346
}
3447

3548
#[Test]
@@ -52,9 +65,15 @@ public function canClassifyContent(): void
5265
],
5366
];
5467

55-
$actual = $this->contentObjectRenderer->cObjGetSingle(Classification::CONTENT_OBJECT_NAME, $configuration);
56-
$expected = serialize(['cms']);
57-
self::assertEquals($expected, $actual);
68+
$actual = $this->contentObjectRenderer->cObjGetSingle(
69+
Classification::CONTENT_OBJECT_NAME,
70+
$configuration,
71+
);
72+
73+
self::assertEquals(
74+
serialize(['cms']),
75+
$actual,
76+
);
5877
}
5978

6079
public static function excludePatternDataProvider(): Traversable
@@ -69,8 +88,10 @@ public static function excludePatternDataProvider(): Traversable
6988
];
7089
}
7190

72-
#[DataProvider('excludePatternDataProvider')]
7391
#[Test]
92+
#[DataProvider(
93+
methodName: 'excludePatternDataProvider',
94+
)]
7495
public function canExcludePatterns($input, $expectedOutput): void
7596
{
7697
$this->contentObjectRenderer->start(['content' => $input]);
@@ -86,8 +107,14 @@ public function canExcludePatterns($input, $expectedOutput): void
86107
],
87108
];
88109

89-
$actual = $this->contentObjectRenderer->cObjGetSingle(Classification::CONTENT_OBJECT_NAME, $configuration);
90-
$expected = serialize($expectedOutput);
91-
self::assertEquals($expected, $actual);
110+
$actual = $this->contentObjectRenderer->cObjGetSingle(
111+
Classification::CONTENT_OBJECT_NAME,
112+
$configuration,
113+
);
114+
115+
self::assertEquals(
116+
serialize($expectedOutput),
117+
$actual,
118+
);
92119
}
93120
}

Tests/Unit/ContentObject/SetUpContentObject.php

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,13 @@ protected function setUp(): void
4343
parent::setUp();
4444

4545
$request = new ServerRequest();
46+
4647
$this->contentObjectRenderer = $this->createMock(ContentObjectRenderer::class);
47-
$this->contentObjectRenderer->method('getRequest')->willReturn($request);
48+
$this->contentObjectRenderer
49+
->expects(self::any())
50+
->method('getRequest')
51+
->willReturn($request);
52+
4853
$cObjectFactoryMock = $this->getMockBuilder(ContentObjectFactory::class)
4954
->disableOriginalConstructor()
5055
->getMock();
@@ -53,9 +58,12 @@ protected function setUp(): void
5358
$this->testableContentObject->setRequest($request);
5459
$this->testableContentObject->setContentObjectRenderer($this->contentObjectRenderer);
5560

56-
$cObjectFactoryMock->method('getContentObject')->willReturnMap([
57-
[($this->getTestableContentObjectClassName())::CONTENT_OBJECT_NAME, $request, $this->contentObjectRenderer, $this->testableContentObject],
58-
]);
61+
$cObjectFactoryMock
62+
->expects(self::any())
63+
->method('getContentObject')
64+
->willReturnMap([
65+
[($this->getTestableContentObjectClassName())::CONTENT_OBJECT_NAME, $request, $this->contentObjectRenderer, $this->testableContentObject],
66+
]);
5967

6068
$container = new Container();
6169
$container->set(ContentObjectFactory::class, $cObjectFactoryMock);
@@ -64,33 +72,43 @@ protected function setUp(): void
6472

6573
// Track data set via start() for use in stdWrap
6674
$data = [];
67-
$this->contentObjectRenderer->method('start')->willReturnCallback(
68-
function (array $inputData) use (&$data) {
69-
$data = $inputData;
70-
},
71-
);
75+
$this->contentObjectRenderer
76+
->expects(self::any())
77+
->method('start')
78+
->willReturnCallback(
79+
function (array $inputData) use (&$data) {
80+
$data = $inputData;
81+
},
82+
);
7283

7384
// Configure stdWrap to return field values from data
74-
$this->contentObjectRenderer->method('stdWrap')->willReturnCallback(
75-
function (string $content, array $conf) use (&$data) {
76-
if (isset($conf['field']) && isset($data[$conf['field']])) {
77-
return $data[$conf['field']];
78-
}
79-
return $content;
80-
},
81-
);
85+
$this->contentObjectRenderer
86+
->expects(self::any())
87+
->method('stdWrap')
88+
->willReturnCallback(
89+
function (string $content, array $conf) use (&$data) {
90+
if (isset($conf['field']) && isset($data[$conf['field']])) {
91+
return $data[$conf['field']];
92+
}
93+
return $content;
94+
},
95+
);
8296

8397
// Configure the mock to call the actual content object's render method
8498
$testableContentObject = $this->testableContentObject;
85-
$this->contentObjectRenderer->method('cObjGetSingle')->willReturnCallback(
86-
function (string $name, array $conf) use ($testableContentObject) {
87-
$contentObjectName = ($testableContentObject::class)::CONTENT_OBJECT_NAME;
88-
if ($name === $contentObjectName) {
89-
return $testableContentObject->render($conf);
90-
}
91-
return '';
92-
},
93-
);
99+
100+
$this->contentObjectRenderer
101+
->expects(self::any())
102+
->method('cObjGetSingle')
103+
->willReturnCallback(
104+
function (string $name, array $conf) use ($testableContentObject) {
105+
$contentObjectName = ($testableContentObject::class)::CONTENT_OBJECT_NAME;
106+
if ($name === $contentObjectName) {
107+
return $testableContentObject->render($conf);
108+
}
109+
return '';
110+
},
111+
);
94112
}
95113

96114
abstract protected function getTestableContentObjectClassName(): string;

0 commit comments

Comments
 (0)