Add executeQuery and executeUpdate #23665

rullzer · 2020-10-24T09:53:55Z

Names shamelessly copied from Doctrine itself.
Internally it is still using the same flow. But I added some checks
around it.

This should make static analysis a bit more happy. Which in turn makes
me more happy.

Signed-off-by: Roeland Jago Douma [email protected]

nickvergessen

Deprecate execute?

rullzer · 2020-10-25T09:27:25Z

Deprecate execute?

Probably.

I was even thinking maybe we should not leak the doctrine types in the new return type. But just return our own wrapper. As to decouple the whole thing.

MorrisJobke · 2020-10-25T21:34:31Z

I don't get this weird CI failure ... Any idea why this is happening?

kesselb · 2020-10-26T08:35:45Z

OCA\DAV\Tests\unit\Connector\Sabre\RequestTest\EncryptionMasterKeyUploadTest::testUploadOverWriteReadLocked
TypeError: Argument 2 passed to OCA\DAV\Events\CalendarCreatedEvent::__construct() must be of the type array, null given, called in /drone/src/apps/dav/lib/CalDAV/CalDavBackend.php on line 790

server/apps/dav/lib/CalDAV/CalDavBackend.php

Lines 789 to 790 in 9956d13

    
           $calendarData = $this->getCalendarById($calendarId); 
        
           $this->dispatcher->dispatchTyped(new CalendarCreatedEvent((int)$calendarId, $calendarData));

getCalendarById returns null or array. CalendarCreatedEvent expects an array ;) But yeah maybe a check if the execute operation worked is missing here.

MorrisJobke · 2020-10-26T13:21:03Z

getCalendarById returns null or array. CalendarCreatedEvent expects an array ;) But yeah maybe a check if the execute operation worked is missing here.

But why isn't it on all phpunit runs but only on one of them?

ChristophWurst

Fine but we really should have our own abstraction of the statement class from dbal :)

rullzer · 2020-10-27T20:16:32Z

Ok added the statement wrapper (to just have that right away else things might do boom)
And deprectated the main execute method.

The statement wrapper is very tiny right now. But will most likely already cover the 90%. We can extend later when needed.

ChristophWurst · 2020-10-27T20:29:18Z

lib/public/DB/QueryBuilder/IStatement.php

+	/**
+	 * @sinc 21.0.0
+	 */
+	public function fetch(): array;


array … of? 😉

Yeah work your magic if you want. Array of whatever.

@return mixed[] values of a row @psalm-return array<string, mixed>

should do

@psalm-return array<string, string>

All db values returned are strings and need custom casting afterwards as far as I know

kesselb · 2020-10-27T20:31:26Z

lib/public/DB/QueryBuilder/IStatement.php

+namespace OCP\DB\QueryBuilder;
+
+/**
+ * @sinc 21.0.0


Suggested change

* @sinc 21.0.0

* @since 21.0.0

lib/public/DB/QueryBuilder/IQueryBuilder.php

faily-bot · 2020-10-27T23:27:01Z

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 34647: failure

checkers

I'm a little sad 🤖 and was not able to find the logs for this failed job - please improve me at https://github.com/MorrisJobke/drone-logs to provide this to you

mysql8.0-php7.4

Show full log

There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) Test\Files\Cache\UpdaterLegacyTest::testWriteWithMountPoints
Failed asserting that two strings are not identical.

/drone/src/tests/lib/Files/Cache/UpdaterLegacyTest.php:139

integration-sharing-v1-video-verification

build/integration/sharing_features/sharing-v1-video-verification.feature:8

Show full log

  Scenario: Creating a link share with send password by Talk # /drone/src/build/integration/sharing_features/sharing-v1-video-verification.feature:8
[Tue Oct 27 22:49:58 2020] TypeError: Argument 4 passed to OC\Comments\Manager::__construct() must implement interface OCP\AppFramework\Utility\ITimeFactory, instance of OC\InitialStateService given, called in /drone/src/apps/spreed/lib/Chat/CommentsManager.php on line 48 at /drone/src/lib/private/Comments/Manager.php#77
[Tue Oct 27 22:49:58 2020] 127.0.0.1:56380 [200]: /ocs/v2.php/cloud/users/user0
[Tue Oct 27 22:49:58 2020] TypeError: Argument 4 passed to OC\Comments\Manager::__construct() must implement interface OCP\AppFramework\Utility\ITimeFactory, instance of OC\InitialStateService given, called in /drone/src/apps/spreed/lib/Chat/CommentsManager.php on line 48 at /drone/src/lib/private/Comments/Manager.php#77
[Tue Oct 27 22:49:58 2020] 127.0.0.1:56386 [200]: /ocs/v2.php/cloud/users/user0
    Given user "user0" exists                                # SharingContext::assureUserExists()
    And As an "user0"                                        # SharingContext::asAn()
[Tue Oct 27 22:49:58 2020] Login failed: 'user0' (Remote IP: '127.0.0.1')
[Tue Oct 27 22:49:58 2020] 127.0.0.1:56392 [401]: /ocs/v1.php/apps/files_sharing/api/v1/shares
    When creating a share with                               # SharingContext::creatingShare()
      | path               | welcome.txt |
      | shareType          | 3           |
      | password           | secret      |
      | sendPasswordByTalk | true        |
    Then the OCS status code should be "100"                 # SharingContext::theOCSStatusCodeShouldBe()
      Failed asserting that SimpleXMLElement Object &0000000000bc2a79000000003bb07c09 (
          0 => '997'
      ) matches expected '100'.
    And the HTTP status code should be "200"                 # SharingContext::theHTTPStatusCodeShouldBe()
    And last share with password "secret" can be downloaded  # SharingContext::lastShareWithPasswordCanBeDownloaded()

Names shamelessly copied from Doctrine itself. Internally it is still using the same flow. But I added some checks around it. This should make static analysis a bit more happy. Which in turn makes me more happy. Signed-off-by: Roeland Jago Douma <[email protected]>

nickvergessen

didnt test

rullzer added enhancement 3. to review Waiting for reviews labels Oct 24, 2020

rullzer added this to the Nextcloud 21 milestone Oct 24, 2020

rullzer requested review from ChristophWurst, MorrisJobke, juliusknorr, kesselb and nickvergessen October 24, 2020 09:53

nickvergessen approved these changes Oct 24, 2020

View reviewed changes

kesselb approved these changes Oct 24, 2020

View reviewed changes

ChristophWurst approved these changes Oct 27, 2020

View reviewed changes

rullzer requested review from ChristophWurst, kesselb and nickvergessen October 27, 2020 20:16

ChristophWurst reviewed Oct 27, 2020

View reviewed changes

kesselb reviewed Oct 27, 2020

View reviewed changes

nickvergessen reviewed Oct 27, 2020

View reviewed changes

lib/public/DB/QueryBuilder/IQueryBuilder.php Show resolved Hide resolved

This was referenced Dec 14, 2020

21 beta2 #24685

Merged

21 beta3 #24765

Merged

rullzer modified the milestones: Nextcloud 21, Nextcloud 22 Dec 21, 2020

rullzer force-pushed the enh/querybuilder/execute_query_update branch 3 times, most recently from 660326b to 76a6328 Compare March 4, 2021 13:03

nickvergessen approved these changes Mar 4, 2021

View reviewed changes

rullzer merged commit 7b73588 into master Mar 4, 2021

rullzer deleted the enh/querybuilder/execute_query_update branch March 4, 2021 14:46

ChristophWurst mentioned this pull request May 5, 2021

Rename IQueryBuilder::executeUpdate to IQueryBuilder::executeStatement #26877

Merged

rettenbs mentioned this pull request Jun 20, 2021

Exception when changing access level of a Nextcloud user julien-nc/cospend-nc#57

Closed

Uh oh!

Add executeQuery and executeUpdate #23665

Add executeQuery and executeUpdate #23665

Uh oh!

Conversation

rullzer commented Oct 24, 2020

Uh oh!

nickvergessen left a comment

Choose a reason for hiding this comment

Uh oh!

rullzer commented Oct 25, 2020

Uh oh!

MorrisJobke commented Oct 25, 2020

Uh oh!

kesselb commented Oct 26, 2020

Uh oh!

MorrisJobke commented Oct 26, 2020

Uh oh!

ChristophWurst left a comment

Choose a reason for hiding this comment

Uh oh!

rullzer commented Oct 27, 2020

Uh oh!

ChristophWurst Oct 27, 2020

Choose a reason for hiding this comment

Uh oh!

rullzer Oct 27, 2020

Choose a reason for hiding this comment

Uh oh!

ChristophWurst Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickvergessen Oct 28, 2020

Choose a reason for hiding this comment

Uh oh!

kesselb Oct 27, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

faily-bot bot commented Oct 27, 2020

Status of 34647: failure

checkers

mysql8.0-php7.4

integration-sharing-v1-video-verification

Uh oh!

nickvergessen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ChristophWurst Oct 28, 2020 •

edited

Loading