Update patch set 8
Patch Set 8:
I'm note sure what I can add what I haven't already said. Also note that I already moved this patch to an other, future ticket: T166691.
I cannot really see that it will/should be used in some API code […]
How are you going to deserialize the Lexeme serializations users are providing via the wbeditentity API then? You would need to duplicate the deserializer then, and the only difference will be the check we are discussing here.
What is the point of such a duplication? You talk about safety. Sure. I'm not opposing this. But this safety check is done on the wrong level. It can not be the job of a deserializer to check if the database is consistent or not. How could it? This class can not know. It does not have access to the database. Even if you make the nextFormId field non-optional, it can still contain a number that must be considered invalid later when trying to store this Lexeme to the database.
- Lexeme L1 contains two Forms, F1 and F2. nextFormId is 3.
- F2 is deleted. nextFormId is still 3.
- The LexemeDeserializer gets an invalid serialization for L1 with F1, and nextFormId set to 2.
- How can the LexemeDeserializer know this is invalid?
In this scenario the provided nextFormId is worthless. It's not better than just guessing the nextFormId based on the maximum Form ID.
So what's the point of making the field non-optional, if this restriction doesn't make anything better?
I used the term "brain-dead". The only way to make this Deserializer not "brain-dead" any more is to connect it to the database, which I hope we agree is the wrong approach. This must be done further down in the actual storage layer. The deserializer infrastructure is not part of that.