Move include/dba.php to src/

[MrPetovan]

Follow-up to #3878

This very impatcful process will be done in four different PRs:

1. Move dba class out of include/dba.php to \Friendica\Database\dba (Move \dba to \Friendica\Database\dba #5418)
2. Rename \Friendica\Database\dba to \Friendica\Database\DBA (Rename Friendica\Database\dba to Friendica\Database\DBA #5422)
3. Rename badly cased \Friendica\Database\DBA methods (Move DBA to src/ part 3.1: Rename DBA methods #5431 and Move DBA to src/ part 3.2: Remove DBM class #5438)
4. Convert the remaining q() function calls to \Friendica\Database\DBA methods calls.

Per @annando

Concerning the fourth step: It is important to not simply replace them with "dba::p" or "dba::e". We should have a look at the logic and replace them with "::select" whenever possible. And especially when the item table is involved, we have to change the behaviour in a way that we can use the "item::select" functions.

I want to split the user related fields in the item table from the rest. But this will only be possible after having replaced nearly all item related "q" functions. Because of this, I guess I have to postpone some of my intended changes to the item table.

[nupplaphil]

If I have a wish, I'd like that these changes will include particular unittests too. I think this would help us (especially for the fourth step from @annando !)

[MrPetovan]

I'm not even sure what they would look like, dba methods are a mess of conditionals.

What kind of unit tests are you thinking about?

[nupplaphil]

Hm.. An idea:

• select with one argument
• select with combined argument
• select with "?"
• select with hard coded value
• ...

And assertions what it should return in this caeses.

It hasn't to be a "real" logic. Just to make sure that the specific methods work as desired.

@Rudloff what do you say? Helpful? Other ideas? Or useless?

[MrPetovan]

How would it help the transition from old-style p() and q() to new-style dba::select and dba::update?

[nupplaphil]

To separate logic errors when using the new style from errors because of the new style itself.

It's more for safety and normally nothing "should" happen. But it's easier to find bugs IF something happen :-)

[Rudloff]

Of course, I think it's always good to add tests to new code. But in this case, I don't think it is a high priority, because a bug in dba would probably cause failures in other testsuites.

(For example, ApiTest executes ~50% of the code in dba.php.)

[nupplaphil]

Hm.. OK.. I thought maybe because of using the DBA class with maybe refactored features, you probably get into a new execution path in the dba. That's why I thought it's good to add unittests before.

[annando]

The dba class needn't to be really hard refactored. it is already a class and most functions are named correctly. So normally this shouldn't be a problem.

[Quix0r]

So is someone working on this? Or should I take over? Line 319 is AS, should be as! And I spot some missing type-hints.

[MrPetovan]

I'm doing the first step tonight.

[annando]

Concerning the replacing of the dba functions here is my suggestion:

• Every call of q(...) must be replaced.
• Every database call (even existing ones) should be replaced (if possible) with dba functions in the following priority:

1. dba::exists
2. dba::selectFirst
3. dba::select
4. dba::p

The function dba::fetch_first is some relict of the very first days that I don't really like.

The strange constructs with using some intval stuff with LIMIT isn't needed anymore and should be replaced with ?.

We should also use centralized functions when there are repeating queries - like the one that fetches the owner to a current user id.

The calls to the item table are very special. Here we should only use the calls from the Item class. (Of course with exception from the database calls inside the Item class)

If there are complicated joins or other strange constructs, we should have a look at them and replace them with easier calls. Mostly this doesn't make the things slower - but easier to read.

[nupplaphil]

There ist still require_once "include/dba.php"; in a lot of files. Next step would be to remove them, wouldn't it?

[MrPetovan]

Absolutely. include/dba.php held not only the dba class, but also several global functions (like p() and q()) that have to go in Part 4 of this Issue. Until then the require will stay.

[annando]

The function dba_timer in include/dba.php can be removed now - it isn't in use at all. So only dbesc and q are still there. Whenever none of these two functions is used in a file, we can remove the include even now.

[MrPetovan]

I'd rather remove the require_once calls when I remove the actual file. I'm going to review each use of dbesc() and q() anyway.