[PP-1] Deprecate file_get_file_references(). Move the logic to file.usage service

Re-rolling against 9.1.x-dev.

remove unused use statement from /core/modules/file/tests/src/Kernel/FileUsageDeprecationTest.php

Drupal 9 was released, so we need to update the deprecation messages.

@Hardik_Patel_12 ok i will check

wrong patch added in #18 please ignore

last two interdiffs, for completeness

+++ b/core/includes/bootstrap.inc
@@ -13,7 +13,7 @@
- * Minimum allowed version of PHP for Drupal to be bootstrapped.
+ * Minimum allowed version of PHP.

@@ -22,13 +22,12 @@
- * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use
- *   \Drupal::MINIMUM_PHP instead.
- *
- * @see https://www.drupal.org/node/2909361
  * @see install.php
+ *
+ * @todo Move this to an appropriate autoloadable class. See
+ *   https://www.drupal.org/project/drupal/issues/2908079
  */
-const DRUPAL_MINIMUM_PHP = \Drupal::MINIMUM_PHP;
+const DRUPAL_MINIMUM_PHP = '7.3.0';

@@ -39,12 +38,10 @@
- * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use
- *   \Drupal::MINIMUM_SUPPORTED_PHP instead.
- *
- * @see https://www.drupal.org/node/2909361
+ * @todo Move this to an appropriate autoloadable class. See
+ *   https://www.drupal.org/project/drupal/issues/2908079
  */
-const DRUPAL_MINIMUM_SUPPORTED_PHP = \Drupal::MINIMUM_SUPPORTED_PHP;
+const DRUPAL_MINIMUM_SUPPORTED_PHP = '7.3.0';

@@ -53,12 +50,10 @@
- * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use
- *   \Drupal::RECOMMENDED_PHP instead.
- *
- * @see https://www.drupal.org/node/2909361
+ * @todo Move this to an appropriate autoloadable class. See
+ *   https://www.drupal.org/project/drupal/issues/2908079
  */
-const DRUPAL_RECOMMENDED_PHP = \Drupal::RECOMMENDED_PHP;
+const DRUPAL_RECOMMENDED_PHP = '7.3';

@@ -67,12 +62,10 @@
- * @deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use
- *   \Drupal::MINIMUM_PHP_MEMORY_LIMIT instead.
- *
- * @see https://www.drupal.org/node/2909361
+ * @todo Move this to an appropriate autoloadable class. See
+ *   https://www.drupal.org/project/drupal/issues/2908079
  */
-const DRUPAL_MINIMUM_PHP_MEMORY_LIMIT = \Drupal::MINIMUM_PHP_MEMORY_LIMIT;
+const DRUPAL_MINIMUM_PHP_MEMORY_LIMIT = '64M';

Are these intentional changes? They do not look in any way related to the patch. They were added in #21.

Edit: They seem to be partially reverting #2908079: Move some of the bootstrap.inc PHP-related constants to \Drupal and deprecate the old versions, indicating a botched re-roll. I'll fix it.

Fixing the bad reroll.

Also updating the deprecation messages from 8.8.0/9.0.0 to 9.1.0/10.0.0.

@cburschka agreed on changes of updating the deprecation messages from 8.8.0/9.0.0 to 9.1.0/10.0.0

Just for consistency of is deprecated in drupal:9.1.0 and is removed in drupal:10.0.0 to is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0 did and fixed test cases fail.

Thanks! Please review

fix the test cases failure in #37

+++ b/core/modules/file/tests/src/Kernel/FileUsageDeprecationTest.php
@@ -0,0 +1,113 @@
+   * @expectedDeprecation file_get_file_references() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use \Drupal\file\FileUsage\FileUsageInterface::getReferences() instead. See https://www.drupal.org/node/3035357.

Now in 9.3.x we need to remove all the instance of @expectedDeprecation.

Instead use

$this->expectDeprecation(DEPRECATION_MESSAGE_HERE)

You can just copy the string from after @expectedDeprecation

It looks like beside that the test should pass 🎉

On it.

I changed to MR approach to make the review easier.
I addressed #46 and #47.

Setting to needs work because of PHPCS fail in last commit https://www.drupal.org/pift-ci-job/2105914

@paulocs thanks starting a new MR from the existing patch!

Needs work for my merge request comments.

This looks almost done to me.

1. I compared \Drupal\file\FileUsage\FileUsageBase::getReferences() and the existing version of file_get_file_references() and confirmed they were the same expect for changes needed to move it to a class.
2. I confirmed the doc for \Drupal\file\FileUsage\FileUsageInterface::getReferences() matches the doc of file_get_file_references()
3. The only thing left is editor_file_download() still has references to file_get_file_references() in its doc

Thanks, @tedbow! Fixed that old reference.

#58.3 was fixed. Will RTBC when tests are green

Hiding previous patches in favor of the merge request.

Looks good!

I merged branch '9.3.x' into 3035352-deprecate-filegetfilereferences so MR could be mergeble.

I have made a start rebasing on 10.1.x

Apologies for moving away from the open MR - Its against 9.3.x and I couldn't see how to change the MR to 10.x - it looks like I would have to create a new MR anyway, but if there's a way to change the MR to go against 10.1.x I can push these changes to that branch.

This patch takes the MR at bf95950f18f6960ddbd87b84df22dcced653066a (prior to 9.3.x merge commit) and
- Rebased onto 10.1.x
- Update deprecation warnings to deprecating in 10.1,0 removing in 11.0.0
- Addresses feedback from https://git.drupalcode.org/project/drupal/-/merge_requests/857#note_32161 , https://git.drupalcode.org/project/drupal/-/merge_requests/857#note_32162 , https://git.drupalcode.org/project/drupal/-/merge_requests/857#note_32198

I believe the only comment in the MR not addressed is https://git.drupalcode.org/project/drupal/-/merge_requests/857#note_32197 so leaving as needs work.

Oops, missing the change to file.services.yml in that one - ignore the patch above.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Did not test but looking at the code

For new properties think it would be nice to add typehints

CI failures in #69

Reading the CR and think it could use some tweaks. The 3 functions are deprecated but the before/after only referes to file_get_file_references() correct? And it's being replaced by dependency injection?

The other 2 seemed perfectly covered by the last sentence.

Try to Fix Cs issue & re-roll patch #69

Change record updates still need to happen.

1. +++ b/core/modules/file/src/FileUsage/FileUsageBase.php
   @@ -18,13 +22,58 @@
   +  protected $references = [];
   

   Should this be a memory cache https://www.drupal.org/project/drupal/issues/3047289

   Edit - removing this as it seems there are many examples of using field properties for things like this.

2. +++ b/core/modules/file/src/FileUsage/FileUsageInterface.php
   @@ -67,4 +69,30 @@ public function delete(FileInterface $file, $module, $type = NULL, $id = NULL, $
   +   * @param string $age
   

   Unless I'm missing something this param doesn't appear to work as described - its only used for the cache key

Adding related issues

#3361361: file_get_file_references does not return correct results when using $field param
#1805690: file_get_file_references() is rather bogus

Reviewed this issue at the DrupalSouth sprint day.

There are some issues with the current implementation of file_get_file_references are still an issue with the current patch.

There are some issues with the optional parameters - namely

• #3361361: file_get_file_references does not return correct results when using $field param
• #1805690: file_get_file_references is rather bogus (the issue with $age is still an issue)

As previously noted in #64 core is not making use of this filtering ability - doing a review of contrib I could not find any usage of filtering by field, and could only find 1 usage of filtering by type (https://www.drupal.org/project/protected_file). It may be worth discussing deprecating these without replacement?

Another possibility raised was that the function could be deprecated without replacement if core can provide a generic entity usage API. I have added a related issue #3361364 for this.

#1452100: Private file download returns access denied, when file attached to revision other than current is the primary related issue I'd say, where I've pointed out similar things on extra arguments and also provided an implementation basically deprecates the age argument as well as it's done as a fallback and implicitly.

> Another possibility raised was that the function could be deprecated without replacement if core can provide a generic entity usage API. I have added a related issue #3361364 for this.

I don't think entity_usage as it is can replace this function, that's not the problem it's trying to solve. The problem it's trying to solve is figure out which field, if any (as it could be referenced by an old revision), references a given file so that field access can be checked on that as well.

Looks like we should postpone on #1452100: Private file download returns access denied, when file attached to revision other than current then.

When this is picked back up, we should use a memory cache bin instead of class property.

The existing implementation has a problem with returning stale / outdated data.

Here is a patch that shows the issue, its against core rather than the work here and not completed but most relevant is the test which would still be relevant for this approach. Leaving #72 as the displayed patch as that is still the relevant starting point once this is no longer postponed.

This is postponed on #1452100: Private file download returns access denied, when file attached to revision other than current but in https://www.drupal.org/project/drupal/issues/1452100#comment-15209547 @berdir suggested we do it the other way around so we can deprecate the arguments here?

#1452100: Private file download returns access denied, when file attached to revision other than current now comes with a replacement service and full deprecation of this function. What's left is the cache invalidation stuff, which could be done in the referenced issue.

I think we can go ahead and close this as a duplicate now.