[jira] [Comment Edited] (OAK-6378) Move the SegmentWriter API to its own interface

["Francesco Mari (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/OAK-6378?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16058952#comment-16058952 ] 

Francesco Mari edited comment on OAK-6378 at 6/22/17 8:19 AM:
--------------------------------------------------------------

I think that there is value in explicitly stating what a record needs to provide its services. In my opinion, hiding these dependencies is what makes the code in the record implementations hard to understand. Regarding your comment about some record implementations not being stateless, I would argue that they actually are. The most compelling reason why {{SegmentWriter}} and {{BlobStore}} are passed around is the need to create a {{SegmentNodeBuilder}} from a {{SegmentNodeState}}.

Anyway, I already independently started to explore the {{RecordFactory}} solution or, at least, the first steps towards it. In [this branch|https://github.com/francescomari/jackrabbit-oak/tree/record-package], based on the one linked in the description, I moved the record classes to their own package. The implementation of the record classes is now mostly based on {{SegmentReader}}. When needed, {{SegmentWrier}} and {{BlobStore}} are provided as well. What do you think about it?


was (Author: frm):
I think that there is value in explicitly stating what a record needs to provide its services. In my opinion, hiding these dependencies is what makes the code in the record implementations hard to understand. Regarding your comment about some record implementations not being stateless, I would argue that they actually are. The most compelling reason why {{SegmentWriter}} and {{BlobStore}} are passed around is the need to create a {{SegmentNodeBuilder}} from a {{SegmentNodeState}}.

Anyway, I already independently started to explore the {{RecordFactory}} solution or, at lease, the first steps towards it. In [this branch|https://github.com/francescomari/jackrabbit-oak/tree/record-package], based on the one linked in the description, I moved the record classes to their own package. The implementation of the record classes is now mostly based on {{SegmentReader}}. When needed, {{SegmentWrier}} and {{BlobStore}} are provided as well. What do you think about it?

> Move the SegmentWriter API to its own interface
> -----------------------------------------------
>
>                 Key: OAK-6378
>                 URL: https://issues.apache.org/jira/browse/OAK-6378
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: segment-tar
>            Reporter: Francesco Mari
>            Assignee: Francesco Mari
>             Fix For: 1.8, 1.7.3
>
>
> In order to isolate {{SegmentWriter}} from the rest of the implementation, and to match the corresponding {{SegmentReader}}, I extracted the API exposed by {{SegmentWriter}} to its own interface and moved its implementation in a separate class.
> Moreover, I cleaned up the {{SegmentWriter}} a bit by letting. every method of its interface always return a {{RecordId}}. Before the refactoring, some methods returned concrete implementations of record classes. The cleanup improves the uniformity of the {{SegmentWriter}} interface.
> I see potential in this change for the following reasons.
> * The concrete record implementations ({{SegmentNodeState}}, {{Template}}, {{MapRecord}}, etc.) might be implemented directly on top of the {{SegmentReader}} and {{SegmentWriter}} API, moved to a different package and tested separately from the rest of the code.
> * {{SegmentWrier}} and {{SegmentReader}} provide a higher level API that isolates the {{SegmentStore}} and its supporting classes. Code using only {{SegmentWriter}} and {{SegmentReader}} might be able to use {{RecordId}} instances as opaque handles to the underlying records, with beneficial effects on code decoupling.
> I have a working version of the refactoring in [this branch on GitHub|https://github.com/francescomari/jackrabbit-oak/tree/segment-writer].



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)