いやいや、bakeのcontrollerなんか使わんよ、という気もしますが、プログラミング初心者には結構ありがたいそうなのです。
しかし、Rails育ちのRESTfulおじさんにはモヤモヤポイントがあって悩んでいます。
環境
CakePHP 3.6.11。
一応、生成のコマンドを載せておきます。
(Productsはチュートリアルからもってきました)
bin/cake bake migration CreateProducts name:string
description:text created modified
bin/cake migrations migrate
bin/cake bake all products
生成されるコントローラ
<?php
namespace App\Controller;
use App\Controller\AppController;
/**
* Products Controller
*
* @property \App\Model\Table\ProductsTable $Products
*
* @method \App\Model\Entity\Product[]|\Cake\Datasource\ResultSetInterface paginate($object = null, array $settings = [])
*/
class ProductsController extends AppController
{
/**
* Index method
*
* @return \Cake\Http\Response|void
*/
public function index()
{
$products = $this->paginate($this->Products);
$this->set(compact('products'));
}
/**
* View method
*
* @param string|null $id Product id.
* @return \Cake\Http\Response|void
* @throws \Cake\Datasource\Exception\RecordNotFoundException When record not found.
*/
public function view($id = null)
{
$product = $this->Products->get($id, [
'contain' => []
]);
$this->set('product', $product);
}
/**
* Add method
*
* @return \Cake\Http\Response|null Redirects on successful add, renders view otherwise.
*/
public function add()
{
$product = $this->Products->newEntity();
if ($this->request->is('post')) {
$product = $this->Products->patchEntity($product, $this->request->getData());
if ($this->Products->save($product)) {
$this->Flash->success(__('The product has been saved.'));
return $this->redirect(['action' => 'index']);
}
$this->Flash->error(__('The product could not be saved. Please, try again.'));
}
$this->set(compact('product'));
}
/**
* Edit method
*
* @param string|null $id Product id.
* @return \Cake\Http\Response|null Redirects on successful edit, renders view otherwise.
* @throws \Cake\Network\Exception\NotFoundException When record not found.
*/
public function edit($id = null)
{
$product = $this->Products->get($id, [
'contain' => []
]);
if ($this->request->is(['patch', 'post', 'put'])) {
$product = $this->Products->patchEntity($product, $this->request->getData());
if ($this->Products->save($product)) {
$this->Flash->success(__('The product has been saved.'));
return $this->redirect(['action' => 'index']);
}
$this->Flash->error(__('The product could not be saved. Please, try again.'));
}
$this->set(compact('product'));
}
/**
* Delete method
*
* @param string|null $id Product id.
* @return \Cake\Http\Response|null Redirects to index.
* @throws \Cake\Datasource\Exception\RecordNotFoundException When record not found.
*/
public function delete($id = null)
{
$this->request->allowMethod(['post', 'delete']);
$product = $this->Products->get($id);
if ($this->Products->delete($product)) {
$this->Flash->success(__('The product has been deleted.'));
} else {
$this->Flash->error(__('The product could not be deleted. Please, try again.'));
}
return $this->redirect(['action' => 'index']);
}
}
以下のエンドポイントができるのを想定しているものと思います。
「思います」と書いたのはCakePHPでは config/routes.php に書かなくてもメソッドが外から呼べるため、正確にはわからないからです。
-
get /products
で一覧を取得 -
get /products/:id
で一覧を詳細取得 -
get /products/add
で新規追加画面表示 -
post /products/add
で新規追加処理 -
get /products/edit
で更新画面表示 -
patch or post or put /products/edit
で更新処理 -
post or delete /products/:id
で削除処理
モヤモヤポイント
ざっと眺めて、嫌な臭いを感じて「よし! 本腰をいれてくぞ!」って気持ちになるのは僕だけではないはず…?
例としてaddをコードレビューしてみる。
public function add()
{
$product = $this->Products->newEntity();
# [must] 新規追加画面の表示と新規追加の処理を分けたほうが見通しがよいです
if ($this->request->is('post')) {
# [must] routesでpostだけを許可すればこの分岐はいりませんね
$product = $this->Products->patchEntity($product, $this->request->getData());
if ($this->Products->save($product)) {
$this->Flash->success(__('The product has been saved.'));
return $this->redirect(['action' => 'index']);
}
$this->Flash->error(__('The product could not be saved. Please, try again.'));
# [imo] 成功の場合はindexに、そうでない場合はこのまま、ということですね。
# 処理が読みづらく感じました。
}
# [must] post以外は新規追加画面表示にするというのは乱暴すぎやしませんか?
# 他は`allowMethod`で弾いてるのにここはなんでやらないんですか?
$this->set(compact('product'));
}
fatなコントローラはどこでもよく目にしますが、これがそのまま育っていくと思うと、頭が痛くなってきます。
(もちろん実際見ました)
そして経験の浅い方は「CakePHPが出力したコードだ。これでよいのだ!」と思いますよね。
どうすればいいか考える
CakePHPにはRESTfulなルーティングを生成する機能があります。
config/routes.php
$routes->resources('Products');
生成されるエンドポイント。
bin/cake routes | grep product
| products:index | /products | {"_method":"GET","action":"index","controller":"Products","plugin":null} |
| products:add | /products | {"_method":"POST","action":"add","controller":"Products","plugin":null} |
| products:view | /products/:id | {"_method":"GET","action":"view","controller":"Products","plugin":null} |
| products:edit | /products/:id | {"_method":["PUT","PATCH"],"action":"edit","controller":"Products","plugin":null} |
| products:delete | /products/:id | {"_method":"DELETE","action":"delete","controller":"Products","plugin":null} |
そうそう、こういうのでいいんだよ。
これを使えば、if ($this->request->is('post')) {
や allowMethod
を一掃できそうですね。
しかし…
以下のエンドポイントがありません。APIでの利用を想定している、ということでしょう。
(APIの場合、パラメータを入力するための画面はいらないので)
-
get /products/add
で新規追加画面表示 -
get /products/edit
で更新画面表示
こう…?
$routes->resources('Products', ['map' => [
'add' => [
'action' => 'new',
'method' => 'GET'
],
':id/edit' => [
'action' => 'viewEdit',
'method' => 'GET'
]
]]);
edit
はもう使われているので、名前が悩ましい…。
| products:index | /products | {"_method":"GET","action":"index","controller":"Products","plugin":null} |
| products:add | /products | {"_method":"POST","action":"add","controller":"Products","plugin":null} |
| products:view | /products/:id | {"_method":"GET","action":"view","controller":"Products","plugin":null} |
| products:edit | /products/:id | {"_method":["PUT","PATCH"],"action":"edit","controller":"Products","plugin":null} |
| products:delete | /products/:id | {"_method":"DELETE","action":"delete","controller":"Products","plugin":null} |
| products:new | /products/add | {"_method":"GET","action":"new","controller":"Products","plugin":null} |
| products:viewedit | /products/:id/edit | {"_method":"GET","action":"viewEdit","controller":"Products","plugin":null} |
うーん、冗長感あるし、メソッド名がフレームワークで規定されないのが悶々とする。
ベターな解決策があれば教えていただけると幸いです
参考
Railsの場合。
sites GET /sites(.:format) sites#index
POST /sites(.:format) sites#create
new_site GET /sites/new(.:format) sites#new
edit_site GET /sites/:id/edit(.:format) sites#edit
site GET /sites/:id(.:format) sites#show
PATCH /sites/:id(.:format) sites#update
PUT /sites/:id(.:format) sites#update
DELETE /sites/:id(.:format) sites#destroy
そうそう、こういうのでいいんだよ。