Deprecate FIELD_LOAD_* constants

The actual usage is internal.

I guess the problem are the places that happen to use it from the outside, like EFQ (should really be a different constant, as it has a different meaning), maybe use loadRevisions() instead of age(constant)?

Then there's views integration, no idea what that is exactly doing and that file reference stuff...

Marked #2029709: Convert FIELD_LOAD_CURRENT to a constant on some class/interface as a dupe of this.

rephrase

Is this issue valid? Constants are in EntityStorageControllerInterface
Suppose it make sense to remove FIELD_ prefix

4: 2081513-4.patch queued for re-testing.

4: 2081513-4.patch queued for re-testing.

Hello
Replaced FIELD_LOAD_* constants by boolean flag.
Removed QueryInterface::age() and created a new method QueryInterface::loadRevisions().

Regards

In field.views.inc I found the use of FALSE/TRUE as array keys entirely unintuitive, and this also seems to be the cause of the test failure. Instead it seems safe to use strings here, as they are only ever referred to inside the same function; the attached patch uses 'current' and 'revision' here.

Discussed this issue with @Berdir in IRC as it is related to #2458543: Entity query age(EntityStorageInterface::FIELD_LOAD_REVISION) only gets current revision ID. That issue makes this one much simpler and also makes the constant have even less meaning and relevance. So this is a followup of that issue.

Also the use of the $age param in file_get_file_references() looks very broken.

Yes, only usages in file and views are remaining I think.

Anyone up for reroll?

Tagging as novice for the reroll.

Patch rerolled

That reroll happened a bit early, #2458543: Entity query age(EntityStorageInterface::FIELD_LOAD_REVISION) only gets current revision ID was not yet committed. Let's try again :)

I think better to use $load_revisions as param name, because it mans to load all revisions on not

+++ b/core/modules/file/file.module
@@ -1431,13 +1427,13 @@ function file_icon_map($mime_type) {
-function file_get_file_references(FileInterface $file, FieldDefinitionInterface $field = NULL, $age = EntityStorageInterface::FIELD_LOAD_REVISION, $field_type = 'file') {
+function file_get_file_references(FileInterface $file, FieldDefinitionInterface $field = NULL, $load_revisions = TRUE, $field_type = 'file') {
...
-  if (!isset($references[$file->id()][$age])) {
-    $references[$file->id()][$age] = array();
+  if (!isset($references[$file->id()][$load_revisions])) {
+    $references[$file->id()][$load_revisions] = array();

@@ -1470,7 +1466,7 @@ function file_get_file_references(FileInterface $file, FieldDefinitionInterface
-                $references[$file->id()][$age][$field_name][$entity_type_id][$entity->id()] = $entity;
+                $references[$file->id()][$load_revisions][$field_name][$entity_type_id][$entity->id()] = $entity;

@@ -1479,7 +1475,7 @@ function file_get_file_references(FileInterface $file, FieldDefinitionInterface
-  $return = $references[$file->id()][$age];
+  $return = $references[$file->id()][$load_revisions];

+++ b/core/modules/file/src/FileAccessControlHandler.php
@@ -62,7 +62,7 @@ protected function checkAccess(EntityInterface $entity, $operation, $langcode, A
-    return file_get_file_references($file, NULL, EntityStorageInterface::FIELD_LOAD_REVISION, NULL);
+    return file_get_file_references($file, NULL, TRUE, NULL);

This is just swapping the param name - fine but it actually does not do anything. Which is a bug. And looking at FileAccessControlHandler - is this a security issue?

This issue is a major task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

@Berdir does the issue still makes sense for 8.0?

8.1 material, but needs BC now

Chaning title to reflect new aim

Wondering what we want to do exactly? There are two remaining uses for that constant:

1. file_get_file_references() very strange left-over that uses that constant in a completely unrelated way. I guess we could add new constants with the same value, but I'd rather just move the whole function to a service with a better API and deprecate the old function completely.

2. (apparently) completely internal usage in views_field_default_views_data(). We could just stop using them there I think and instead use a string or so. Or we could just keep them and deprecate anyway.

1. would need a separate issue I think and this would be blocked on that, after that we could just deprecate the constants and either update 2. to use something else or don't care and assume the whole function will sooner or later be rewritten or moved and deprecated as a whole (e.g. there's the pending issue of integrating per field type views data into EntityViewsData).

That said, whatever we do, I don't think this is a major anymore. Usage of this was far more widespread when this issue was created and I think the most problematic/confusing one was in entity queries. But any usage in storage handlers and entity query has been refactored away in other issues.

And one remaining public usage in an API function that is weird on its own isn't a major problem IMHO.

@Berdir now revision api in core any idea how to clean-up this?