Uri should be required base field for file entities

Are you sure we don't need an update path for it?

We need to clear the cached definitions - just need an empty hook_update_N in the file module for this.

1. +++ b/core/modules/file/file.install
   @@ -120,3 +120,19 @@ function file_requirements($phase) {
   + * The simple presence of this update function clears cached field definitions.
   + */
   +function file_update_8001() {
   +  // Uri base field is now required.
   +}
   

   The description of this function is shown in the UI when running updates. So it should be written in a way that tells the user what it does.

2. +++ b/core/modules/file/src/Entity/File.php
   @@ -244,6 +244,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
          ->setDescription(t('The URI to access the file (either local or remote).'))
   +      ->setRequired(TRUE)
          ->setSetting('max_length', 255)
   

   I agree that it makes sense that URI should be required.

   But why does the schema have a NOT NULL if the field is not required? Are we altering the schema somewhere manually or automatically (e.g. due to the existence of an index on that column?)

   Checking.... Yeah, FileStorageSchema adds that not null automatically due to the call to addSharedTableFieldIndex().

   That doesn't seem 100% right.. it should only do that if the field is indeed required? Or maybe throw an exception if there's a mismatch?

   Might be good to have some feedback from @plach.

The following error happened when submitted /webform/%/test

Drupal\Core\Entity\EntityStorageException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'uri' cannot be null: UPDATE {file_managed} SET fid=:db_update_placeholder_0, type=:db_update_placeholder_1, uuid=:db_update_placeholder_2, langcode=:db_update_placeholder_3, uid=:db_update_placeholder_4, filename=:db_update_placeholder_5, uri=:db_update_placeholder_6, filemime=:db_update_placeholder_7, filesize=:db_update_placeholder_8, status=:db_update_placeholder_9, created=:db_update_placeholder_10, changed=:db_update_placeholder_11 WHERE fid = :db_condition_placeholder_0; Array ( [:db_update_placeholder_0] => 852 [:db_update_placeholder_1] => undefined [:db_update_placeholder_2] => f3f5989c-82ac-4e9b-8110-6b865e760501 [:db_update_placeholder_3] => en [:db_update_placeholder_4] => 1 [:db_update_placeholder_5] => [:db_update_placeholder_6] => [:db_update_placeholder_7] => application/octet-stream [:db_update_placeholder_8] => [:db_update_placeholder_9] => 0 [:db_update_placeholder_10] => 1499292302 [:db_update_placeholder_11] => 1499292302 [:db_condition_placeholder_0] => 852 ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 777 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Drupal\Core\Database\Statement->execute(Array, Array) (Line: 624)
Drupal\Core\Database\Connection->query('UPDATE {file_managed} SET fid=:db_update_placeholder_0, type=:db_update_placeholder_1, uuid=:db_update_placeholder_2, langcode=:db_update_placeholder_3, uid=:db_update_placeholder_4, filename=:db_update_placeholder_5, uri=:db_update_placeholder_6, filemime=:db_update_placeholder_7, filesize=:db_update_placeholder_8, status=:db_update_placeholder_9, created=:db_update_placeholder_10, changed=:db_update_placeholder_11
WHERE fid = :db_condition_placeholder_0', Array, Array) (Line: 87)
Drupal\Core\Database\Driver\mysql\Connection->query('UPDATE {file_managed} SET fid=:db_update_placeholder_0, type=:db_update_placeholder_1, uuid=:db_update_placeholder_2, langcode=:db_update_placeholder_3, uid=:db_update_placeholder_4, filename=:db_update_placeholder_5, uri=:db_update_placeholder_6, filemime=:db_update_placeholder_7, filesize=:db_update_placeholder_8, status=:db_update_placeholder_9, created=:db_update_placeholder_10, changed=:db_update_placeholder_11
WHERE fid = :db_condition_placeholder_0', Array, Array) (Line: 148)
Drupal\Core\Database\Query\Update->execute() (Line: 822)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->doSaveFieldItems(Object) (Line: 263)
Drupal\Core\Entity\ContentEntityStorageBase->doSave('852', Object) (Line: 392)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 768)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 364)
Drupal\Core\Entity\Entity->save() (Line: 32)
Drupal\file\FileUsage\FileUsageBase->delete(Object, 'webform', 'webform_submission', '578', 0) (Line: 92)
Drupal\file\FileUsage\DatabaseFileUsageBackend->delete(Object, 'webform', 'webform_submission', '578', 0) (Line: 354)
Drupal\webform\Plugin\WebformElement\WebformManagedFileBase->postSave(Array, Object, ) (Line: 149)
Drupal\webform\Plugin\WebformElementManager->invokeMethod('postSave', Array, Object, , NULL) (Line: 1802)
Drupal\webform\Entity\Webform->invokeElements('postSave', Object, , NULL) (Line: 826)
Drupal\webform\WebformSubmissionStorage->invokeWebformElements('postSave', Object, ) (Line: 767)
Drupal\webform\WebformSubmissionStorage->doPostSave(Object, ) (Line: 395)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 768)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 364)
Drupal\Core\Entity\Entity->save() (Line: 668)
Drupal\webform\Entity\WebformSubmission->save() (Line: 1046)
Drupal\webform\WebformSubmissionForm->save(Array, Object)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 585)
Drupal\Core\Form\FormBuilder->processForm('webform_submission_repairs_taps_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('webform_submission_repairs_taps_form', Object) (Line: 48)
Drupal\Core\Entity\EntityFormBuilder->getForm(Object, 'default') (Line: 877)

I haven't test the patch yet.

Here's the patch for drupal 8.7.4.

I choosed not to include changes on file.install and clear the caches manually.

This needs to reroll from #5 and include the changes from @Berdirs review

Rerolled patch against #5 and included the changes from @Berdirs review

Patch #24 applied successfully in 9.5.x-dev branch

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. 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 require as a guide.

@immaculatexavier thank you for the patch but please include an interdiff so we can see the changes.

From what I can tell #8.1 or #8.2 have not been addressed.
8.1 = the description appears to be the same from what it was
8.2 = dont' see any follow up for that.

Also the hook update is targeting D8 so that's not correct.

Thanks