jenkins-bot has submitted this change and it was merged.
Change subject: Add FIXMEs to StatementGrouper code
......................................................................
Add FIXMEs to StatementGrouper code
Bug: T120661
Change-Id: I40fafa5df80eff005e8200f50f1b8574f02c7857
---
M repo/includes/DispatchingEntityTypeStatementGrouper.php
M repo/includes/StatementGrouperBuilder.php
2 files changed, 8 insertions(+), 2 deletions(-)
Approvals:
Jonas Kress (WMDE): Looks good to me, approved
Thiemo Mättig (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/DispatchingEntityTypeStatementGrouper.php
b/repo/includes/DispatchingEntityTypeStatementGrouper.php
index 558d25c..5f2562c 100644
--- a/repo/includes/DispatchingEntityTypeStatementGrouper.php
+++ b/repo/includes/DispatchingEntityTypeStatementGrouper.php
@@ -55,7 +55,7 @@
* @return StatementList[]
*/
public function groupStatements( StatementList $statements ) {
- return $this->getStatementGrouper( $statements
)->groupStatements( $statements );
+ return $this->guessStatementGrouper( $statements
)->groupStatements( $statements );
}
/**
@@ -63,13 +63,15 @@
*
* @return StatementGrouper
*/
- private function getStatementGrouper( StatementList $statements ) {
+ private function guessStatementGrouper( StatementList $statements ) {
foreach ( $statements->toArray() as $statement ) {
$entityType = $this->getEntityType( $statement );
if ( array_key_exists( $entityType,
$this->statementGroupers ) ) {
return $this->statementGroupers[$entityType];
}
+
+ // FIXME: Check all statements and fail if they don't
share the same entity type?
}
return new NullStatementGrouper();
@@ -84,6 +86,7 @@
try {
$guid = $this->guidParser->parse( $statement->getGuid()
);
} catch ( StatementGuidParsingException $ex ) {
+ // FIXME: Fail when there is a statement with no GUID?
return null;
}
diff --git a/repo/includes/StatementGrouperBuilder.php
b/repo/includes/StatementGrouperBuilder.php
index b1da762..0b399e4 100644
--- a/repo/includes/StatementGrouperBuilder.php
+++ b/repo/includes/StatementGrouperBuilder.php
@@ -111,6 +111,9 @@
case 'propertySet':
$this->requireField( $spec, 'propertyIds' );
return new PropertySetStatementFilter(
$spec['propertyIds'] );
+ // Be aware that this switch statement is a possible
violation of the open-closed
+ // principle. When the number of filters grows, please
try to extract this in a way that
+ // it can be injected.
}
throw new InvalidArgumentException( 'Unknown filter type: ' .
$spec['type'] );
--
To view, visit https://gerrit.wikimedia.org/r/258130
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I40fafa5df80eff005e8200f50f1b8574f02c7857
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Adrian Lang <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
