[>=8.5] Remove JSON API's "file URL" field work-around now that Drupal core 8.5 fixed it

Something like this.

Title was inaccurate.

Also needs BC tests.

#6: reply to follow later.

For now … #2825487: Fix normalization of File entities: file entities should expose the file URL as a computed property on the 'uri' base field landed, which means this is now unblocked! Except … that we need #2921257: On Drupal 8.5, JSON API should respect Typed Data's new ::isInternal(), and JSON API Extras should use ::setInternal() to be fixed first.

If possible, #2921257: On Drupal 8.5, JSON API should respect Typed Data's new ::isInternal(), and JSON API Extras should use ::setInternal() should land first, and then this should take care of BC/update path concerns.

Updating to PP-2, because #2921257 is PP-1.

#11: yep, the extra step you've taken there is a hard blocker for this issue. And for that, we have a separate issue: #2921257.

Would you like to help out with that? :) Initial patch at #2921257-10: On Drupal 8.5, JSON API should respect Typed Data's new ::isInternal(), and JSON API Extras should use ::setInternal(), which includes your interdiff in #11, plus one extra necessary change. Would be great to get to a green patch there!

To clarify: this is blocked on #2921257: On Drupal 8.5, JSON API should respect Typed Data's new ::isInternal(), and JSON API Extras should use ::setInternal() which is blocked on #2930028: Comprehensive JSON API integration test coverage phase 1: for every entity type, individual resources only.

#2930028: Comprehensive JSON API integration test coverage phase 1: for every entity type, individual resources only landed, less blocked now.

1. +++ b/jsonapi.install
   @@ -0,0 +1,35 @@
   +function jsonapi_install() {
   +  // For new sites, also still enable JSON API's custom base field, if it's a
   +  // new site starting on Drupal <8.5. Because only in Drupal >=8.5, core ships
   +  // with an alternative.
   +  // @todo Remove this when JSON API requires Drupal 8.5.
   +  if (floatval(\Drupal::VERSION) < 8.5) {
   

   This ensures that new sites starting to use JSON API will get the BC layer if they use Drupal 8.4 or older, and will not get the BC layer of they use Drupal 8.5 or newer.

2. +++ b/jsonapi.install
   @@ -0,0 +1,35 @@
   +function jsonapi_update_8101() {
   

   This ensures all existing sites using JSON API continue to behave exactly the same.

I wrote #16 to explain the patch, and in doing so, I found a mistake in a part I also wanted to leave a comment for.

CR created: https://www.drupal.org/node/2947882.

Ah, darn, I wish you'd said that in a previous comment … oh well, that happens :)

We need to give people time to update, so your second suggestion is the only one that can really work.

I'm not sure how exactly you envision that would work, so assigning to you.

Sorry, hadn't seen #25 until now!

I would love to not have to deal with BC configuration. But that's the right thing to do to ease the transition.

1. Creating a new major release and dropping the legacy code is a way to not have to deal with BC. It would also mean we could require Drupal 8.5 as the minimum version, which also would allow us to remove lots existing version-dependent logic (and especially tests). Plus, it'd make it easier to move JSON API into core. So … that sounds wonderful! :)
2. Supporting both URLs is exactly what #20 does, but with a BC config flag so that only existing sites/sites already in development can continue to work without making any changes. New sites would only get the "proper" behavior.

   The approach you propose would also allow new sites to use the old behavior. The deprecation notice in meta would be easily missed. I don't understand how you could use Partial Success for this, can you explain that?

I'd love to do #1, but it does force JSON API users to adjust their consumer code. If we can live with that, then sure, yes, that sounds great!

I have the impression that @e0ipso wants to branch version 2 and remove this in there? Tentatively moving to that branch and awaiting his feedback.

The 2.x branch is about to start … 🚀

So, time to reroll this patch! I'm very grateful to @e0ipso for insisting to not add a BC layer and instead only drop this in the next version of the JSON API module. Because it makes this an order of magnitude simpler!

I missed two details :)

CR created: https://www.drupal.org/node/2982209.

IS updated.

Green patch, simple patch, epic diffstat (5 files changed, 1 insertions, 185 deletions), based on consensus with @e0ipso. What's not to like? :)