Revisit Relation contract. Implements #457 #524
Conversation
|
@lemonboston I want to handle the "reverse relation" issue in a separate story |
| * When writing a relation, exactly one of {@link #RELATED_ID}, {@link #RELATED_UID} or {@link #RELATED_URI} must be given. {@link #RELATED_CONTENT_URI} | ||
| * will be populated automatically if possible. | ||
| * </p> | ||
| * When writing a relation, exactly one of {@link #RELATED_ID} or {@link #RELATED_UID}. {@link #RELATED_CONTENT_URI} will be |
There was a problem hiding this comment.
Something is missing at the end of the first sentence here?
| @@ -37,23 +36,23 @@ | |||
| public final class RelationData extends DelegatingRowData<TaskContract.Properties> | |||
| { | |||
| public RelationData(@NonNull RowSnapshot<TaskContract.Tasks> relatingTask, | |||
| @NonNull RelType relType, | |||
| @NonNull int relType, | |||
There was a problem hiding this comment.
Primitive int as @NonNull here, should be Integer, I suppose.
There was a problem hiding this comment.
the @NonNull should be removed. I'll take care of that.
|
|
||
| if (id == null && uri == null && uid != null) | ||
| if (id == null && uid != null) |
There was a problem hiding this comment.
Shouldn't this validate() method just simply throw if none or both of RELATED_ID and RELATED_UID are present?
Isn't the value already null when values.putNull(Relation.RELATED_ID); is called?
There was a problem hiding this comment.
that would only be the case for new rows. When you override a row we need to reset the other value explicitly. That being said, I already considered making rows write-once. So you can only create and delete them. Updates would throw an exception. I'm starting to get used to this idea and it would certainly make a couple of things much simpler.
There was a problem hiding this comment.
I still don't get this - so just by looking at this code:
Long id = values.getAsLong(Relation.RELATED_ID);
String uid = values.getAsString(Relation.RELATED_UID);
if (id == null && uid != null)
{
values.putNull(Relation.RELATED_ID);
}and just ignore the 'uid' for a second and inline the 'id' variable, it becomes this:
if (values.getAsLong(Relation.RELATED_ID) == null)
{
values.putNull(Relation.RELATED_ID);
}So if the value is null we set it to null.. Or do I miss something?
There was a problem hiding this comment.
getAsLong also returns null if the value is absent (i.e. not not present in the ContentValues), so we need to explicitly put a null value to ensure we override the old value, otherwise it would remain unchanged.
* remove all the `public static final` modifiers * remove RELATED_URL * remove some currently unsupported RelTypes * convert RelTypes from enum to ints to allow unsupported reltypes.
bb539bb to
4d55666
Compare
|
@lemonboston ready for re-review. |
4d55666 to
42b3f80
Compare
public static finalmodifiers