[PUB-1825, PUB-1826] Add spec for applying incoming OBJECT messages #343
Conversation
Spec IDs from [1]. This also fixes a couple of minor spec implementation details. [1] ably/specification#343
VeskeR
changed the title
Based on [1] at 2b5651e. Development approach as described in 4494033. [1] ably/specification#343
Based on [1] at 2b5651e. Development approach as described in 4494033. [1] ably/specification#343
VeskeR
force-pushed
the
objects-operation-application
branch
from
2b5651e to
29276a5
Compare
Spec IDs from [1]. This also fixes a couple of minor spec implementation details. [1] ably/specification#343
Based on [1] at 29276a5. Development approach as described in 4494033. [1] ably/specification#343
This applies the changes from [1] at 29276a5. Development approach as described in 4494033. [1] ably/specification#343
Based on [1] at 29276a5. I wrote the implementation. We have a separate issue for applying RTO8a's buffering during a sync, so I haven't done that here. [1] ably/specification#343
Based on [1] at 29276a5. I wrote the implementation, and for the tests followed the development approach described in 4494033. We have a separate issue for applying RTO8a's buffering during a sync, so I haven't done that here. [1] ably/specification#343
This is based on the values stated in RTOL3 of [1] at 29276a5. (We'll formalise the fact that these are common LiveObject properties in an upcoming commit.) [1] ably/specification#343
Based on [1] at 29276a5. [1] ably/specification#343
Based on [1] at 29276a5. Development approach as described in cb427d8. [1] ably/specification#343
This applies the changes from [1] at 29276a5. Development approach as described in cb427d8. [1] ably/specification#343
Based on [1] at 29276a5. I wrote the implementation, and for the tests followed the development approach described in cb427d8. We have a separate issue for applying RTO8a's buffering during a sync, so I haven't done that here. [1] ably/specification#343
Based on [1] at 29276a5. I wrote the implementation, and for the tests followed the development approach described in cb427d8. We have a separate issue for applying RTO8a's buffering during a sync, so I haven't done that here. [1] ably/specification#343
Based on [1] at 29276a5. I wrote the implementation, and for the tests followed the development approach described in cb427d8. [1] ably/specification#343
Based on [1] at 29276a5. I wrote the implementation, and for the tests followed the development approach described in cb427d8. [1] ably/specification#343
Based on [1] at 29276a5. I wrote the implementation, and for the tests followed the development approach described in cb427d8. [1] ably/specification#343
| ** @(RTLC7d)@ The @ObjectMessage.operation.action@ field (see "@ObjectOperationAction@":../features#OOP2) determines the type of operation to apply: | ||
| *** @(RTLC7d1)@ If @ObjectMessage.operation.action@ is set to @COUNTER_CREATE@, apply the operation as described in "RTLC8":#RTLC8, passing in @ObjectMessage.operation@ | ||
| *** @(RTLC7d2)@ If @ObjectMessage.operation.action@ is set to @COUNTER_INC@, apply the operation as described in "RTLC9":#RTLC9, passing in @ObjectMessage.operation.counterOp@ | ||
| *** @(RTLC7d3)@ Otherwise, log a warning that an object operation message with an unsupported action has been received, and discard the current @ObjectMessage@ without taking any action |
There was a problem hiding this comment.
In ably-js, we throw error for spec id RTLC7d3 => https://github.com/ably/ably-js/blob/837afe6585f68bd9fabf6bf8fd14b723258e9b30/src/plugins/objects/livecounter.ts#L215
There was a problem hiding this comment.
The ably-js implementation is incorrect, and I intend to change it.
Throwing an error does not adhere to the robustness principle that our SDKs should follow https://sdk.ably.com/builds/ably/specification/main/features/#RTF1, and ably-js LiveObjects implementation violates this principle in multiple places by throwing errors instead of logging a debug/warning message and ignoring the operation.
The spec is correct in this case
There was a problem hiding this comment.
Got it, in case of kotlin, we catch all types of exceptions at one place where processing of incoming liveobject message starts.
https://github.com/ably/ably-java/blob/993b308f93090c4d6ccda4870ac21cf64f41f7b9/liveobjects/src/main/kotlin/io/ably/lib/objects/DefaultRealtimeObjects.kt#L231-L244
There was a problem hiding this comment.
Maybe we can add an explicit liveobjects spec point for the same.
I don't think we should be that explicit in the spec. It is up to the implementation to implement it in the most convenient/idiomatic for the platform. If the spec instructs to log something at a specific point (and even that might change as a result of #374) it should be as concise as possible
There was a problem hiding this comment.
Maybe we can add an explicit liveobjects spec point for the same.
There was a problem hiding this comment.
I don't think a spec point like that would be specific to LiveObjects; it would establish generic logging rules for the entire spec.
Feel free to create an issue or open a PR to clarify the logging procedure in the spec, but I don't believe it should be included in this PR.
There was a problem hiding this comment.
I’ve added the same comment to the previous issue: #362
One of the problems was that ably-js was throwing an internal exception, which ideally should be caught. This could potentially happen in other SDKs unknowingly or runtime exceptions can be thrown. Introducing an explicit spec point for such a mechanism would help improve the overall robustness and consistency of the SDKs.
Spec IDs from [1]. This also fixes a couple of minor spec implementation details. [1] ably/specification#343
Spec IDs from [1]. This also fixes a couple of minor spec implementation details. [1] ably/specification#343
Spec IDs from [1]. This also fixes a couple of minor spec implementation details. [1] ably/specification#343
Spec IDs from [1]. This also fixes a couple of minor spec implementation details. [1] ably/specification#343
VeskeR
force-pushed
the
objects-access-api
branch
from
7791cd3 to
24e151e
Compare
VeskeR
force-pushed
the
objects-access-api
branch
from
24e151e to
08a837d
Compare
VeskeR
force-pushed
the
objects-operation-application
branch
from
4a1f7c6 to
6524bc0
Compare
VeskeR
force-pushed
the
objects-operation-application
branch
from
6524bc0 to
0aa15d6
Compare
VeskeR
force-pushed
the
objects-access-api
branch
from
0bff266 to
dba8ca4
Compare
Resolves PUB-1825, PUB-1826
VeskeR
force-pushed
the
objects-operation-application
branch
from
0aa15d6 to
84fc685
Compare
Resolves PUB-1825, PUB-1826