Abstract the Datasource classes

[creativedutchmen]

Right now datasources use a very strange include to do all the hard work.

By itself this is no problem, but because the code included is not split into functions each doing its own thing, it is very hard to build upon. For example, while building the Newsletter plugin I wanted a datasource that grabs filtered, paginated entries from a section, and return it in array format (so, basically it is a standard datasource with a different output). With the current approach, this is not possible without rewriting or copying a lot of the existing code.

In my eyes, there are two ways we can solve this. The first approach (this gets my vote) is to create a class for each datasource type (datasource.author.php, datasource.dynamic_xml.php, etc). If these classes split up the logic in the grab function into seperate functions (getWhere, getJoins, getCount, parseAsXml, etc) it would be a lot easier to build on the existing code.

The second method (which might be easier to code, but is still messy) is to keep the same include logic in the datasources itself, but to rewrite the include files to have seperate functions for each part of the logic.

This feels especially useful, since we are talking about supporting JSON and other formats.

[brendo]

Heh, you must of read my mind. I've actually started doing this on a branch on my repo.

I've started pulling out the common code into functions (second method), with the goal of then abstracting them to classes.

That would be the best result in the end, as it means extensions can actually extend the core Datasources instead of having to duplicate the same logic (which I currently do in the Union Datasource)

[creativedutchmen]

Do you think this should be on the 2.3 roadmap?

[brendo]

I think we can discuss in the chat, I definitely think it should be on at least some roadmap

[brendo]

Main considerations for 2.3 is that this has to maintain backwards compatibility.

[simoneeconomo]

Any news about it? I must have missed the chat!

[brendo]

No news at the moment, getting the datasource.*.php as separate classes is definitely still a goal.

I am feeling the best approach to this is Datasources as Extensions.

I think some work in #777 will make 'extending' datasources easier, but it's still not as clean as a Datasource extensions.

[nils-werner]

I have started implementing this in a separate branch oop-components in my repository.

I kept backwards-compatibility by keeping the oldfashioned files that are being included separate from the new classes (file listing). Those oldfashioned files will have to be marked as deprecated and for example any forthcoming implementation may only go into the classbased ones.

Current regular DSes are also still compatible with the backend pages we have now. Only once you've changed a DS using the backend editor it will be overwritten with the new template that's using the new object oriented method.

Next step: chopping DS-/Event-classes into bits and pieces that can be overridden individually.

[brendo]

Take a look at the providers branch too. That branch makes datasources extensions so I'm guessing we can combine our efforts :)

[brendo]

Ok, I pushed a new branch, providers to test out the Providers spec.

For a quick recap, this allows extensions to add new Datasource types to Symphony and they can be used within the native editor. This means that extensions like Cacheable Datasource and Union Datasource don't have to create their own UI's (unless they want to).

I have implemented this using the Remote Datasource, which is the new Dynamic XML datasource for 2.3.

I have not implemented any migration scripts for existing Dynamic XML datasources.

I have not merged with integration so that we can review and test these changes without halting other development.

@nils-werner, does this now give you a better base to continue your OOP components?

@designermonkey You emailed me asking about this, the interface is documented, if it's unclear, let me know and we'll improve it.

@nickdunn Would appreciate your eye over this to see if there is anything that would stop you from implementing Cacheable using this idea.

Anybody else, more than welcome to have a poke around!

@nilshoerrmann, we'll address the 'Dynamic' concept of this in #757 as that will require integration with the Pickable component.

[nickdunn]

@nickdunn Would appreciate your eye over this to see if there is anything that would stop you from implementing Cacheable using this idea.

I have a branch of Cacheable DS that uses delegates only, no more messing with the individual DS files themselves. I was waiting for 2.3 to release it, since it relies on a new delegate currently in the integration branch. I don't really have time to check compatibility, but provided the delegates are roughly the same and a DS still has a grab method that returns an XMLElement it should be dandy.

[designermonkey]

I'm really excited about this, as it allows me to continue my unhealthy fascination of geographical hierarchies, and create a Datasource API for a great geographical API.

Teh awesomes.

I will get on this ASAP @brendo and post any suggestions from a 'datasource developer's' perspective.

[brendo]

Please do. The documentation will need further fleshing out I feel.

Depending on the #757 work, the buildEditor function may become less confusing :)

[brendo]

The majority of this has been completed by @nils-werner. AFAIK the work left is mainly refinements to make these classes more flexible and it won't have any functionality changes.

Non blocking for Beta release.

[brendo]

I've cleaned up all the old include to use create new Datasource objects and return the result. There is still work to be done as the actual class files are not done well (the old include code has pretty much just been copied in and wrapped into a class).

As I said before, these changes are non blocking as the functionality of these datasources won't change, they'll just become cleaner to use in a programmatic fashion (which will make Provider datasources easier to create)

Dropping down to the 2.3.x milestone as the initial work has been done by @creativedutchmen and cleaned up a little by me :)

[jonmifsud]

Started messing around with 2.3RC1 - and noticed that the iDatasource implementation does not allow variables to be added to the param pool in the grab. Is this on purpose?

basically the grab function is defined as grab($param_pool) instead of grab(&$param_pool)

[brendo]

No, wasn't on purpose, have fixed now, thanks for noticing :)

[jonmifsud]

cheers :) - had to notice couldn't pass anything back to the param pool.

The code is so clean its really easy to understand - So far no other issues encountered

[nickdunn]

Is there anything outstanding on this?

[brendo]

Not officially, but I would like to leave it open to catch any further improvements from the 2.3 release.

While updating Union Datasource I could see a couple of things that would be nice to tweak for better extensibility.

[brendo]

Closing, these Datasource classes can be abstracted further as required/demanded/needed. As Providers gains more traction and users are working more with these classes I imagine we can refine as required.