First question to be asked: why are we adopting an ORM? If we are using it to abstract the DB engine selection (i.e. making Koha usable with Postgres instead of only MySQL) I'm sure we are wasting our time. What's the point of rewriting the whole thing to just move away from an enterprise grade DB engine to another? Is it worth the trouble? I'm glad that using an ORM also serves that purpose, but... I think our project is complex enough to benefit from strictly sticking to design patterns, such as the repository pattern; in which the ORM takes care of persistence and nothing more, and we build our business logic on top of that. On that subject, I fully agree with Robin, that we shouldn't rely on hooking the ORM to solve any business logic issue: we need to have our own abstraction layer with a proper API we maintain and unit test (mocked tests, and let integration tests deal with the ORM interaction). Hooking the ORM mixes layers, businesses and problems. That's why it is so hard to write some tests for Koha. Also, our ERD needs some tweaking, to aid maintenance, code simplicity and to support an OO design [1]. For instance, a biblio should definitely have its table, and if biblioitems breaks the design, then we should just merge those tables. I wouldn't implement any hack on the code to preserve that discussed structure. I'd rather spend the time fixing it. So at that level, we should implement the repository pattern, with the following goals: - Consistent API, independent of the DB model (people use our API, no need to know how we built our DB structure). - Separation of concerns between layers (easier to code, easier to test, etc) - Let the ORM take care of DB stuff and use the spare time to talk about cats and food. [2] I'd also like to have a RESTful layer for that API, that should be straightforward to maintain: endpoint routing, authorization layer, call to the right method, output. It is outside the scope of this thread, but a better separation of concerns might make it easier to achieve in a short term. We could the gradually move away from our .pl mess, that contains lots of business logic that doesn't get tested. Best regards Tomás [1] We've just approved a new coding guideline addition in that sense. [2] I forgot to mention beer. On Wed, Sep 10, 2014 at 10:19 PM, Robin Sheat <robin@catalyst.net.nz> wrote:
Kyle Hall schreef op wo 10-09-2014 om 20:51 [-0400]:
It's funny, I've been trying to think of an example search to justify this and I can't really think of one where we can't use existing Result relationships to get our data. Maybe find() and single() are really all we need from pl files, but I don't think a hard and fast rule against search() is a good idea.
We'd still be coupling to the data storage schema, which is bad. It's exactly the same logic that says not to have SQL in the .pl files.
Result and ResultSet are part of the Koha namespace. I also feel that
Only in the sense that they have Koha:: in the name. They are an API to the database, and are generated directly from the database schema. They are not an API that is designed to provide logical access to whatever it is.
you aren't quite correct about how one modules need to know the change a present the same interface. I can't count the number of times I've added a new parameter to an existing subroutine and had to update every function call in many perl files. Now that hashref's are more common, it's become less of a problem but it's still a problem that exists.
"It's been broken in the past" isn't a justification for "so we should keep it broken." As we go to a more OO and thought out method of writing things (again, especially with the Koha:: stuff), we are moving away from "here's a hashref with the fields from the biblio table" to "here's an object that represents a biblio, with accessors and modifiers." We want more of this, and less things going and talking to the database themselves.
I just don't think this is a entirely valid point. Remember, Result and ResultSet methods are no different than methods anywhere else in Koha and C4.
They are totally different. They are much closer to an abstraction of the database, compared to an abstraction of whatever it is the data is representing. And the code at the .pl level should be working with abstractions of data, e.g. "a reserve", or "a biblio with attached items", not "a set of hashrefs containing a biblio, a biblioitem, and the associated item records." This latter is what Result/ResultSet represent.
In the ES code, I've been keeping this in mind. For example, I can go (from memory):
my $it = Koha::Biblio->get_iterator; while (my $bib = $it->next) { add_to_index($bib); }
which means I don't have to care how they're stored. It also meant that it took very little work to refactor it to handle authorities, as everything had the same style of API. ResultSet stuff happened in the modules, but none of it was exposed to the script that was using those modules.
Exactly. If you look at my accounts rewrite you'll see I didn't try to shoehorn everything into Result and ResultSet. I used a Koha module that uses Results and ResultSets in a sane fashion.
And that's fine. Just not at the .pl level. The job of modules is to put a sensible abstraction between the storage layer and the user interface layer. As soon as UI related code can touch the storage layer, you're violating that principle and causing maintenance headaches for the next person who comes along.
-- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5FA7 4B49 1E4D CAA4 4C38 8505 77F5 B724 F871 3BDF
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Tomás Cohen Arazi Prosecretaría de Informática Universidad Nacional de Córdoba ✆ +54 351 5353750 ext 13168 GPG: B76C 6E7C 2D80 551A C765 E225 0A27 2EA1 B2F3 C15F