Add validation constraints to system.file

Tests are failing and I think we can fix that by just pushing the latest origin/11.x to this issue fork 🤞

Review posted on MR — if I'd found nothing in my review, then I'd have done #9 for you — but since I can't RTBC this yet anyway, I'll let you handle #9 too 🤓

On a standard profile install on 11.x
Installed configuration inspector and applied the MR

Shows that the system.file is fully validated.

Checking the MR everything appears to have a return type

Believe this one is good to go.

If path is deprecated we should deprecate it. See https://www.drupal.org/about/core/policies/core-change-policies/drupal-d...

Added some review comments.

Addressed feedback from #14

I think as per #14, a deprecation message needs to be added for path in system.schema.yml before removing it completely here.

Changes looks good to me.

One question on the MR.

I agree with catch in #19, moving this to Drupal\Core\Config seems like a better solution, let's do that.

Updated the class namespace as suggested, did do a rebase to clean up.

Back to rtbc now that it is moved.

If we changing the 10.3 database dumps because of invalid schema this points to us needing an update function. I think we should not be changing the dumps.

Also I think we should be adding a new constraint that works like callback but uses the class resolver service to my more flexible.

So, the path config key was deprecated in Drupal 8.8 it seems (https://www.drupal.org/node/3039255). That kinda feels out of scope of this change, the fact is that it will need to go through deprecation? I don't even really see any usage in core, but that could be my search skills failing me.

But i don't feel we should do that here. Hopefully you agree.

Waiting for build, since i had some ci issues ;x

Adjusted it to actually do what was asked for.

Did revert the deprecation message for the path property. Would like to know if we need to make the change here, or if we should defer that. Seems like that has been here for quite a while. I do think we might be able to ignore it here, since it doesnt seem to be used.

Edit:
A possible update would be:

function system_post_update_remove_path_key(): void {
  if (\Drupal::config('system.file')->get('path') !== NULL) {
    \Drupal::configFactory()->getEditable('system.file')
      ->clear('path')
      ->save();
  }
}

Edit 2:
After reading #3400368: Deprecate path.temporary in system.file configuration schema it was mostly path.temporary that was deprecated. Although it seems that was the only key that used that. So the deprecation does make sense, and an upgrade path also make sense to remove the key from the config.

Lets do that.

Ok, lets try again. Seems good now.

#3517070: Remove system.file.path config from system.schema.yml created to do the deprecation and upgrade path

Postpone on child issue.

Child is committed

Pipe is all green after the upgrade stuff in the child. Think we are there.

#3517070: Remove system.file.path config from system.schema.yml was merged, rebased this one.

Using configuration inspector

Does the new constraint need a CR? I vote yes but don't want to hold it up on that.

Rest appears to be addressed

Committed 63187d5 and pushed to 11.x. Thanks!

Published a change record