Last modified: 2011-12-14 22:42:30 UTC
The file SemanticMediaWiki/includes/storage/SMW_QueryResult.php has this: 435: $recordValue = SMWDataValueFactory::newDataItemValue( $diContainer, $propertyValue->getDataItem() ); 436: $dataItems = $recordValue->getDataItems(); But according to the class reference (http://semantic-mediawiki.org/doc/classSMWDataValueFactory.html) newDataItemValue returns an SMWDataValue, and this in turn has no method getDataItems()! This problem breaks the runjobs and refresh scripts. The error message: PHP Fatal error: Call to undefined method SMWQueryCallMetadataValue::getDataItems() in .../extensions/SemanticMediaWiki/includes/storage/SMW_QueryResult.php on line 432 (Apparently, the only class claiming to extend SMWDataValue that actually includes the method is SMWRecordValue.) The SMWQueryCallMetadataValue is a class in the Halo extension that extends SMWDataValue and (correctly) does not implement getDataItems(). My setup: MediaWiki 1.17.0 PHP 5.3.3-1ubuntu9.5 (apache2handler) Semantic MediaWiki (Version 1.6.1) SMWHalo Extension (Version 1.6.0_5 [B18])
Not sure what's going wrong, but the issue is not that the object you ought to get there does not have a getDataItems method. You should get a SMWRecordValue, which derives from SMWDataValue, but clearly you are not. Records are working fine for me on trunk (but then again, a bunch of work was done since 1.6.1 affecting this code). So either this issue has been very likely fixed, or it's coming from and extension to SMW that is doing stuff incorrectly.
The error is that the SMW code just assumes that the SMWDataValue it gets has the method getDataItems, even though that method is not in the interface. True, the class SMWRecordValue has it, but where does it say SMWDataValueFactory::newDataItemValue has to return one of those?
In any case, I made a nasty little workaround: if (method_exists($recordValue, 'getDataItems')) { $dataItems = $recordValue->getDataItems(); if ( array_key_exists( $pos, $dataItems ) && ( $dataItems[$pos] !== null ) ) { $newcontent[] = $dataItems[$pos]; } }
> True, the class SMWRecordValue has it, but where does it say SMWDataValueFactory::newDataItemValue has to return one of those? It should if you pass it the correct arguments. You seem to be suggesting adding a method for every data type we handle. This is not how dynamic languages work. And it's done all through MediaWiki code.
> It should if you pass it the correct arguments. Then please update the class reference: http://semantic-mediawiki.org/doc/classSMWDataValueFactory.html because right now it says: static SMWDataValueFactory::newDataItemValue(SMWDataItem $dataItem, $property, $caption = false) Returns: SMWDataValue Which is actually what it does now, but apparently not what SMWQueryResult expects. The problem is in SMW itself, not in the Halo extension. > You seem to be suggesting adding a method for every data type we handle. Not at all, I'm just saying there is a mismatch between what SMWQueryResult expects (something with getDataItems) and what SMWDataValueFactory produces (SMWDataValue), and this leads to problems.
> Which is actually what it does now, but apparently not what SMWQueryResult expects. SMWQueryResult expects a specific case. If you have an adittion method that returns an int no matter what you feed it, you document it as returning an int. If you then have code that passes 2 and 3 and expects 5, the method is not wrongly documented. > The problem is in SMW itself, not in the Halo extension. What makes you think this? Can you reproduce it without Halo? Can you reproduce it on trunk?
The method is documented as returning an object of type SMWDataValue, and that type does not have a method getDataItems. I hope we can agree on that bit at least? > What makes you think this? The Halo extension has a class SMWURIIntegrationValue that extends SMWDataValue, and that class does not implement "getDataItems". And then, why should it? Such an object is returned by SMWDataValueFactory, and that is when the code gets a fatal error and just dies. I have no clue why SMWDataValueFactory returns that instead of an SMWRecordValue, but according to the documentation it is in its right to do that.
> I hope we can agree on that bit at least? Yeah, we can. I'm assuming you are mentioning it because you have a problem with it though, while I think it's perfectly fine. > And then, why should it? You can keep complaining about this, but I am not putting in a check like you wrote as it seems to treat a symptom rather them fixing the actual issue. > I have no clue why SMWDataValueFactory returns that instead of an SMWRecordValue This just makes me think it actually is Halo doing something wrong. If you can give me some set of actions to do to reproduce this using extensions that I can get from the WMF repo, I'll be happy to debug it for you and create a fix where applicable.
> You can keep complaining about this, but I am not putting in a check like you wrote as it seems to treat a symptom rather them fixing the actual issue. Of course not! I just thought I'd share it as a dirty workaround in case someone else has the same problem (http://www.xkcd.com/979/). It just bugs me that the caller is assuming things that are not consistent with the documentation. > This just makes me think it actually is Halo doing something wrong. > If you can give me some set of actions to do to reproduce this using > extensions that I can get from the WMF repo, I'll be happy to debug it > for you and create a fix where applicable. OK I'll do my best!
