Hi, In case we want to move forward with the change to AbstractDataStoreService, I’ve created a pull request [0] that outlines what I think the changes would be to that class. This PR should basically cover the extent of the changes to existing classes that this refactor entails.
I ran the full test suite with the change, and also ran a couple of integration level tests at my disposal testing this change with FileDataStore and S3DataStore. As best I can tell everything checks out. Please review and provide feedback, thanks in advance. Or if we don’t want the risk at this stage, I can explore other options. [0] - https://github.com/apache/jackrabbit-oak/pull/71 -MR On October 26, 2017 at 10:24:53 AM, Matt Ryan (o...@mvryan.org) wrote: Hi, In another thread there’s a discussion going on regarding the implementation of CompositeDataStore and the need for a factory class in order to have multiple data stores of the same type used by the composite. I’ve been looking at how this is done by SegmentNodeStore as an example. In the case of SegmentNodeStore, both SegmentNodeStoreService and SegmentNodeStoreFactory share much of the logic for registering the segment store. To follow this pattern with the data stores is a bit more complex, since most of the data store services inherit from AbstractDataStoreService. It would require a non-trivial refactor of AbstractDataStoreService plus possibly the creation of a companion AbstractDataStoreFactory class that the subsequent data store implementations would have to implement if they are to be used with the composite (e.g. FileDataStoreFactory extends AbstractDataStoreFactory) or something like that. My question: Is this something we want to try to do at this point for the CompositeDataStore? It may be the “best” approach in terms of code organization, but it also makes the change much broader and far-reaching, which means more risk at this stage for Oak 1.8 (or 2.0 or whatever). An alternative would be to make each factory (e.g. FileDataStoreFactory) completely independent, sharing little to no code with the current implementation, which will likely have a lot of duplicate code, but will also make the CompositeDataStore addition more clean and less risky. If we want to move forward with the refactor of AbstractDataStoreService and formalize data store factory creation e.g. AbstractDataStoreFactory, I could start that in a separate branch and issue it as a separate PR from the composite data store. WDYT? -MR
