Merge lp://qastaging/~mhr3/dee/more-props into lp://qastaging/dee
Proposed by
Michal Hruby
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Mikkel Kamstrup Erlandsen | ||||
Approved revision: | 364 | ||||
Merged at revision: | 353 | ||||
Proposed branch: | lp://qastaging/~mhr3/dee/more-props | ||||
Merge into: | lp://qastaging/dee | ||||
Prerequisite: | lp://qastaging/~mhr3/dee/more-tests | ||||
Diff against target: |
890 lines (+512/-30) 10 files modified
src/dee-peer.c (+67/-4) src/dee-peer.h (+10/-8) src/dee-shared-model.c (+92/-11) src/dee-shared-model.h (+28/-3) tests/Makefile.am (+11/-3) tests/model-helper-append1.c (+55/-0) tests/model-helper-replace.c (+70/-0) tests/test-client-server.c (+63/-1) tests/test-model-interactions.c (+106/-0) vapi/dee-1.0.vapi (+10/-0) |
||||
To merge this branch: | bzr merge lp://qastaging/~mhr3/dee/more-props | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mikkel Kamstrup Erlandsen (community) | Approve | ||
Review via email:
|
Description of the change
Added extra properties to help us deal with the problem of remote peers writing to a model which they shouldn't.
Added tests for the new properties. UNBLOCK
To post a comment you must log in.
Why rename *self to *peer in all method args? That goes the conventions used elsewhere in Dee..?
272 + * Note that this does NOT mean that the peer is owner of the swarm! Check also is_swarm_ leader( ).
273 + * dee_peer_
s/peer is owner/peer is leader/
375 /** disable- remote- writes:
376 + * DeeSharedModel:
377 + *
378 + * Boolean property defining whether or not the model will accept changes
379 + * done by remote peers.
380 + *
381 + * This is useful when one process is considered an "owner" of a model and
382 + * all the other peers are supposed to only synchronize it for reading.
383 + * Note that this will only have effect if the associated peer is swarm
384 + * leader.
385 + */
Can you add a cross ref to the DeePeer:swarm-owner property
On "disable- remote- writes" : I think we need to turn this into a set of flags or an enum in order to ensure forward compat. And enum is probably best. This way we can prevent us getting into an n! combinatorial explosion (or worse, semantic ambiguity) if we need to add new access modes.
I am thinking; call the property "access-mode" and define a DeeSharedModelA ccessMode enumeration with two values (for now) DEE_SHARED_ MODEL_ACCESS_ MODE_WORLD_ WRITABLE (the default value) and DEE_SHARED_ MODEL_ACCESS_ MODE_LEADER_ WRITABLE.
We might also want to add client side validation on the writability of the model... (Just logging a critical and leaving the model unchanged) In order to catch programming errors more easily and not leading to desktop crunch if we get into a Invalidate/Clone loop... I think this could be done cleanly in DeeProxyModel with a "writable" boolean property (which we can then bind to "is leader" to state of the DeeSharedModel)
And finally - does the testing also apply against the p2p mode of DeeSharedModel? I think that needs to be validated...