Penny Leach: 12:42:42
What does moodle hq think about me ignoring the 1.9 thing and doing instead ?
Penny Leach: 12:42:47
feels rebellious
Penny Leach: 12:45:04
crap, I need to merge my gsoc student code too
Eloy Lafuente: 12:46:29
Really radical! Absolutely!
Penny Leach: 12:46:53
really ?
Penny Leach: 12:46:56
you reckon ?
Penny Leach: 12:47:08
Eloy - I'd like to discuss it with you a little bit,
Eloy Lafuente: 12:47:09
Penny Leach: 12:47:20
last time I did it it broke backwards compatibility with a few functions because tehy need a new argument
Eloy Lafuente: 12:47:22
yep, me too.
Martín Langhoff: 12:47:43
at moot uk, talking with tim we came up[ with a backwards compat strategy for that...
Martín Langhoff: 12:47:56
with a few other added benefits...
Eloy Lafuente: 12:47:59
really? I'm all ears
Martín Langhoff: 12:48:17
keep current dmllib functions as tehy are for bw compat
Martín Langhoff: 12:49:07
and have a new Moodle-style global DB object with identical methods - subclassing or wrapping around AdoDB
Eloy Lafuente: 12:49:55
well, that's the design view. nothing against that (nor the opposite).
Eloy Lafuente: 12:49:59
Eloy Lafuente: 12:50:14
what about compatibility?
Penny Leach: 12:50:28
martin - I don't understand your proposal
Eloy Lafuente: 12:50:48
the key here is that dmllib themselves aren't compatible nor incompatible.
Martín Langhoff: 12:50:54
this means that:
- insert_record() can coexist wuth $mdb->insert_record()
- during transition, scripts can declare - before they include config.php - that they do not want addslashes/magicquotes
Penny Leach: 12:50:57
do you mean go through and change all current calls to (eg get_record) with $newdb->get_record with the different parameters ?
Eloy Lafuente: 12:50:59
IMO the incompatibility is in:
Eloy Lafuente: 12:51:05
Eloy Lafuente: 12:51:13
and it's spreaded in ALL code
Penny Leach: 12:51:21
Eloy - you have to switch it to detect it ON and turn it OFF, and remove in code
Martín Langhoff: 12:51:24
we still have to go through and study all through the code things like addslashes - as eloy mentions
Penny Leach: 12:51:31
but that doesn't break backwards compat, the problem is the function parameters
Penny Leach: 12:51:39
you need to start passing $values to SOME of them
Penny Leach: 12:51:40
like ..
Penny Leach: 12:51:48
finds patch
Eloy Lafuente: 12:51:56
yep, yep. Penny. Apart from the array of placeholders...
Penny Leach: 12:52:04;a=commitdiff;h=553765dbdba08162745fe10710ac20153f3c12d2
Penny Leach: 12:52:10
^^^ very old patch, but gives you an idea
Penny Leach: 12:52:26
-function count_records_select($table, $select='', $countitem='COUNT(*)') {
+function count_records_select($table, $select='', $values=null, $countitem='COUNT(*)') {
Penny Leach: 12:52:30
Eloy Lafuente: 12:52:31
the first thing to decide is HOW are we going to keep compatibility (about slashes) or no.
Penny Leach: 12:52:57
you have to switch the workaround in lib/setup.php to detect it on and go through and stripslashes everything
Eloy Lafuente: 12:52:58
yep, yep. I think that all we...
Penny Leach: 12:53:02
and grep code for addslashes
Eloy Lafuente: 12:53:12
think the same about placeholders. It's the easy part IMO.
Eloy Lafuente: 12:53:19
it's addslashes. yep.
Martín Langhoff: 12:53:21
... would this not help the transition and support old code as well?
Penny Leach: 12:53:24
Eloy - even if it changes the argument order ?
Martín Langhoff: 12:53:41
it measn that we are free to change function prototypes more freely
Eloy Lafuente: 12:53:49
yep, yep-
Penny Leach: 12:54:00
oh ok I thought that would be the issue :)
Eloy Lafuente: 12:54:06
agree. I love OOP so I haven't nothing against that.
Eloy Lafuente: 12:54:23
What I don't as a correct solution...
Penny Leach: 12:54:28
I don't see how changing the order of arguments in a function definition relates to OOP ?
Martín Langhoff: 12:54:37
and once we have that - we can also use MoodleDB for other external DBs, like in Auth/DB, etc -- (if we want to)
Eloy Lafuente: 12:54:41
is to PLAGUE code with conditional addslashes !
Penny Leach: 12:54:51
Eloy - it must GO totally
Penny Leach: 12:54:54
Penny Leach: 12:54:57
Eloy Lafuente: 12:55:02
That makes me think if wouldn't be better to BREAK backwards completely.
Martín Langhoff: 12:55:04
Penny - having the 2 apis in parallel means we can break more with the old API
Martín Langhoff: 12:55:16
as we'll have to walk the code anyway
Penny Leach: 12:55:28
I think having 2 APIs is a bad idea
Eloy Lafuente: 12:55:34
yep, agree.
Penny Leach: 12:55:41
Eloy - with who? ;)
Eloy Lafuente: 12:55:46
2.0 release shouldn't have both.
Martín Langhoff: 12:55:55
it's just a transition thing - it will be hard, but it will give us bw compat.
Eloy Lafuente: 12:55:56
only one IMO.
Eloy Lafuente: 12:56:07
sorry Martin....
Martín Langhoff: 12:56:22
and it is a good chance to rearrange the method params.
Eloy Lafuente: 12:56:41
what I don't understand.... is how are we going to kill all those addslashes currently in code (or change them to conditionals).
Eloy Lafuente: 12:56:53
IMO that's a horrible hack.
Eloy Lafuente: 12:57:20
or are you talking... only... about contrib modules being able to use the old API ?
Eloy Lafuente: 12:57:30
and ALL the core using the new ONE?
Eloy Lafuente: 12:57:46
or what?
Martín Langhoff: 12:57:52
of course, I'd want to ship v2 only using the new API ;-)
Martín Langhoff: 12:58:12
but have the old calls available - perhaps force an option to enable it
Eloy Lafuente: 12:58:22
Martín Langhoff: 12:58:33
it also means that while we develop the code for teh transition, we have something working all the time
Eloy Lafuente: 12:58:44
2.0 is about to break things IMO. And that's a good a justified break IMO.
Penny Leach: 12:58:51
I agree
Martín Langhoff: 12:58:53
avoid having to prepare one huge patch against all of moodle
Penny Leach: 12:58:54
2.0 is the time to do it
Penny Leach: 12:59:02
Martin -
Penny Leach: 12:59:14
I think it's worth doing properly if we're going to do it
Penny Leach: 12:59:21
keeping 2 APIs encourages laziness
Penny Leach: 12:59:48
and it introduces the possibility of security nightmare
Eloy Lafuente: 12:59:54
one 1-month branch should be able to do all the BIG job IMO.
Eloy Lafuente: 13:00:02
then merge and polish.
Penny Leach: 13:00:06
if you're not either always knowing you have addslashes or knowing you have placeholders... there's SO many entry points for security bugs
Eloy Lafuente: 13:00:42
after all... the BIG job includes:
Martín Langhoff: 13:00:49
PEnny - no, security problems with the 2 APIs - the legacy API simply does a stripslashes() and passes on to the new API
Penny Leach: 13:01:05
Penny Leach: 13:01:16
if each script has to say whether it wants its variables addslashed or not, that's not true
Martín Langhoff: 13:01:25
the challenge is to do this without breaking old modules completely
Penny Leach: 13:01:33
Eloy Lafuente: 13:01:33
1) Create the new API and test it 300%.
2) Repetitive addslahes() killing.
3) Repetitiveencapsulate placeholder calls.
Eloy Lafuente: 13:02:02
2 & 3 can be performed in branch relatively quickly.
Eloy Lafuente: 13:02:09
1 can be done in paralell.
Eloy Lafuente: 13:02:16
Penny Leach: 13:02:19
Eloy - y ep
Eloy Lafuente: 13:02:54
or... in other words... what's the advantage of keeping both APIs?
Eloy Lafuente: 13:03:06
I cannot see that.
Martín Langhoff: 13:03:23
the advantage is called contrib, private modules, etc
Penny Leach: 13:03:39
you have to break backwards compatibility sometimes
Penny Leach: 13:03:46
if the benefit is worth it
Penny Leach: 13:03:50
moodle 1.6 broke themes remember
Penny Leach: 13:03:52
so much better now
Eloy Lafuente: 13:03:59
and UTF8
Penny Leach: 13:04:04
oh yeah totally
Eloy Lafuente: 13:04:10
and install/upgrade
Penny Leach: 13:04:18
well there was a transition for that
Penny Leach: 13:04:25
postgres7.* and mysql.* stuck around for a release
Eloy Lafuente: 13:04:44
yep. The key here, IMO is about to perform 1) (publish the new API) ASAP.
Penny Leach: 13:04:46
moving to use referential integrity will almost CERTAINLY break backwards compatibility
Martín Langhoff: 13:04:46
install/upgrade got 2 releases
Penny Leach: 13:05:01
eg code deleting things from 2 tables in incorrect order
Eloy Lafuente: 13:05:02
That will provide people with ... 9 months... to migrate?
Penny Leach: 13:05:07
won't work with FKs
Eloy Lafuente: 13:05:13
IMO far enough for maintained modules.
Penny Leach: 13:05:16
Eloy - I agree, which is why I propose doing it now, early, get it in
Eloy Lafuente: 13:06:11
that's only my opinion, of course. But I cannot see really both APIs working together in a correct way.
Penny Leach: 13:06:27
lets wait for MD to come online for his input too