You make a good point about not touching anything. I agree totally about not modifying the behavior of storage without a configuration switch but to not modify the code at all has a few other consequences. 1. xwiki-core is serving a dual purpose, it is the client of all the services provided by the modules and it is the grave yard for dead and deprecated code. I would like the idea much better if we could usher the dead code from xwiki-core to another module so that core of the application is not the dead code cemetery. 2. The point about custom storage implementations could be applied to any old code, any line we change might be breaking someone who depends on it, this is especially true because groovy allows access to private fields and methods without even a warning.

From the community standpoint, people who make patch sets and keep them secret are harming the community by keeping their (potentially very good) modifications to themselves, this is the same discussion which goes on with the Linux kernel and closed source modules, I agree with Linus here that although we should not try to stop this practice we should also not protect it by preserving internal APIs.

The approach I have thus far taken has been to remove unused code, implement APIs externally, and move things over one at a time. This gives us results right away and makes xwiki-core less bloated but it leaves behind sub-optimal APIs which still need to be rewritten again and introduces more bugs since every interface is at some point reimplemented. Your method (as I understand it) is to rewrite things in large chunks but (at least with rendering) never to break the old API. Reimplementing storage this way would mean we need the thing to store (XWikiDocument) and the objects and properties which amounts to a total rewrite of core before the first result comes in. If we are to keep deprecated code such as rendering1.0 (forever?) I think we need to move it out of the core where it confuses attempts to fix core bugs. Caleb On 02/09/2011 09:21 AM, Vincent Massol wrote:

+0 for the idea but I don't know that part of the code enough to know what problems this is going to cause (custom storage implementations out there, etc).

Personally I'd have preferred to not touch a single bit at any existing storage interfaces and instead introduce new storage APIs in the new xwiki-store module and have a flag to direct to the old or new implementations. This is basically the strategy we followed with the new rendering engine and that has allowed us to keep 100% backward compat for the XWiki 1.0 syntax.

Thanks -Vincent

On Feb 9, 2011, at 2:20 PM, Caleb James DeLisle wrote:

...

I would like to propose another round of storage deprecations, the goal of these is to remove and decrease visibility of code in order to simplify storage and move as much as possible over from API to implementation details. I am proposing deprecation of each of the following, after 2 releases this may be revisited and they may be removed or altered. The following are changes I have made locally and found xwiki-core does compile and test with those changes. For now I propose adding deprecation comments and annotations to each class or method.

WDYT?

Caleb

XWikiAttachmentStoreInterface.java saveAttachmentContent(XWikiAttachment attachment, XWikiContext context, boolean bTransaction) remove

cleanUp(XWikiContext context) remove

XWikiBatcher.java remove entirely

XWikiBatcherFactory.java remove entirely

XWikiBatcherStats.java remove entirely

XWikiDefaultStore.java remove entirely

XWikiHibernateBaseStore.java getDatabaseProductName(XWikiContext context) public --> protected

shutdownHibernate(XWikiContext context) remove

updateSchema(XWikiContext context) public --> private

getSchemaFromWikiName(String wikiName, DatabaseProduct databaseProduct, XWikiContext context) protected --> private

getSchemaFromWikiName(XWikiContext context) protected --> private

getSchemaUpdateScript(Configuration config, XWikiContext context) public --> private

updateSchema(String[] createSQL, XWikiContext context) public --> private

updateSchema(BaseClass bclass, XWikiContext context) remove

isVirtual(XWikiContext context) public --> protected

beginTransaction(SessionFactory sfactory, XWikiContext context) public --> protected

beginTransaction(SessionFactory sfactory, boolean withTransaction, XWikiContext context) public --> protected

endTransaction(XWikiContext context, boolean commit, boolean withTransaction) public --> protected

execute(XWikiContext context, boolean bTransaction, boolean doCommit, HibernateCallback<T> cb) public --> private

XWikiHibernateStore.java getContent(XWikiDocument doc, StringBuffer buf) remove

public List search(Query query, int nb, int start, XWikiContext context) remove

injectCustomMappingsInSessionFactory(BaseClass bclass, XWikiContext context) public --> private

injectCustomMappingsInSessionFactory(XWikiContext context) public --> private

XWikiHibernateVersioningStore.java loadAllRCSNodeInfo(XWikiContext context, final long id, boolean bTransaction) protected --> private

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

On Thu, Feb 10, 2011 at 15:53, Caleb James DeLisle <[email protected]> wrote:

You make a good point about not touching anything. I agree totally about not modifying the behavior of storage without a configuration switch but to not modify the code at all has a few other consequences.

1. xwiki-core is serving a dual purpose, it is the client of all the services provided by the modules and it is the grave yard for dead and deprecated code. I would like the idea much better if we could usher the dead code from xwiki-core to another module so that core of the application is not the dead code cemetery.

Note that there is xwiki-legacy project and aspect in xwiki-core to remove deprecated code from project or java files we want to cleanup.

2. The point about custom storage implementations could be applied to any old code, any line we change might be breaking someone who depends on it, this is especially true because groovy allows access to private fields and methods without even a warning. From the community standpoint, people who make patch sets and keep them secret are harming the community by keeping their (potentially very good) modifications to themselves, this is the same discussion which goes on with the Linux kernel and closed source modules, I agree with Linus here that although we should not try to stop this practice we should also not protect it by preserving internal APIs.

The approach I have thus far taken has been to remove unused code, implement APIs externally, and move things over one at a time. This gives us results right away and makes xwiki-core less bloated but it leaves behind sub-optimal APIs which still need to be rewritten again and introduces more bugs since every interface is at some point reimplemented.

Your method (as I understand it) is to rewrite things in large chunks but (at least with rendering) never to break the old API. Reimplementing storage this way would mean we need the thing to store (XWikiDocument) and the objects and properties which amounts to a total rewrite of core before the first result comes in.

If we are to keep deprecated code such as rendering1.0 (forever?) I think we need to move it out of the core where it confuses attempts to fix core bugs.

Caleb

On 02/09/2011 09:21 AM, Vincent Massol wrote:

...

+0 for the idea but I don't know that part of the code enough to know what problems this is going to cause (custom storage implementations out there, etc).

Personally I'd have preferred to not touch a single bit at any existing storage interfaces and instead introduce new storage APIs in the new xwiki-store module and have a flag to direct to the old or new implementations. This is basically the strategy we followed with the new rendering engine and that has allowed us to keep 100% backward compat for the XWiki 1.0 syntax.

...

Thanks -Vincent

On Feb 9, 2011, at 2:20 PM, Caleb James DeLisle wrote:

...

I would like to propose another round of storage deprecations, the goal of these is to remove and decrease visibility of code in order to simplify storage and move as much as possible over from API to implementation details. I am proposing deprecation of each of the following, after 2 releases this may be revisited and they may be removed or altered. The following are changes I have made locally and found xwiki-core does compile and test with those changes. For now I propose adding deprecation comments and annotations to each class or method.

WDYT?

Caleb

XWikiAttachmentStoreInterface.java  saveAttachmentContent(XWikiAttachment attachment, XWikiContext context, boolean bTransaction)  remove

 cleanUp(XWikiContext context)  remove

XWikiBatcher.java  remove entirely

XWikiBatcherFactory.java  remove entirely

XWikiBatcherStats.java  remove entirely

XWikiDefaultStore.java  remove entirely

XWikiHibernateBaseStore.java  getDatabaseProductName(XWikiContext context)  public --> protected

 shutdownHibernate(XWikiContext context)  remove

 updateSchema(XWikiContext context)  public --> private

 getSchemaFromWikiName(String wikiName, DatabaseProduct databaseProduct, XWikiContext context)  protected --> private

 getSchemaFromWikiName(XWikiContext context)  protected --> private

 getSchemaUpdateScript(Configuration config, XWikiContext context)  public --> private

 updateSchema(String[] createSQL, XWikiContext context)  public --> private

 updateSchema(BaseClass bclass, XWikiContext context)  remove

 isVirtual(XWikiContext context)  public --> protected

 beginTransaction(SessionFactory sfactory, XWikiContext context)  public --> protected

 beginTransaction(SessionFactory sfactory, boolean withTransaction, XWikiContext context)  public --> protected

 endTransaction(XWikiContext context, boolean commit, boolean withTransaction)  public --> protected

 execute(XWikiContext context, boolean bTransaction, boolean doCommit, HibernateCallback<T> cb)  public --> private

XWikiHibernateStore.java  getContent(XWikiDocument doc, StringBuffer buf)  remove

 public List search(Query query, int nb, int start, XWikiContext context)  remove

 injectCustomMappingsInSessionFactory(BaseClass bclass, XWikiContext context)  public --> private

 injectCustomMappingsInSessionFactory(XWikiContext context)  public --> private

XWikiHibernateVersioningStore.java  loadAllRCSNodeInfo(XWikiContext context, final long id, boolean bTransaction)  protected --> private

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs

-- Thomas Mortagne