Replace all calls to db_update, which is deprecated

Alternative way to update database method used now.

I am not sure about the scope here. I will look into it.

See the new parent issue. Let's replace this in all of core.

Replaced all the calls to db_update with correct format , except in test cases.

Hi,

Found some code style related issues in patch #7, Kindly correct these.
Also, 1 more instance of "db_update()" found in core/modules/path/path.api.php:42: not covered in #7. Include this as well.

1. +++ b/core/lib/Drupal/Core/Entity/entity.api.php
   @@ -989,13 +989,13 @@ function hook_ENTITY_TYPE_insert(Drupal\Core\Entity\EntityInterface $entity) {
   -  db_update('example_entity')
   -    ->fields(array(
   -      'updated' => REQUEST_TIME,
   -    ))
   -    ->condition('type', $entity->getEntityTypeId())
   -    ->condition('id', $entity->id())
   -    ->execute();
   +	$query = \Drupal::database()->update('example_entity');
   +	$query->fields([
   +		'updated' => REQUEST_TIME,
   +	]);
   +	$query->condition('type', $entity->getEntityTypeId());
   +	$query->condition('id', $entity->id());
   +	$query->execute();
   
   @@ -1013,12 +1013,12 @@ function hook_entity_update(Drupal\Core\Entity\EntityInterface $entity) {
   -  db_update('example_entity')
   -    ->fields(array(
   -      'updated' => REQUEST_TIME,
   ...
   +	$query->execute();
   
   +++ b/core/modules/locale/locale.translation.inc
   @@ -342,11 +342,14 @@ function locale_cron_fill_queue() {
   -    db_update('locale_file')
   -      ->fields(array('last_checked' => REQUEST_TIME))
   -      ->condition('project', $file->project)
   -      ->condition('langcode', $file->langcode)
   -      ->execute();
   +
   +  $query = \Drupal::database()->update('locale_file');
   +  $query->fields([
   +    'last_checked' => REQUEST_TIME,
   +  ]);
   +  $query->condition('project', $file->project);
   +  $query->condition('langcode', $file->langcode);
   +  $query->execute(); ¶
      }
   
   +++ b/core/modules/search/search.module
   @@ -570,11 +570,11 @@ function search_index($type, $sid, $langcode, $text) {
     *   $type and $sid is marked.
   

   Use spaces instead of Tabs.

2. +++ b/core/modules/locale/locale.translation.inc
   @@ -342,11 +342,14 @@ function locale_cron_fill_queue() {
   -    db_update('locale_file')
   -      ->fields(array('last_checked' => REQUEST_TIME))
   -      ->condition('project', $file->project)
   -      ->condition('langcode', $file->langcode)
   -      ->execute();
   +
   +  $query = \Drupal::database()->update('locale_file');
   +  $query->fields([
   +    'last_checked' => REQUEST_TIME,
   +  ]);
   +  $query->condition('project', $file->project);
   +  $query->condition('langcode', $file->langcode);
   +  $query->execute(); ¶
      }
   

   Use proper Indentation.

3. +++ b/core/modules/search/search.module
   @@ -570,11 +570,11 @@ function search_index($type, $sid, $langcode, $text) {
   +   $query = \Drupal::database()->update('search_dataset');
   +    $query->fields(['reindex' => REQUEST_TIME]);
        // Only mark items that were not previously marked for reindex, so that
        // marked items maintain their priority by request time.
   -    ->condition('reindex', 0);
   +    $query->condition('reindex', 0);
   

   Check indentation.

4. +++ b/core/scripts/generate-d7-content.sh
   @@ -290,29 +290,32 @@
   +$query->condition('entity_type', 'node');  ¶
   ...
   +$query->condition('entity_type', 'node');  ¶
   

   unecessary space at end of line.

Thanks !!

I have done all the corrections and replaced all calls to db_update with correct syntax.

#12 Looks good except for minor coding standards. I'd recomend to run your code thru coder module.
minor spaces and indentation issues. could you pls fix that.

Thanks !!

There are still coding standards violations.

1. +++ b/core/modules/locale/locale.translation.inc
   @@ -342,11 +342,14 @@ function locale_cron_fill_queue() {
   +
   

   No need for this empty line.

2. +++ b/core/modules/tracker/tracker.module
   @@ -353,20 +353,24 @@ function _tracker_remove($nid, $uid = NULL, $changed = NULL) {
   -      db_update('tracker_node')
   -        ->fields(array(
   -          'changed' => $changed,
   -          'published' => $node->isPublished(),
   -        ))
   -        ->condition('nid', $nid)
   -        ->execute();
   -      db_update('tracker_node')
   -        ->fields(array(
   -          'changed' => $changed,
   -          'published' => $node->isPublished(),
   -        ))
   -        ->condition('nid', $nid)
   -        ->execute();
   +
   +    $query = \Drupal::database()->update('tracker_node');
   +    $query->fields([
   +      'changed' => $changed,
   +      'published' => $node->isPublished(),
   +    ]);
   +    $query->condition('nid',$nid);
   +    $query->execute();
   +
   +    $query = \Drupal::database()->update('tracker_node');
   +    $query->fields([
   +      'changed' => $changed,
   +      'published' => $node->isPublished(),
   +    ]);
   +    $query->condition('nid',$nid);
   +    $query->execute();
   +
   +
   
   +++ b/core/modules/user/user.api.php
   @@ -55,11 +55,14 @@ function hook_user_cancel($edit, $account, $method) {
   -      db_update('node_field_revision')
   -        ->fields(array('uid' => 0))
   -        ->condition('uid', $account->id())
   -        ->execute();
   -      break;
   +
   +    $query = \Drupal::database()->update('node_field_revision');
   +    $query->fields([
   +      'uid' => 0,
   +    ]);
   +    $query->condition('uid', $account->id());
   +    $query->execute();
   +    break;
   

   Not the same indentation and unnecessary empty lines added.

Looks good to me.

I am moving this to database system to be like this others, although this is about calls into the system.

There is documentation in database.inc telling people to use db_update().

The meta issue reads: "...replace all usages of the function (except tests for the function itself)...", which would mean to replace usages except for tests that are specifically for db_update itself, which in this case I think is Drupal\KernelTests\Core\Database\UpdateTest.

We can have separate issue for documentation. Documentation is not exactly usage , that comment can be edited when definition of db_update will be removed.

+++ b/core/scripts/generate-d7-content.sh
@@ -290,29 +290,32 @@
-db_update('node')

See #2232391: Revert drupal 8 changes made to generate-d7-content.sh and generate-d6-content.sh. This file should not be changed.

OK on #21 - open a single issue to fix all the documentation references to using db_* functions and relate it to the meta issue please.

(edited)

Checked this issue (https://www.drupal.org/node/2232391) and have restored file generate-d7-content.sh

Looks good to me. Back to RTBC.

Did you see comment #20?

@cilefen: Sorry, I missed that one.

@cilefen are you suggesting that calls to db_update in Drupal\KernelTests\Core\Database\UpdateTest should also be replaced with new syntax.??

No. As far as I know (still discussing with others) those are the usages not to change. But things like AggregatorCronTest::testCron call it.

replaced the usage of "db_update" in database.inc file.

I have some remarks about this patch:

1. diff --git a/core/includes/database.inc b/core/includes/database.inc
   index ff03885..e22e081 100644
   --- a/core/includes/database.inc
   +++ b/core/includes/database.inc
   @@ -26,7 +26,8 @@
     * instead.
     *
     * Do not use this function for INSERT, UPDATE, or DELETE queries. Those should
   - * be handled via db_insert(), db_update() and db_delete() respectively.
   + * be handled via db_insert(), \Drupal::database()->update() and db_delete()
   + * respectively.
     *
     * @param string|\Drupal\Core\Database\StatementInterface $query
     *   The prepared statement query to run. Although it will accept both named and
   

   The changes in the documentation will be done in #2849745: Replace documentation recommending db_*() wrappers.

3. +++ b/core/lib/Drupal/Core/Entity/entity.api.php
   @@ -986,13 +986,13 @@ function hook_ENTITY_TYPE_insert(Drupal\Core\Entity\EntityInterface $entity) {
   -  db_update('example_entity')
   -    ->fields(array(
   -      'updated' => REQUEST_TIME,
   -    ))
   -    ->condition('type', $entity->getEntityTypeId())
   -    ->condition('id', $entity->id())
   -    ->execute();
   +  $query = \Drupal::database()->update('example_entity');
   +  $query->fields([
   +    'updated' => REQUEST_TIME,
   +  ]);
   +  $query->condition('type', $entity->getEntityTypeId());
   +  $query->condition('id', $entity->id());
   +  $query->execute();
   

   The only thing that should be changed is the first line: db_update('example_entity') to \Drupal::database()->update('example_entity'). Watch out for the semicolon.

4. The same as the previous point for all other code changes.
5. The function db_update() is still not changed in a lot of places.

@daffie, I have updated the patch.

The function db_update() is still not changed in a lot of places.

db_update is used in various test files. Are they also needs to be replaced in this patch?

@shashikant_chauhan: Yes all usages of db_update must be changed, including all tests! The changes that you made in comment #33 are good to me.

I think on the meta we had reasoned that we should not remove tests for the function itself.

The function db_update() is used in a lot of tests, mostly in testing for other functionality. I had the idea that we would change ALL usages of db_update(). Can you point me to that decision @cilefen?

I thought about it some more and I agree that if we remove all usage in testing then we will have no more testing for the functions in database.inc. I would really like to remove all usage of the functions in testing. My suggesting would be to create a new test for the functions in database.inc. If we do not remove all usage in testing we cannot a the start of Drupal 9.0 just delete the functions in database.inc.

I would not call it an official decision: #2848161-25: [meta] Replace calls to deprecated db_*() wrappers because I don't know the project standard for leaving in tests that are testing a deprecated function directly. In either case, that issue is the place for that discussion.

I have fixed the 'drupal-replace-calls-to-db_update-2848137-33.patch' patch because it was not applying any more because 'array()' notation has already been applied in the files affected by the patch.

After that, I searched in all code to realize the size of the work we still need to do.

cit008490cpsubuntu:drupal progestag* 8.4.x$ find . -type f \( -name '*.inc' -or -name '*.module' -or -name '*.php' \) -exec grep db_update {} \+ | wc -l
86

cit008490cpsubuntu:drupal progestag* 8.4.x$ patch -p1 < /tmp/replace_all_calls_to-2848137-39.patch
patching file core/lib/Drupal/Core/Entity/entity.api.php
patching file core/modules/locale/locale.translation.inc
patching file core/modules/path/path.api.php
patching file core/modules/search/search.module
patching file core/modules/tracker/tracker.module
patching file core/modules/user/user.api.php

cit008490cpsubuntu:drupal progestag* 8.4.x$ find . -type f \( -name '*.inc' -or -name '*.module' -or -name '*.php' \) -exec grep db_update {} \+ | wc -l
77

@hgunicamp i have replaced db_update in remaining files,now after running this command find . -type f \( -name '*.inc' -or -name '*.module' -or -name '*.php' \) -exec grep db_update {} \+ | wc -l
61 please review the patch and advise me

@brahmjeet789 you can change a little bit the command to make it show which files still have the 'db_update'.
For example:
find . -type f \( -name '*.inc' -or -name '*.module' -or -name '*.php' \) -exec grep db_update {} \+ | cut -f1 -d':' | uniq

Explaining each part of the command:

1. find . -type f \( -name '*.inc' -or -name '*.module' -or -name '*.php' \) -exec grep db_update {} \+ will list all occurrences of 'db_update' starting the search from current directory.
2. cut -f1 -d':' the first part will return a kind of table and the first column will be the name of file in which its has found the 'db_update'. This first column is separated of the others using a ':'. Then this part is filtering the first column.
3. uniq will make each result appear only once.

Using this command I got:


find . -type f \( -name '*.inc' -or -name '*.module' -or -name '*.php' \) -exec grep db_update {} \+ | cut -f1 -d':' | uniq
./core/tests/Drupal/KernelTests/Core/Database/UpdateLobTest.php
./core/tests/Drupal/KernelTests/Core/Database/UpdateComplexTest.php
./core/tests/Drupal/KernelTests/Core/Database/UpdateTest.php
./core/tests/Drupal/KernelTests/Core/Config/Storage/DatabaseStorageTest.php
./core/modules/system/tests/fixtures/update/drupal-8.block-content-uninstall.php
./core/modules/system/tests/modules/entity_test/entity_test.module
./core/modules/system/tests/modules/update_script_test/src/Controller/UpdateScriptTestController.php
./core/modules/system/system.module
./core/modules/system/src/Tests/Update/UpdateScriptTest.php
./core/modules/system/src/Tests/Update/DbUpdatesTrait.php
./core/modules/system/src/Tests/Update/UpdateSchemaTest.php
./core/modules/system/src/Tests/Update/UpdatePathTestBase.php
./core/modules/system/src/Controller/DbUpdateController.php
./core/modules/system/src/Theme/DbUpdateNegotiator.php
./core/modules/update/update.authorize.inc
./core/modules/update/update.module
./core/modules/user/tests/src/Kernel/TempStoreDatabaseTest.php
./core/modules/user/src/Tests/UserPasswordResetTest.php
./core/modules/user/src/Tests/UserCancelTest.php
./core/modules/user/src/Tests/UserBlocksTest.php
./core/modules/user/src/Tests/UserPictureTest.php
./core/modules/search/tests/src/Functional/SearchMultilingualEntityTest.php
./core/modules/contact/tests/drupal-7.contact.database.php
./core/modules/node/tests/src/Functional/NodeRevisionsAllTest.php
./core/includes/database.inc
./core/lib/Drupal/Core/Database/Driver/pgsql/Update.php
./core/lib/Drupal/Core/Database/database.api.php
./core/lib/Drupal/Core/Database/Query/Update.php
./core/lib/Drupal/Core/Update/UpdateKernel.php

I hope it may help.

I posted the 'replace_all_calls_to-2848137-45.patch' fixing a typo in 'replace_all_calls_to-2848137-40.patch' which causes the linter failure.

The patch looks good to me. Quick search inside core folder does not find any instance of db_update in code. Also did sanity testing, everything works fine for me.
+1 for RTBC.

Thanks @hgunicamp for guiding me on this issue i will check and acknowledge it.

Updated all the occurences.

I Have tested it on my local its working fine, Now all calls to db_update is changed to \Drupal::database()->update().

Hi I have replaced the db calls, please review the patch.

Hi I have replaced the db calls, please review the patch.

Reroll

Patch doesn't apply.

Postponed on #2991542: Introduce a DatabaseLegacyTest class for deprecation testing and #2991337: Document the recommended ways to obtain the database connection object

Rerolled.
Added the legacy test.
Reverted changes to D7 related script.

Rerolled

Reroll and following changes

- removed dot at the end of URL to deprecated message cos some editors suppose the dot as part of URL
- connection is final object that could live withing each testcase, so I made a cache for local variable

And test must ensure that functions return proper object instead of what the object can do - so testing was changed

Missed table name

Looks good

Committed/pushed to 8.7.x, thanks!