Rest classes should use StringTranslationTrait instead of t() function when possible

It's best to tackle the whole rest module at once.

You missed 2 in other rest files using the trait.

- core/modules/rest/src/Plugin/views/row/DataFieldRow.php
- core/modules/rest/src/Plugin/ResourceBase.php

Should we tackle the other occurrences for t()? t() is not deprecated so I guess not.

fwiw I did a 'Find in path' for the rest directory using [\( ]{1}t\(

@clemens.tolboom
So you want to solve each instance inside REST right, afaik this issue was opened to just solve the instances in EntityResource itself.

@dawehner your are right about the scope @Cyberwolf fixed but it's base class was not fixed. As there were no followers I thought is was best to do those as well.

@Cyberwolf did a great job at the REST sprint yesterday and be in today probably for 50%.

Changing title accordingly.

Added summary and unassigned @Cyberwolf

This looks good.

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -67,7 +67,7 @@ public function get(EntityInterface $entity) {
-      throw new BadRequestHttpException(t('No entity content received.'));
+      throw new BadRequestHttpException($this->t('No entity content received.'));

@@ -77,16 +77,16 @@ public function post(EntityInterface $entity = NULL) {
-      throw new BadRequestHttpException(t('Invalid entity type'));
+      throw new BadRequestHttpException($this->t('Invalid entity type'));
...
-      throw new BadRequestHttpException(t('Only new entities can be created'));
+      throw new BadRequestHttpException($this->t('Only new entities can be created'));
...
-        throw new AccessDeniedHttpException(t('Access denied on creating field @field.', array('@field' => $field_name)));
+        throw new AccessDeniedHttpException($this->t('Access denied on creating field @field.', array('@field' => $field_name)));

@@ -101,7 +101,7 @@ public function post(EntityInterface $entity = NULL) {
-      throw new HttpException(500, t('Internal Server Error'), $e);
+      throw new HttpException(500, $this->t('Internal Server Error'), $e);

@@ -120,11 +120,11 @@ public function post(EntityInterface $entity = NULL) {
-      throw new BadRequestHttpException(t('No entity content received.'));
+      throw new BadRequestHttpException($this->t('No entity content received.'));
...
-      throw new BadRequestHttpException(t('Invalid entity type'));
+      throw new BadRequestHttpException($this->t('Invalid entity type'));

@@ -141,11 +141,11 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity
-          throw new AccessDeniedHttpException(t('Access denied on deleting field @field.', array('@field' => $field_name)));
+          throw new AccessDeniedHttpException($this->t('Access denied on deleting field @field.', array('@field' => $field_name)));
...
-          throw new AccessDeniedHttpException(t('Access denied on updating field @field.', array('@field' => $field_name)));
+          throw new AccessDeniedHttpException($this->t('Access denied on updating field @field.', array('@field' => $field_name)));

@@ -160,7 +160,7 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity
-      throw new HttpException(500, t('Internal Server Error'), $e);
+      throw new HttpException(500, $this->t('Internal Server Error'), $e);

@@ -187,7 +187,7 @@ public function delete(EntityInterface $entity) {
-      throw new HttpException(500, t('Internal Server Error'), $e);
+      throw new HttpException(500, $this->t('Internal Server Error'), $e);

Actually t() should NOT be used whilst throwing exceptions. Since if t() is broken as well then you can end up hiding the original exception.

[Edit: for correctness - see #14]

I have tried to implement changes suggested in #8

@balagan this issue should also remove the usage of t() in those exception messages that #8 shows.

@alexpott: I am confused. In #8 you wrote "Actually t() should be used whilst throwing exceptions."

If t() thould be removed in the exception messages, then what is wrong with the patch in #5?

@balagan oops I should have wrote "Actually t() should NOT be used whilst throwing exceptions." - sorry.

I still don't understand what is wrong with patch in #5 that uses $this->t(). Should $this->t() be removed as well?

Yep - we should not be translating exception messages ever. This can lead to further exceptions being thrown and can make it very difficult to track down what is really causing the problem.

t() and $this->t() both end up using the translation service - one is dependency injected the other is not. In class we should be using dependency injection to make our code unit testable and its dependencies transparent.

Thanks for the clear answer, working on a new patch.

I worked from patch in comment #5, so the interdiff is between patch #5 and patch #19

Corrected the wrong quotes.

Changed check_plain to String::checkPlain()

Inserted missing dollar sign in front of field_name variable

1. +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
   @@ -79,16 +80,16 @@ public function post(EntityInterface $entity = NULL) {
   +        throw new AccessDeniedHttpException('Access denied on creating field: ' .  String::checkPlain($field_name));
   

   This should use String::format('Access denied on creating field @field.', array('@field' => $field_name)

2. +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
   @@ -143,11 +144,11 @@ public function patch(EntityInterface $original_entity, EntityInterface $entity
   +          throw new AccessDeniedHttpException('Access denied on deleting field ' . String::checkPlain(field_name));
   ...
   +          throw new AccessDeniedHttpException('Access denied on updating field ' . String::checkPlain($field_name));
   

   Same as above.

Thanks for the tip, I fixed all 3 occurences according to #30.

Looks good. The patch applies and fixes every issue noted above; it should be ready to go.

+++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
@@ -79,16 +80,16 @@ public function post(EntityInterface $entity = NULL) {
+        throw new AccessDeniedHttpException( String::format('Access denied on creating field ', array('@field' => $field_name)));

Minor, but the space before String should be removed.

Removing that naughty space.

Awesome, thanks!

Removing usage of t() in exception messages is fixing a bug. This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed cf00c8c and pushed to 8.0.x. Thanks!