Skip to content

Commit c534a5f

Browse files
tabacitutabacitu
authored
Merge pull request #4529 from Laravel-Backpack/refactor-request-anonymous-function
remove anonymous function
2 parents d97c8e6 + 63b95a1 commit c534a5f

File tree

5 files changed

+295
-35
lines changed

5 files changed

+295
-35
lines changed

‎src/app/Library/CrudPanel/Traits/Validation.php‎

Lines changed: 51 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Backpack\CRUD\app\Library\CrudPanel\Traits;
44

5+
use Illuminate\Foundation\Http\FormRequest;
56
use Illuminate\Support\Str;
67

78
trait Validation
@@ -98,7 +99,7 @@ private function getValidationMessagesFromFieldsAndSubfields($fields)
9899
});
99100

100101
foreach ($subfieldsWithValidationMessages as $subfield) {
101-
foreach ($subfield['validationMessages'] as $rule => $message) {
102+
foreach ($subfield['validationMessages'] ?? [] as $rule => $message) {
102103
$messages[$item['name'].'.*.'.$subfield['name'].'.'.$rule] = $message;
103104
}
104105
}
@@ -133,7 +134,7 @@ public function setValidation($classOrRulesArray = false, $messages = [])
133134
$this->setValidationFromFields();
134135
} elseif (is_array($classOrRulesArray)) {
135136
$this->setValidationFromArray($classOrRulesArray, $messages);
136-
} elseif (is_string($classOrRulesArray) || is_class($classOrRulesArray)) {
137+
} elseif (is_string($classOrRulesArray) && class_exists($classOrRulesArray) && is_a($classOrRulesArray, FormRequest::class, true)) {
137138
$this->setValidationFromRequest($classOrRulesArray);
138139
} else {
139140
abort(500, 'Please pass setValidation() nothing, a rules array or a FormRequest class.');
@@ -202,55 +203,71 @@ public function validateRequest()
202203
return app($formRequest);
203204
}
204205

205-
// create an alias of the provided FormRequest so we can create a new class that extends it.
206-
// we can't use $variables to extend classes.
207-
class_alias(get_class(new $formRequest), 'DeveloperProvidedFormRequest');
206+
$formRequest = (new $formRequest)->createFrom($this->getRequest());
207+
$extendedRules = $this->mergeRules($formRequest, $rules);
208+
$extendedMessages = array_merge($messages, $formRequest->messages());
208209

209-
// create a new anonymous class that will extend the provided developer FormRequest
210-
// in this class we will merge the FormRequest rules() and messages() with the ones provided by developer in fields.
211-
$extendedRequest = new class($rules, $messages) extends \DeveloperProvidedFormRequest
212-
{
213-
private $_rules;
214-
215-
private $_messages;
210+
// validate the complete request with FormRequest + controller validation + field validation (our anonymous class)
211+
return $this->checkRequestValidity($extendedRules, $extendedMessages, $formRequest);
212+
}
216213

217-
public function __construct($rules, $messages)
218-
{
219-
parent::__construct();
220-
$this->_rules = $rules;
221-
$this->_messages = $messages;
222-
}
214+
return ! empty($rules) ? $this->checkRequestValidity($rules, $messages) : $this->getRequest();
215+
}
223216

224-
public function rules()
225-
{
226-
return array_merge(parent::rules(), $this->_rules);
227-
}
217+
/**
218+
* Return an array containing the request rules and the field/controller rules merged.
219+
* The rules in request will take precedence over the ones in controller/fields.
220+
*
221+
* @param \Illuminate\Http\Request $request
222+
* @param array $rules
223+
* @return array
224+
*/
225+
private function mergeRules($request, $rules)
226+
{
227+
$extendedRules = [];
228+
$requestRules = $this->getRequestRulesAsArray($request);
229+
$rules = array_map(function ($ruleDefinition) {
230+
return is_array($ruleDefinition) ? $ruleDefinition : explode('|', $ruleDefinition);
231+
}, $rules);
232+
233+
foreach ($requestRules as $ruleKey => $rule) {
234+
$extendedRules[$ruleKey] = array_key_exists($ruleKey, $rules) ? array_merge($rule, $rules[$ruleKey]) : $rule;
235+
unset($rules[$ruleKey]);
236+
}
228237

229-
public function messages()
230-
{
231-
return array_merge(parent::messages(), $this->_messages);
232-
}
233-
};
238+
return array_merge($rules, $extendedRules);
239+
}
234240

235-
// validate the complete request with FormRequest + controller validation + field validation (our anonymous class)
236-
return app(get_class($extendedRequest), ['rules' => $rules, 'messages' => $messages]);
241+
/**
242+
* Return the request rules as an array of rules if developer provided a rule string configuration.
243+
*
244+
* @param \Illuminate\Http\Request $request
245+
* @return array
246+
*/
247+
private function getRequestRulesAsArray($request)
248+
{
249+
$requestRules = [];
250+
foreach ($request->rules() as $ruleKey => $rule) {
251+
$requestRules[$ruleKey] = is_array($rule) ? $rule : explode('|', $rule);
237252
}
238253

239-
return ! empty($rules) ? $this->checkRequestValidity($rules, $messages) : $this->getRequest();
254+
return $requestRules;
240255
}
241256

242257
/**
243-
* Checks if the current crud request is valid against the provided rules.
258+
* Checks if the request is valid against the rules.
244259
*
245260
* @param array $rules
246261
* @param array $messages
262+
* @param \Illuminate\Http\Request|null $request
247263
* @return \Illuminate\Http\Request
248264
*/
249-
private function checkRequestValidity($rules, $messages)
265+
private function checkRequestValidity($rules, $messages, $request = null)
250266
{
251-
$this->getRequest()->validate($rules, $messages);
267+
$request = $request ?? $this->getRequest();
268+
$request->validate($rules, $messages);
252269

253-
return $this->getRequest();
270+
return $request;
254271
}
255272

256273
/**

‎tests/Unit/CrudPanel/CrudPanelValidationTest.php‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
<?php
2+
3+
namespace Backpack\CRUD\Tests\Unit\CrudPanel;
4+
5+
use Backpack\CRUD\Tests\Unit\Http\Requests\UserRequest;
6+
use Backpack\CRUD\Tests\Unit\Models\User;
7+
8+
/**
9+
* @covers Backpack\CRUD\app\Library\CrudPanel\Traits\Validation
10+
*/
11+
class CrudPanelValidationTest extends BaseDBCrudPanelTest
12+
{
13+
public function testItThrowsValidationExceptions()
14+
{
15+
$this->crudPanel->setModel(User::class);
16+
$this->crudPanel->setValidation(UserRequest::class);
17+
18+
$request = request()->create('users/', 'POST', [
19+
'email' => '[email protected]',
20+
'password' => 'test',
21+
]);
22+
23+
$this->crudPanel->setRequest($request);
24+
$this->expectException(\Illuminate\Validation\ValidationException::class);
25+
$validatedRequest = $this->crudPanel->validateRequest();
26+
}
27+
28+
public function testItMergesFieldValidationWithRequestValidation()
29+
{
30+
$this->crudPanel->setModel(User::class);
31+
$this->crudPanel->setValidation(UserRequest::class);
32+
33+
$request = request()->create('users/', 'POST', [
34+
'name' => 'test name',
35+
'email' => '[email protected]',
36+
'password' => 'test',
37+
]);
38+
39+
$request->setRouteResolver(function () use ($request) {
40+
return (new Route('POST', 'users', ['Backpack\CRUD\Tests\Unit\Http\Controllers\UserCrudController', 'create']))->bind($request);
41+
});
42+
43+
$this->crudPanel->addFields([
44+
[
45+
'name' => 'email',
46+
'validationRules' => 'required',
47+
],
48+
[
49+
'name' => 'name',
50+
],
51+
[
52+
'name' => 'password',
53+
],
54+
]);
55+
56+
$this->crudPanel->setRequest($request);
57+
58+
$this->crudPanel->validateRequest();
59+
60+
$this->assertEquals(['email'], array_keys($this->crudPanel->getOperationSetting('validationRules')));
61+
}
62+
63+
public function testItMergesAllKindsOfValidation()
64+
{
65+
$this->crudPanel->setModel(User::class);
66+
67+
$this->crudPanel->setOperation('create');
68+
$this->crudPanel->setValidation([
69+
'password' => 'required',
70+
]);
71+
$this->crudPanel->setValidation(UserRequest::class);
72+
73+
$request = request()->create('users/', 'POST', [
74+
'name' => '',
75+
'password' => '',
76+
'email' => '',
77+
]);
78+
79+
$request->setRouteResolver(function () use ($request) {
80+
return (new Route('POST', 'users', ['Backpack\CRUD\Tests\Unit\Http\Controllers\UserCrudController', 'create']))->bind($request);
81+
});
82+
83+
$this->crudPanel->addFields([
84+
[
85+
'name' => 'email',
86+
'validationRules' => 'required',
87+
],
88+
[
89+
'name' => 'name',
90+
],
91+
[
92+
'name' => 'password',
93+
],
94+
]);
95+
96+
$this->crudPanel->setRequest($request);
97+
98+
$this->expectException(\Illuminate\Validation\ValidationException::class);
99+
100+
try {
101+
$this->crudPanel->validateRequest();
102+
} catch (\Illuminate\Validation\ValidationException $e) {
103+
$this->assertEquals(['password', 'email', 'name'], array_keys($e->errors()));
104+
throw $e;
105+
}
106+
}
107+
108+
public function testItCanGetTheValidationFromFields()
109+
{
110+
$this->crudPanel->setModel(User::class);
111+
$this->crudPanel->setOperation('create');
112+
113+
$request = request()->create('users/', 'POST', [
114+
'name' => 'test name',
115+
'email' => '[email protected]',
116+
'password' => 'test',
117+
]);
118+
119+
$request->setRouteResolver(function () use ($request) {
120+
return (new Route('POST', 'users', ['Backpack\CRUD\Tests\Unit\Http\Controllers\UserCrudController', 'create']))->bind($request);
121+
});
122+
123+
$this->crudPanel->addField([
124+
'name' => 'email',
125+
'validationRules' => 'required',
126+
]);
127+
128+
$this->crudPanel->addField([
129+
'name' => 'name',
130+
'validationRules' => 'required',
131+
'validationMessages' => [
132+
'required' => 'required ma friend',
133+
],
134+
]);
135+
136+
$this->crudPanel->addField([
137+
'name' => 'password',
138+
'subfields' => [
139+
[
140+
'name' => 'test',
141+
'validationRules' => 'required',
142+
'validationMessages' => [
143+
'required' => 'required ma friend',
144+
],
145+
],
146+
],
147+
]);
148+
149+
$this->crudPanel->setRequest($request);
150+
151+
$this->crudPanel->setValidation();
152+
153+
$validatedRequest = $this->crudPanel->validateRequest();
154+
155+
$this->assertEquals(['email', 'name', 'password.*.test'], array_keys($this->crudPanel->getOperationSetting('validationRules')));
156+
}
157+
158+
public function testItThrowsExceptionWithInvalidValidationClass()
159+
{
160+
$this->crudPanel->setModel(User::class);
161+
$this->crudPanel->setOperation('create');
162+
163+
try {
164+
$this->crudPanel->setValidation('\Backpack\CRUD\Tests\Unit\Models\User');
165+
} catch (\Throwable $e) {
166+
}
167+
$this->assertEquals(
168+
new \Symfony\Component\HttpKernel\Exception\HttpException(500, 'Please pass setValidation() nothing, a rules array or a FormRequest class.'),
169+
$e
170+
);
171+
}
172+
173+
public function testItCanDisableTheValidation()
174+
{
175+
$this->crudPanel->setModel(User::class);
176+
$this->crudPanel->setOperation('create');
177+
$this->crudPanel->setValidation([
178+
'name' => 'required',
179+
'password' => 'required',
180+
]);
181+
$this->crudPanel->setValidation(UserRequest::class);
182+
$this->assertEquals(['name', 'password'], array_keys($this->crudPanel->getOperationSetting('validationRules')));
183+
184+
$this->crudPanel->disableValidation();
185+
$this->assertEquals([], $this->crudPanel->getOperationSetting('validationRules'));
186+
$this->assertEquals([], $this->crudPanel->getOperationSetting('validationMessages'));
187+
$this->assertEquals([], $this->crudPanel->getOperationSetting('requiredFields'));
188+
$this->assertEquals(false, $this->crudPanel->getFormRequest());
189+
}
190+
191+
public function testItCanGetTheRequiredFields()
192+
{
193+
$this->crudPanel->setModel(User::class);
194+
$this->crudPanel->setOperation('create');
195+
$this->assertFalse($this->crudPanel->isRequired('test'));
196+
$this->crudPanel->setValidation([
197+
'email' => 'required',
198+
'password.*.test' => 'required',
199+
]);
200+
201+
$this->crudPanel->setValidation(UserRequest::class);
202+
$this->assertEquals(['email', 'password[test]', 'name'], array_values($this->crudPanel->getOperationSetting('requiredFields')));
203+
$this->assertTrue($this->crudPanel->isRequired('email'));
204+
$this->assertTrue($this->crudPanel->isRequired('password.test'));
205+
$this->assertTrue($this->crudPanel->isRequired('name'));
206+
}
207+
}

‎tests/Unit/Http/Controllers/UserCrudController.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,9 @@ public function setup()
2020
public function setupUpdateOperation()
2121
{
2222
}
23+
24+
protected function create()
25+
{
26+
return response('create');
27+
}
2328
}

‎tests/Unit/Http/Requests/UserRequest.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
namespace Backpack\CRUD\Tests\Unit\Http\Requests;
4+
5+
use Illuminate\Foundation\Http\FormRequest;
6+
7+
class UserRequest extends FormRequest
8+
{
9+
/**
10+
* Determine if the user is authorized to make this request.
11+
*
12+
* @return bool
13+
*/
14+
public function authorize()
15+
{
16+
// only allow updates if the user is logged in
17+
return true;
18+
}
19+
20+
/**
21+
* Get the validation rules that apply to the request.
22+
*
23+
* @return array
24+
*/
25+
public function rules()
26+
{
27+
return [
28+
'name' => 'required',
29+
];
30+
}
31+
}

‎tests81/Unit/CrudPanel/CrudPanelFieldsTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ public function testCheckReturnTypesForWhenInferingRelation()
1717
$this->crudPanel->addField('isARelation');
1818

1919
$this->assertEquals(false, $this->crudPanel->fields()['isAnAttribute']['entity']);
20-
$this->assertEquals(true, $this->crudPanel->fields()['isARelation']['entity']);
20+
$this->assertEquals('isARelation', $this->crudPanel->fields()['isARelation']['entity']);
2121
}
2222
}

0 commit comments

Comments
 (0)