Improve Statement/Claim GUID handling
Closed this issue · 15 comments
Right now the way Claim GUIDs are handled is distinctly sub-optimal.
One cannot add a Statement to an Item if the Claim of the Statement does not have a GUID. In many cases one does not care about the guid, and might not have one. This happens often in tests, and is very annoying.
One question we have to answer is if GUIDs should be assigned to Claims or Statements.
And this even creates another question: Will claims be used as "front end objects" and can they stand alone or are they always connected to a statement. For the first, the GUID should be assigned to the claim and for the second, it makes only sense to add a GUID to a statement.
Currently, we only have statements being able to be added to entities and I think this will remain so in future so I'd propose to make the GUID a property of the statement and consider Claim only as a data object without any further context assigned.
I'm inclined to agree with moving the GUID to Statement. If at a future point we want to expose something that has the data of a Claim and also has an identity, then we can always create a new object for that.
@mkroetzsch I know we talked about this before. What was your opinion on having a GUID in Statement or Claim? I suspect it was along the line of what I wrote above, though am not sure.
Maybe it would be possible to allow claims without a GUID, and leave it to the ClaimSerializer to assign one on the fly if none is present yet. We should never serialize a claim without a GUID, to the database or API.
However, to create a valid Claim GUID, you have to know the item ID. It would have to be handed down to the ClaimSerializer somehow, perhaps as a parameter to the serialize() method. This would require us to scrap the notion that all serializers implement a shared interface, but I don't see a problem with that.
@brightbyte Thoughts on the questions @Benestar raised?
Why is there a need for claims to remain at all? It does not seem to capture a logical purpose anymore. I understand that code complexity might benefit to a certain extend but the logical complexity would decrease considerably when dropping Claim. Right now, users of the component need to understand the term and concept of "Claim" in addition to "Statement" without that resulting in any benefit. (The same argument applies on Fingerprint btw.)
I'd like to second that question. I always have to look up which parts are in the Claim and which in the Statement.
@snaterlicious @adrianlang That is also a question that needs answering and is very relevant for how we get rid of the inheritance between Claim and Statement. (If we don't need Claim, the way forward is quite clear.)
If we can indeed remove Claim, that'd be great. Unlike @snaterlicious suggests, the existence of Claim increases the complexity of the code, so there is incentive to remove it from that angle, rather than to keep it.
In addition, it'd settle the question of where the GUID belongs.
To proceed with that, we need to verify removing Claim does not cause any issues. While it does not contain any real logic in DM that cannot go into Statement, there might be code elsewhere that relies on its existence. For instance, you might have a StatementView that contains a ClaimView or something like that. You'd also need to remove the ClaimView obviously, though there might be good reason to have such a view.
I'm not aware of any concrete reasons to not remove Claim altogether, and figured I needed to ask you two in particular about usage in the fronted. Are you saying you also do not see any reason to not remove it altogether?
Does anyone else see a problem with removing Claim?
@lydiapintscher says Claim probably not needed, @brightbyte says Claim not needed.
Dedicated issue created no #315
Please still do raise concerns if you have any.
There is jQuery.wikibase.claimview in the front-end and as a result of the additional complexity it is creating in the front-end, it is causing more trouble than benefit. Since, after changing from inheritance to composition, jQuery.wikibase.claimview and jQuery.wikibase.statementview need to be maintained in a more individual fashion, the existence of jQuery.wikibase.claimview is binding resources for no good reason. I do not see a reason to keep jQuery.wikibase.claimview; front-end would definitely benefit from removing the concept of Claim.
The initial work to remove Claim is now submitted in #317.
Strong support for a way to use statements with no GUID.
By the way, this is very similar to #310/#312, the question if we should support Entitys with no ID or enforce one. I suggest to be consistent in these cases.
@Benestar: Neither nor. Claim should just die.
@JeroenDeDauw: Not exactly sure what you mean with "move GUID to Statement". Currently Statement extends Claim and therefor already does have a GUID. No need to move anything. The questions we are asking (should Statement inherit or composite Claim or should Claim just die) have nothing to do with the GUID. GUID support is there. Are we talking about removing the currently existing GUID support from Statement?
@snaterlicious and @adrianlang: 100% agree to what you say but I wonder why you never responded to #264/#268 (a month old) where I raise exactly this question. Frustrating.
@thiemowmde Yes, we should deal with GUIDs in the same way as we deal with Entity IDs: we require them to be present when saving to the database (though I suggest that at least for Claim/Statement GUIDs, we should be a bit structer and require them for all serialization). TI should be possibel to construct and use Statements without GUIDs, and also place them in Items (are we using any collections that use the GUID as a key? That would be a problem).
Search for ->getGuid()] and $guid] in our code base. GUIDs are used as array keys in Claims and ClaimsTest and in the StatementListDiffer and StatementListPatcher classes.