Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "professional-wiki/semantic-wikibase",
"name": "baillyk/semantic-wikibase",
"type": "mediawiki-extension",
"description": "MediaWiki extension that makes Wikibase data available in Semantic MediaWiki",
"keywords": [
Expand All @@ -12,7 +12,7 @@
"semantic web",
"wikidata"
],
"homepage": "https://professional.wiki/en/extension/semantic-wikibase",
"homepage": "https://github.com/baillyk/SemanticWikibase",
"license": "GPL-2.0-or-later",
"authors": [
{
Expand Down
4 changes: 3 additions & 1 deletion extension.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@
"callback": "MediaWiki\\Extension\\SemanticWikibase\\EntryPoints\\HookHandlers::onExtensionRegistration",

"Hooks": {
"ParserFirstCallInit":"MediaWiki\\Extension\\SemanticWikibase\\EntryPoints\\HookHandlers::onParserFirstCallInit",
"SMW::Property::initProperties": "MediaWiki\\Extension\\SemanticWikibase\\EntryPoints\\HookHandlers::onSmwInitProperties",
"SMW::SQLStore::AddCustomFixedPropertyTables": "MediaWiki\\Extension\\SemanticWikibase\\EntryPoints\\HookHandlers::onSmwAddCustomFixedPropertyTables",
"SMWStore::updateDataBefore": "MediaWiki\\Extension\\SemanticWikibase\\EntryPoints\\HookHandlers::onSmwUpdateDataBefore"
"SMW::SQLStore::BeforeDataUpdateComplete": "MediaWiki\\Extension\\SemanticWikibase\\EntryPoints\\HookHandlers::onSmwUpdateDataBefore",
"MultiContentSave": "MediaWiki\\Extension\\SemanticWikibase\\EntryPoints\\HookHandlers::onMultiContentSave"
},

"config": {
Expand Down
47 changes: 46 additions & 1 deletion src/EntryPoints/HookHandlers.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,22 @@

namespace MediaWiki\Extension\SemanticWikibase\EntryPoints;

use MediaWiki\Revision\RenderedRevision;
use MediaWiki\User\UserIdentity;
use CommentStoreComment;
use Status;
use MediaWiki\MediaWikiServices;
use MediaWiki\TitleFactory;
use MediaWiki\Extension\SemanticWikibase\SemanticWikibase;
use SMW\Services\ServicesFactory as ApplicationFactory;
use SMW\PropertyRegistry;
use SMW\SemanticData;
use SMW\DIWikiPage;
use SMW\StoreFactory;
use SMW\Store;

use Parser;

class HookHandlers {

public static function onExtensionRegistration(): void {
Expand All @@ -17,18 +28,52 @@ public static function onExtensionRegistration(): void {
$smwgNamespacesWithSemanticLinks[WB_NS_PROPERTY] = true;
}

public static function onParserFirstCallInit( Parser $parser ) {
# getInstance() will call initProperties() which aktivates existing hook onSmwInitProperties()
wfDebug( __METHOD__ . "SWB: onParserFirstCallInit" );
PropertyRegistry::getInstance();
}
Comment on lines +31 to +35
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Typo in comment.

Line 32: "aktivates" should be "activates".

📝 Proposed fix
 	public static function onParserFirstCallInit( Parser $parser ) {
-		# getInstance() will call initProperties() which aktivates existing hook onSmwInitProperties()
+		# getInstance() will call initProperties() which activates existing hook onSmwInitProperties()
 		wfDebug( __METHOD__ . "SWB: onParserFirstCallInit" );
 		PropertyRegistry::getInstance();
 	}
🧰 Tools
🪛 PHPMD (2.15.0)

[warning] 31-31: Avoid unused parameters such as '$parser'. (undefined)

(UnusedFormalParameter)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/EntryPoints/HookHandlers.php` around lines 31 - 35, The comment in the
onParserFirstCallInit method contains a typo: change "aktivates" to "activates"
in the inline comment that references getInstance() calling initProperties()
which activates the existing hook onSmwInitProperties(); update the comment text
near PropertyRegistry::getInstance() inside the onParserFirstCallInit function
accordingly.


public static function onSmwInitProperties( PropertyRegistry $propertyRegistry ): void {
SemanticWikibase::getGlobalInstance()->registerProperties( $propertyRegistry );
wfDebug( __METHOD__ . "SWB: onSmwInitProperties..." );
SemanticWikibase::getGlobalInstance()->registerProperties( $propertyRegistry );
}

public static function onSmwAddCustomFixedPropertyTables( array &$customFixedProperties, array &$fixedPropertyTablePrefix ): void {
wfDebug( __METHOD__ . "SWB: onSmwAddCustomFixedPropertyTables" );
SemanticWikibase::getGlobalInstance()->getFixedProperties()
->registerFixedTables( $customFixedProperties, $fixedPropertyTablePrefix );
}

public static function onSmwUpdateDataBefore( Store $store, SemanticData $semanticData ): void {
wfDebug( __METHOD__ . "SWB: onSmwUpdateDataBefore" );
SemanticWikibase::getGlobalInstance()->getSemanticDataUpdate()
->run( $semanticData );

}

public static function onPageSaveComplete( WikiPage $wikiPage, MediaWiki\User\UserIdentity $user, string $summary, int $flags, MediaWiki\Revision\RevisionRecord $revisionRecord, MediaWiki\Storage\EditResult $editResult ) {
wfDebug( __METHOD__ . "SWB: onPageSaveComplete" );
// Access the semantic data of the page via the Semantic MediaWiki API
$semanticData = \SMW\MediaWiki\Hooks\ParserHooks::getSemanticDataForPage($wikiPage);

onSmwUpdateDataBefore(null, $semanticData);
}
Comment on lines +55 to +61
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's see the actual file and the specific lines
cat -n src/EntryPoints/HookHandlers.php | sed -n '50,70p'

Repository: ProfessionalWiki/SemanticWikibase

Length of output: 1374


🏁 Script executed:

# Check if onSmwUpdateDataBefore is defined in the class as static
rg -A 5 'function onSmwUpdateDataBefore' src/EntryPoints/HookHandlers.php

Repository: ProfessionalWiki/SemanticWikibase

Length of output: 326


🏁 Script executed:

# Search for all function definitions in HookHandlers.php to understand structure
rg -n 'public\s+static\s+function' src/EntryPoints/HookHandlers.php

Repository: ProfessionalWiki/SemanticWikibase

Length of output: 927


🏁 Script executed:

# Search for ParserHooks class definition
find . -name "*.php" -type f | xargs rg -l "class ParserHooks"

Repository: ProfessionalWiki/SemanticWikibase

Length of output: 61


🏁 Script executed:

# Search for getSemanticDataForPage method
rg -n "getSemanticDataForPage" --type php

Repository: ProfessionalWiki/SemanticWikibase

Length of output: 205


🏁 Script executed:

# Check if there are any imports or references to SMW\MediaWiki\Hooks\ParserHooks
rg "ParserHooks" src/EntryPoints/HookHandlers.php -A 2 -B 2

Repository: ProfessionalWiki/SemanticWikibase

Length of output: 354


Fix critical fatal error: missing self:: prefix on static method call.

Line 60 calls onSmwUpdateDataBefore(null, $semanticData) without the self:: prefix. Since onSmwUpdateDataBefore is a static method defined in the same class (line 48), this will cause a fatal "Call to undefined function" error at runtime.

Proposed fix
 	public static function onPageSaveComplete( WikiPage $wikiPage, MediaWiki\User\UserIdentity $user, string $summary, int $flags, MediaWiki\Revision\RevisionRecord $revisionRecord, MediaWiki\Storage\EditResult $editResult ) {
         wfDebug( __METHOD__ . "SWB: onPageSaveComplete" );
 		// Access the semantic data of the page via the Semantic MediaWiki API
         $semanticData = \SMW\MediaWiki\Hooks\ParserHooks::getSemanticDataForPage($wikiPage);
 
-        onSmwUpdateDataBefore(null, $semanticData);
+        self::onSmwUpdateDataBefore(null, $semanticData);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public static function onPageSaveComplete( WikiPage $wikiPage, MediaWiki\User\UserIdentity $user, string $summary, int $flags, MediaWiki\Revision\RevisionRecord $revisionRecord, MediaWiki\Storage\EditResult $editResult ) {
wfDebug( __METHOD__ . "SWB: onPageSaveComplete" );
// Access the semantic data of the page via the Semantic MediaWiki API
$semanticData = \SMW\MediaWiki\Hooks\ParserHooks::getSemanticDataForPage($wikiPage);
onSmwUpdateDataBefore(null, $semanticData);
}
public static function onPageSaveComplete( WikiPage $wikiPage, MediaWiki\User\UserIdentity $user, string $summary, int $flags, MediaWiki\Revision\RevisionRecord $revisionRecord, MediaWiki\Storage\EditResult $editResult ) {
wfDebug( __METHOD__ . "SWB: onPageSaveComplete" );
// Access the semantic data of the page via the Semantic MediaWiki API
$semanticData = \SMW\MediaWiki\Hooks\ParserHooks::getSemanticDataForPage($wikiPage);
self::onSmwUpdateDataBefore(null, $semanticData);
}
🧰 Tools
🪛 PHPMD (2.15.0)

[warning] 55-55: Avoid unused parameters such as '$user'. (undefined)

(UnusedFormalParameter)


[warning] 55-55: Avoid unused parameters such as '$summary'. (undefined)

(UnusedFormalParameter)


[warning] 55-55: Avoid unused parameters such as '$flags'. (undefined)

(UnusedFormalParameter)


[warning] 55-55: Avoid unused parameters such as '$revisionRecord'. (undefined)

(UnusedFormalParameter)


[warning] 55-55: Avoid unused parameters such as '$editResult'. (undefined)

(UnusedFormalParameter)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/EntryPoints/HookHandlers.php` around lines 55 - 61, The call to
onSmwUpdateDataBefore from onPageSaveComplete is missing the self:: static
reference and therefore attempts to call a non-existent function; update the
call inside onPageSaveComplete to use self::onSmwUpdateDataBefore(null,
$semanticData) so it correctly invokes the static method defined in the same
class (ensure you update the invocation in the onPageSaveComplete method).


public static function onMultiContentSave( RenderedRevision $renderedRevision, UserIdentity $user, CommentStoreComment $summary, $flags, Status $hookStatus ) {
wfDebug( __METHOD__ . "SWB: onMultiContentSave" );
$revision = $renderedRevision->getRevision();

$titleFactory = MediaWikiServices::getInstance()->getTitleFactory();
$title = $titleFactory->newFromLinkTarget($revision->getPageAsLinkTarget());
wfDebug( __METHOD__ . "SWB: onMultiContentSave...title:".$title );
$subject = DIWikiPage::newFromTitle( $title );
#$new_content = $revision->getContent(SlotRecord::MAIN, RevisionRecord::RAW)->getNativeData();
$semanticData = StoreFactory::getStore()->getSemanticData( DIWikiPage::newFromTitle( $title ) );
wfDebug( __METHOD__ . "SWB: onMultiContentSave: ".json_encode($semanticData) );
SemanticWikibase::getGlobalInstance()->getSemanticDataUpdate()->run( $semanticData );

return true;
}
Comment on lines +63 to +77
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Redundant variable and duplicated call.

  • Line 70 assigns $subject which is never used (as flagged by static analysis).
  • DIWikiPage::newFromTitle($title) is called twice: at line 70 (unused result) and again at line 72.
🔧 Proposed fix to remove dead code
 	public static function onMultiContentSave( RenderedRevision $renderedRevision, UserIdentity $user, CommentStoreComment $summary, $flags, Status $hookStatus ) {
 		wfDebug( __METHOD__ . "SWB: onMultiContentSave" );
         $revision = $renderedRevision->getRevision();
 		
 		$titleFactory = MediaWikiServices::getInstance()->getTitleFactory();
         $title = $titleFactory->newFromLinkTarget($revision->getPageAsLinkTarget());
 		wfDebug( __METHOD__ . "SWB: onMultiContentSave...title:".$title );
-		$subject = DIWikiPage::newFromTitle( $title );
-        #$new_content = $revision->getContent(SlotRecord::MAIN, RevisionRecord::RAW)->getNativeData();
-		$semanticData = StoreFactory::getStore()->getSemanticData( DIWikiPage::newFromTitle( $title ) );
+		$semanticData = StoreFactory::getStore()->getSemanticData( DIWikiPage::newFromTitle( $title ) );
 		wfDebug( __METHOD__ . "SWB: onMultiContentSave: ".json_encode($semanticData) );
         SemanticWikibase::getGlobalInstance()->getSemanticDataUpdate()->run( $semanticData );
 
         return true;
     } 
🧰 Tools
🪛 PHPMD (2.15.0)

[warning] 63-63: Avoid unused parameters such as '$user'. (undefined)

(UnusedFormalParameter)


[warning] 63-63: Avoid unused parameters such as '$summary'. (undefined)

(UnusedFormalParameter)


[warning] 63-63: Avoid unused parameters such as '$flags'. (undefined)

(UnusedFormalParameter)


[warning] 63-63: Avoid unused parameters such as '$hookStatus'. (undefined)

(UnusedFormalParameter)


[warning] 70-70: Avoid unused local variables such as '$subject'. (undefined)

(UnusedLocalVariable)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/EntryPoints/HookHandlers.php` around lines 63 - 77, The
onMultiContentSave handler creates a redundant $subject variable and calls
DIWikiPage::newFromTitle($title) twice; remove the unused $subject assignment
and replace the second DIWikiPage::newFromTitle call by reusing a single
DIWikiPage instance (e.g. create $page = DIWikiPage::newFromTitle($title) once),
then pass $page into StoreFactory::getStore()->getSemanticData(...) and any
later uses (e.g. when building $semanticData) so the duplicate call and dead
$subject are eliminated.


}
32 changes: 23 additions & 9 deletions src/SMW/SemanticEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,34 @@
use SMW\DIWikiPage;
use SMW\SemanticData;
use SMWDataItem;
use SMW\Subobject;

class SemanticEntity {

private array $dataItemsPerProperty = [];
private array $subObjectsPerProperty=[];

public function addPropertyValue( string $propertyId, SMWDataItem $dataItem ) {
$this->dataItemsPerProperty[$propertyId][] = $dataItem;
public function addPropertyValue( string $NumericPropertyId, SMWDataItem $dataItem ) {
$this->dataItemsPerProperty[$NumericPropertyId][] = $dataItem;
}

public function addSubobject( string $NumericPropertyId, SubObject $subobject ){
$this->subObjectsPerProperty[$NumericPropertyId][] = $subobject;
}

/**
* @param string $propertyId
* @param string $NumericPropertyId
* @return SMWDataItem[]
*/
public function getDataItemsForProperty( string $propertyId ): array {
return $this->dataItemsPerProperty[$propertyId] ?? [];
public function getDataItemsForProperty( string $NumericPropertyId ): array {
return $this->dataItemsPerProperty[$NumericPropertyId] ?? [];
}

public function toSemanticData( DIWikiPage $subject ): SemanticData {
$semanticData = new SemanticData( $subject );

foreach ( $this->dataItemsPerProperty as $propertyId => $dataItems ) {
$property = new DIProperty( $propertyId );
foreach ( $this->dataItemsPerProperty as $NumericPropertyId => $dataItems ) {
$property = new DIProperty( $NumericPropertyId );

foreach ( $dataItems as $dataItem ) {
$semanticData->addPropertyObjectValue(
Expand All @@ -39,6 +45,14 @@ public function toSemanticData( DIWikiPage $subject ): SemanticData {
}
}

foreach ( $this->subObjectsPerProperty as $NumericPropertyId => $subobjects ) {
$property = new DIProperty( $NumericPropertyId );

foreach ( $subobjects as $subobject ) {
$semanticData->addSubobject($subobject);
}
}

return $semanticData;
}

Expand All @@ -52,9 +66,9 @@ public function functionalMerge( self $entity ): self {
}

public function add( self $entity ): void {
foreach ( $entity->dataItemsPerProperty as $propertyId => $dataItems ) {
foreach ( $entity->dataItemsPerProperty as $NumericPropertyId => $dataItems ) {
foreach ( $dataItems as $dataItem ) {
$this->addPropertyValue( $propertyId, $dataItem );
$this->addPropertyValue( $NumericPropertyId, $dataItem );
}
}
}
Comment on lines 68 to 74
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Subobjects are dropped during entity merge.

add() (Line 68+) merges only dataItemsPerProperty. Any subobjects added via addSubobject() are not merged, so functionalMerge() silently loses them.

Suggested fix
 public function add( self $entity ): void {
 		foreach ( $entity->dataItemsPerProperty as $NumericPropertyId => $dataItems ) {
 			foreach ( $dataItems as $dataItem ) {
 				$this->addPropertyValue( $NumericPropertyId, $dataItem );
 			}
 		}
+
+		foreach ( $entity->subObjectsPerProperty as $NumericPropertyId => $subobjects ) {
+			foreach ( $subobjects as $subobject ) {
+				$this->addSubobject( $NumericPropertyId, $subobject );
+			}
+		}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/SMW/SemanticEntity.php` around lines 68 - 74, The add(self $entity)
method currently only merges $entity->dataItemsPerProperty, which drops
subobjects; update add() to also merge subobjects by iterating
$entity->subobjectsPerProperty and calling
$this->addSubobject($NumericPropertyId, $subobject) (or otherwise merging
entries into $this->subobjectsPerProperty) for each subobject so
functionalMerge() preserves subobjects added via addSubobject(); reference the
add(), addSubobject(), dataItemsPerProperty and subobjectsPerProperty symbols
when making the change.

Expand Down
9 changes: 7 additions & 2 deletions src/SemanticDataUpdate.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use SMW\SemanticData;
use Title;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Entity\NumericPropertyId;
use Wikibase\DataModel\Services\Lookup\ItemLookup;
use Wikibase\DataModel\Services\Lookup\PropertyLookup;

Expand Down Expand Up @@ -63,8 +63,13 @@ private function newItemTranslator( Title $title ): ItemTranslator {
}

private function getSemanticEntityForPropertyTitle( Title $title ): SemanticEntity {
wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode($title));
wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode($title->getText()));
wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode(new NumericPropertyId( $title->getText() )));
wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode($this->propertyLookup->getPropertyForId( new NumericPropertyId( $title->getText() ) )));

return $this->newPropertyTranslator( $title )->translateProperty(
$this->propertyLookup->getPropertyForId( new PropertyId( $title->getText() ) )
$this->propertyLookup->getPropertyForId( new NumericPropertyId( $title->getText() ) )
);
Comment on lines +66 to 73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove temporary debug logging and avoid duplicate property lookups.

Lines 66-70 add verbose debug output and perform an extra getPropertyForId call before Line 72 repeats the same lookup. This adds overhead on every property-page update path.

Suggested cleanup
 private function getSemanticEntityForPropertyTitle( Title $title ): SemanticEntity {
-		wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode($title));
-		wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode($title->getText()));
-		wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode(new NumericPropertyId( $title->getText() )));
-		wfDebug(__METHOD__. "swb: getSemanticEntity:".json_encode($this->propertyLookup->getPropertyForId( new NumericPropertyId( $title->getText() ) )));
-		
-		return $this->newPropertyTranslator( $title )->translateProperty(
-			$this->propertyLookup->getPropertyForId( new NumericPropertyId( $title->getText() ) )
-		);
+		$propertyId = new NumericPropertyId( $title->getText() );
+		$property = $this->propertyLookup->getPropertyForId( $propertyId );
+
+		return $this->newPropertyTranslator( $title )->translateProperty( $property );
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/SemanticDataUpdate.php` around lines 66 - 73, Remove the temporary
wfDebug calls and the duplicate property lookup: create a NumericPropertyId once
from $title->getText(), call $this->propertyLookup->getPropertyForId(...) once
and store the result in a local variable, then pass that variable into
$this->newPropertyTranslator($title)->translateProperty(...). Remove the four
wfDebug(...) lines and replace the repeated new NumericPropertyId /
getPropertyForId calls with the two local variables to avoid redundant work.

}

Expand Down
9 changes: 6 additions & 3 deletions src/SemanticWikibase.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,18 @@ protected function getPropertyTypeLookup(): PropertyDataTypeLookup {
}

public function registerProperties( PropertyRegistry $propertyRegistry ) {
wfDebug("SWB: register properties ".json_encode($this->getAllProperties()));
foreach ( $this->getAllProperties() as $property ) {
$propertyRegistry->registerProperty(
$property->getId(),
$property->getType(),
$property->getLabel(),
true,
false
true, #is_visible
true, #is_annotable
false #is_declarative
);

wfDebug("SWB: register property ".$property->getId());
wfDebug("SWB: register property ".$property->getType());
foreach ( $property->getAliases() as $alias ) {
Comment on lines +65 to 77
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid duplicate property retrieval and remove transient debug logs.

Line 65 computes getAllProperties() for logging, and Line 66 computes it again for iteration. Also, Lines 65/75/76 add noisy debug output in a core registration path.

Suggested cleanup
 public function registerProperties( PropertyRegistry $propertyRegistry ) {
-		wfDebug("SWB: register properties ".json_encode($this->getAllProperties()));
-		foreach ( $this->getAllProperties() as $property ) {
+		$properties = $this->getAllProperties();
+		foreach ( $properties as $property ) {
 			$propertyRegistry->registerProperty(
 				$property->getId(),
 				$property->getType(),
 				$property->getLabel(),
 				true, `#is_visible`
 				true, `#is_annotable`
 				false `#is_declarative`
 			);
-			wfDebug("SWB: register property ".$property->getId());
-			wfDebug("SWB: register property ".$property->getType());
 			foreach ( $property->getAliases() as $alias ) {
 				$propertyRegistry->registerPropertyAlias( $property->getId(), $alias );
 			}
 		}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/SemanticWikibase.php` around lines 65 - 77, Store the result of
$this->getAllProperties() in a local variable and iterate that instead of
calling getAllProperties() twice; remove the transient wfDebug debug calls
around the registration (the wfDebug that json_encodes properties and the two
wfDebug calls printing property id/type), keeping only necessary logging if any;
update the loop that calls $propertyRegistry->registerProperty(...) and the
foreach over $property->getAliases() to use the saved variable (e.g.,
$allProperties) to avoid duplicate retrieval and noisy logs.

$propertyRegistry->registerPropertyAlias( $property->getId(), $alias );
}
Expand Down
88 changes: 84 additions & 4 deletions src/Translation/DataValueTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,33 @@
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Entity\NumericPropertyId;
use Wikibase\EDTF\Services\TimeValueBuilder;
use Wikibase\EDTF\EdtfValue;
use MediaWiki\Logger\LoggerFactory;
use EDTF\EdtfFactory;
use EDTF\Model\ExtDate;
use EDTF\Model\ExtDateTime;
use EDTF\Model\Interval;

class DataValueTranslator {

public function translate( TypedDataValue $typedValue ): SMWDataItem {


$value = $typedValue->getValue();

$propertyType = $typedValue->getPropertyType();
if( $value != null) {
wfDebug( 'swb: translate: '.get_class( $value ).' ptype: '.$propertyType );
}

if ( $value instanceof StringValue ) {
wfDebug( 'swb: translate: '. $typedValue->getValue()->getValue() );
if( $propertyType == 'edtf') {
return $this->translateEDTF("".$value->getValue());
}else if ($propertyType == 'localMedia') {
return $this->translateLocalMedia("".$value->getValue());
}
return $this->translateStringValue( $typedValue );
}
if ( $value instanceof BooleanValue ) {
Expand All @@ -44,10 +63,12 @@ public function translate( TypedDataValue $typedValue ): SMWDataItem {
if ( $value instanceof GlobeCoordinateValue ) {
return $this->translateGlobeCoordinateValue( $value );
}

if ( $value instanceof TimeValue ) {
return $this->translateTimeValue( $value );
}



throw new \RuntimeException( 'Support for DataValue type "' . get_class( $value ) . '" not implemented' );
}

Expand All @@ -66,6 +87,12 @@ private function translateGlobeCoordinateValue( GlobeCoordinateValue $globeValue
);
}

private function translateLocalMedia( String $imagePage): SMWDataItem {
return new DIWikiPage(
$imagePage,
NS_FILE
);
}
private function translateEntityIdValue( EntityIdValue $idValue ): SMWDataItem {
return new DIWikiPage(
$idValue->getEntityId()->getSerialization(),
Expand All @@ -78,7 +105,7 @@ private function entityIdToNamespaceId( EntityId $idValue ): int {
return WB_NS_ITEM;
}

if ( $idValue instanceof PropertyId ) {
if ( $idValue instanceof NumericPropertyId ) {
return WB_NS_PROPERTY;
}

Expand All @@ -89,6 +116,59 @@ public function translateDecimalValue( DecimalValue $value ): SMWDataItem {
return new \SMWDINumber( $value->getValueFloat() );
}

private function translateEDTF( String $value ): \SMWDITime {
wfDebug( 'swb: translate edtf' );
$tvb = new TimeValueBuilder( EdtfFactory::newParser() );
$tvArr = $tvb->edtfToTimeValues( $value );
$parser = \EDTF\EdtfFactory::newParser();
$parsingResult = $parser->parse($value);
wfDebug($parsingResult->isValid()); // true
$edtfValue = $parsingResult->getEdtfValue(); // \EDTF\EdtfValue
$humanizer = \EDTF\EdtfFactory::newHumanizerForLanguage( 'en' );
wfDebug($humanizer->humanize($edtfValue)); // string
wfDebug(get_class($edtfValue)); // string
$result = null;

if ( $edtfValue instanceof Interval ) {
if($edtfValue->hasStartDate()){
$edtfValue = $edtfValue->getStartDate();
} else if ($edtfValue->hasEndDate()){
$edtfValue = $edtfValue->getEndDate();
} else {
wfDebug('ERROR: unable to translate empty edtf interval to smw date');
return null;
}
}
Comment on lines +119 to +141
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Type error: Method returns null but declares return type \SMWDITime.

Line 139 returns null when an EDTF interval has neither start nor end date, but the method signature declares \SMWDITime as the return type. This will cause a TypeError at runtime.

🐛 Proposed fix - change return type to nullable
-	private function translateEDTF( String $value ): \SMWDITime {
+	private function translateEDTF( String $value ): ?\SMWDITime {

Additionally, the caller in translate() should handle the null return to avoid propagating issues.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private function translateEDTF( String $value ): \SMWDITime {
wfDebug( 'swb: translate edtf' );
$tvb = new TimeValueBuilder( EdtfFactory::newParser() );
$tvArr = $tvb->edtfToTimeValues( $value );
$parser = \EDTF\EdtfFactory::newParser();
$parsingResult = $parser->parse($value);
wfDebug($parsingResult->isValid()); // true
$edtfValue = $parsingResult->getEdtfValue(); // \EDTF\EdtfValue
$humanizer = \EDTF\EdtfFactory::newHumanizerForLanguage( 'en' );
wfDebug($humanizer->humanize($edtfValue)); // string
wfDebug(get_class($edtfValue)); // string
$result = null;
if ( $edtfValue instanceof Interval ) {
if($edtfValue->hasStartDate()){
$edtfValue = $edtfValue->getStartDate();
} else if ($edtfValue->hasEndDate()){
$edtfValue = $edtfValue->getEndDate();
} else {
wfDebug('ERROR: unable to translate empty edtf interval to smw date');
return null;
}
}
private function translateEDTF( String $value ): ?\SMWDITime {
wfDebug( 'swb: translate edtf' );
$tvb = new TimeValueBuilder( EdtfFactory::newParser() );
$tvArr = $tvb->edtfToTimeValues( $value );
$parser = \EDTF\EdtfFactory::newParser();
$parsingResult = $parser->parse($value);
wfDebug($parsingResult->isValid()); // true
$edtfValue = $parsingResult->getEdtfValue(); // \EDTF\EdtfValue
$humanizer = \EDTF\EdtfFactory::newHumanizerForLanguage( 'en' );
wfDebug($humanizer->humanize($edtfValue)); // string
wfDebug(get_class($edtfValue)); // string
$result = null;
if ( $edtfValue instanceof Interval ) {
if($edtfValue->hasStartDate()){
$edtfValue = $edtfValue->getStartDate();
} else if ($edtfValue->hasEndDate()){
$edtfValue = $edtfValue->getEndDate();
} else {
wfDebug('ERROR: unable to translate empty edtf interval to smw date');
return null;
}
}
🧰 Tools
🪛 PHPMD (2.15.0)

[warning] 122-122: Avoid unused local variables such as '$tvArr'. (undefined)

(UnusedLocalVariable)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Translation/DataValueTranslator.php` around lines 119 - 141, The method
translateEDTF currently declares it returns \SMWDITime but can return null (see
the early return in translateEDTF when an EDTF interval has no start/end);
change the signature of translateEDTF to return a nullable type (?\\SMWDITime)
and update any callers (notably translate()) to handle a null result (e.g. check
for null and bail or propagate an appropriate error) so no TypeError occurs at
runtime.


if( $edtfValue instanceof ExtDate ) {
$result = new \SMWDITime(
SMWDITime::CM_GREGORIAN,
$edtfValue->getYear(),
$edtfValue->getMonth(),
$edtfValue->getDay()
);

} else if ( $edtfValue instanceof ExtDateTime ) {
$result = new \SMWDITime(
SMWDITime::CM_GREGORIAN,
$edtfValue->getYear(),
$edtfValue->getMonth(),
$edtfValue->getDay(),
$edtfValue->getHour(),
$edtfValue->getMinute(),
$edtfValue->getSecond()
);
} else {
$result = new \SMWDITime(
SMWDITime::CM_GREGORIAN,
1970
);

}
Comment on lines +161 to +167
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Silent fallback to epoch date (1970) can cause data integrity issues.

When $edtfValue is neither ExtDate, ExtDateTime, nor Interval, the code silently falls back to January 1, 1970. This masks parsing failures and corrupts data without any indication to the user.

Consider logging a warning or throwing an exception for unexpected types.

🔧 Proposed fix
 		} else {
-		$result = new \SMWDITime(
-				SMWDITime::CM_GREGORIAN,
-				1970
-			);
-				
+			wfDebug( 'swb: WARNING - Unhandled EDTF type: ' . get_class($edtfValue) . ', falling back to epoch' );
+			$result = new \SMWDITime(
+				SMWDITime::CM_GREGORIAN,
+				1970
+			);
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} else {
$result = new \SMWDITime(
SMWDITime::CM_GREGORIAN,
1970
);
}
} else {
wfDebug( 'swb: WARNING - Unhandled EDTF type: ' . get_class($edtfValue) . ', falling back to epoch' );
$result = new \SMWDITime(
SMWDITime::CM_GREGORIAN,
1970
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Translation/DataValueTranslator.php` around lines 161 - 167, The else
branch in DataValueTranslator where $edtfValue falls through to a hardcoded
SMWDITime( CM_GREGORIAN, 1970 ) must be changed to surface unexpected types
instead of silently returning epoch; update the block in the function handling
$edtfValue (the branch that currently creates SMWDITime for unknown types) to
either throw a clear exception (e.g. InvalidArgumentException) or emit a warning
via the project's logger including the actual type (get_class($edtfValue) or
gettype) and return a safe null/empty value; ensure you reference the symbols
$edtfValue, ExtDate, ExtDateTime, Interval and SMWDITime when implementing the
change so callers can handle the new error path.


return $result;
}

private function translateTimeValue( TimeValue $value ): \SMWDITime {
$components = ( new TimeValueParser() )->parse( $value->getTime() );

Expand Down
4 changes: 2 additions & 2 deletions src/Translation/ItemTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ public function __construct( FingerprintTranslator $fingerprintTranslator, State
$this->statementListTranslator = $statementListTranslator;
}

public function translateItem( Item $item ): SemanticEntity {
public function translateItem( ?Item $item ): SemanticEntity {
$semanticEntity = new SemanticEntity();

if ( $item->getId() === null ) {
if ( $item === null || $item->getId() === null ) {
return $semanticEntity;
}

Expand Down
8 changes: 4 additions & 4 deletions src/Translation/PropertyTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use MediaWiki\Extension\SemanticWikibase\SMW\SemanticEntity;
use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Entity\NumericPropertyId;

class PropertyTranslator {

Expand All @@ -18,10 +18,10 @@ public function __construct( FingerprintTranslator $fingerprintTranslator, State
$this->statementListTranslator = $statementListTranslator;
}

public function translateProperty( Property $property ): SemanticEntity {
public function translateProperty( ?Property $property ): SemanticEntity {
$semanticEntity = new SemanticEntity();

if ( $property->getId() === null ) {
if ( $property === null || $property->getId() === null ) {
return $semanticEntity;
}

Expand All @@ -32,7 +32,7 @@ public function translateProperty( Property $property ): SemanticEntity {
return $semanticEntity;
}

private function addId( SemanticEntity $semanticEntity, PropertyId $itemId ): void {
private function addId( SemanticEntity $semanticEntity, NumericPropertyId $itemId ): void {
$semanticEntity->addPropertyValue(
FixedProperties::ID,
new \SMWDIBlob( $itemId->getSerialization() )
Expand Down
Loading