hi
here the summary of some initial observations that i made while
looking at the oak-api today:
a) NodeState is never used in oak-jcr
as far as i saw the NodeState interface is never used in the
scope of oak-jcr in contrast to TransientNodeState that
is omnipresent.
basically i got the impression that TransientNodeState
isn't really about transient JCR modification but in fact
*is* the representation of the persisted MK node state on
the oak-API. oak-api consumers would never need to access
the mk-NodeState.
Suggestion(s):
- remove NodeState from oak-api
- move to 'kernel' package or really use the corresponding
interfaces from mk.model package
- still rename TransientNodeState to (variants following)
> NodeState
in this case i would rename KernelNodeState to KernelNode
as on mk level it is called Node again.
> NodeData, NodeInfo, Node* (to distinguish from jcr Node)
in this case NodeState would still refer to mk and
the MK-API should use the term NodeState instead of Node
> Apply the same pattern then to PropertyState
b) TransientState not extending from NodeState
at a first glance i found that pretty confusing. with the comment
made in a) that actually makes sense... but the name is confusing.
in fact that TransientNodeState already looks much more like a
JCR node than like a mk-NodeState as it refers to it's parent, has
an editor (-> see below) etc.
Suggestion(s):
-> see above.
c) NodeStateEditor : general
the name implies that it is a TransientNodeStateModifier
but in the implementation it is a mixture of ChangeLog|Set and
a ItemModified (not only nodes)... that looks like a confusion
of intents to me.
Suggestion(s):
- at least rename NodeStateEditor to something like Item*Modifier
or ...Editor where * would match whatever solution we come up
with in point a).
- if we made TransientNodeState the main node-like interface
on the oak-api that is really distinct from the NodeState
concept in mk and the core impl, i would like to suggest to
move the (few) modifier methods to that interface.
for the sake of an API that is easy to read and easy to use.
- The editor was then not used to edit states but could be e.g.
renamed to ChangeSet|Log (or similar). it's single
purpose was then to collect the changes.
- I don't know yet to which extend a consumer of the oak-api
really need to get in touch with that changeset. but it felt
natural to me to pass the changeset to the Connection#commit.
... but i would need to fiddle around with the code to get a
clear picture.
d) NodeStateEditor#getBaseNodeState() NodeState
this method is never used from a consumer of the oak-API.
to me that is again (see also NodeState interface) an indication
that we expose information that was in fact an implementation
detail... it's only the oak-core implementation that needs to
know that.
Suggestion:
- remove that method for the oak-api
- use implementation specific method or somehow add mk.model package.
e) NodeStore, NodeStateDiff, ChildNodeEntry:
similar to NodeState i am not sure if that those really needed and
properly located in oak-api. so far i didn't find any usage in
oak-api.
Suggestion:
- remove those classes from the oak-api until the API-consumer
(oak-jcr) would really use it or if we know for sure where
it will be used very soon.
- use mk.model interfaces instead in the oak-core implementations
that are in the kernel package.
f) PropertyState
as stated at point a) this interface currently serves two
purposes as it is used both in NodeState and TransientNodeState.
that looks like a mistake to me in the light of the 2 node state
interfaces not being connected by super-sub-interface relationship.
Suggestion:
- remove PropertyState as a notion of MK-property state from the
oak-api
- add a Property* interface where * would be according to the name
of the oak-api naming convention for item/states being exposed
by that api... right now the consistent name on oak-api probably
was "TransientPropertyState" ;)
so, looking at the findings in this first set of comments, i have
the impression that there are right now a lot of unused interfaces
in oak-api... unless someone objects i would like to go ahead
and clean up the oak-api package as this will help us getting
a clearer picture of what we have and what is really missing.
for the findings that need more fundamental changes (e.g.the
editor/changeset discussion), i would like to get your feedback
on whether that was feasible/desirable (in the first place from
michael, that was working on this).
kind regards
angela
