adjust to new dbal

[icewind1991]

adjust database code to new dbal shipped with nc21.

Fixes

Exception: Undefined class constant 'STRING' at lib/private/AppFramework/Http/Dispatcher.php line 159

error when trying to use with nc21

[seyfeb]

Hi @icewind1991, thanks for taking the iniative. Some things that I noticed immediately from looking at your PR (I did not check this out yet)

• I don’t think you should commit your composer.lock file (@christianlupus should we add this and maybe also build/ to the .gitignore?)
• You need to sign off your commit (see DCO)
• You need to update the CHANGELOG.md file in the root to reflect your changes with a reference to this PR
• Most of the CI tests are failing. Are your changes compatible with older NC versions (<21)?

Did you check https://docs.nextcloud.com/server/latest/developer_manual/app_publishing_maintenance/upgrade-guide.html#upgrading-to-nextcloud-21 and references therein for what needs to be done for NC 21 compatibility?

[codecov]

Codecov Report

Merging #605 (704b265) into master (c3a4529) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##             master    #605      +/-   ##
===========================================
- Coverage      1.01%   0.99%   -0.02%     
- Complexity      444     450       +6     
===========================================
  Files            14      15       +1     
  Lines          1385    1401      +16     
===========================================
  Hits             14      14              
- Misses         1371    1387      +16     

Flag	Coverage Δ	Complexity Δ
integration	0.00% <0.00%> (ø)	0.00 <7.00> (ø)
unittests	0.99% <0.00%> (-0.02%)	0.00 <7.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
lib/Db/DbTypesPolyfillHelper.php	0.00% <0.00%> (ø)	6.00 <6.00> (?)
lib/Db/RecipeDb.php	0.00% <0.00%> (ø)	57.00 <1.00> (ø)
lib/Migration/Version000000Date20190910100911.php	0.00% <0.00%> (ø)	3.00 <0.00> (ø)

[icewind1991]

* I don’t think you should commit your `composer.lock` file (@christianlupus should we add this and maybe also `build/` to the `.gitignore`?)

Removed, If you don't want them committed they should be added to the ignore yes

* You need to sign off your commit (see DCO)

Done

* You need to update the `CHANGELOG.md` file in the root to reflect your changes with a reference to this PR

Done

* Most of the CI tests are failing. Are your changes compatible with older NC versions (<21)?

The new constants aren't compatible with <21 yes, switched to just using the string literars instead

Did you check https://docs.nextcloud.com/server/latest/developer_manual/app_publishing_maintenance/upgrade-guide.html#upgrading-to-nextcloud-21 and references therein for what needs to be done for NC 21 compatibility?

It's listed under the "Updated core libraries"

[seyfeb]

I’m not sure about hardcoding the types. What about definining them version-dependent?

Maybe using the NC version in the constructor to assign them to a local variable (either \Doctrine\DBAL\Types\Type::* or \OCP\DB\Types::*)?

If I understand correctly, the migration guide suggests dropping them completely (temporarily) for multi-NC-version compatibility, but I must admit I don’t like this.

[christianlupus]

I do not like to remove the type hints either. They are there for a reason. I asked this question on the forum some time ago but only got an unofficial answer.
I suggest building a helper/polyfill class that returns the constants from the different classes depending on the NC version in use. Once NC 20 is EOL we can drop that class again. Does this sound reasonable in general, @icewind1991 @seyfeb?

One more thing I saw during a quick glance at the changes: You committed a whole bunch of whitespace changes (dropped indentation of otherwise empty lines). I know that some editors are doing this automatically. It makes the diff on the other side hard to read/check and cumbersome. This typically comes from a git add -a or committing via the graphical editor. It might be beneficial to keep the changes minimal.

[christianlupus]

Closing this PR as it is already merged into branch compatibility/nc21.