Skip to content

ModelAndView as HttpResponse #194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DanielPlainview
Copy link
Contributor

Позволяет делать редирект, выставлять заголовки/cookies более православно.

class HelloWorldController extends HttpController
{
    public function handleRequest(HttpRequest $request)
    {
        // ...

        if ($user->isLocked())
            return $this->createForbiddenResponse();

        $response =
            RawResponse::create()->
                setContent(sprintf('Hello, %s!', $user->getName()))->
        ;

        $response->getHeaderCollection()->set('Content-Type', 'text/plain');

        return $response;
    }
}

Во front controller будет что-то типа

try {
    $response = $controller->handleRequest($request);
} catch (Exception $e) {
    $logger->logException($e);
    $response = new ...;
}

$response->render();

}

public function __construct()
public function __construct(array $headers = array(), array $cookies = array())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему бы тогда до кучи не добавить ещё аргумент $view и аргумент $model? Чем эти два лучше их?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я хочу вообще отказаться от constructor injection в пользу setter injection, чтобы избежать проблемы с наследниками и несовместимым create. Будет, по крайней мере, последовательно.

@AlexeyDsov
Copy link
Member

Я уже написал комментарий в коде что сейчас ModelAndView это просто контейнер. Ну можно добавить в него ещё переменных которые будут возвращаться из контроллера и это будет дельно. Но заставлять его render'ить уже лишнее. Подобную логику стоит держать в отдельном классе обработавающим ModelAndView.

@DanielPlainview
Copy link
Contributor Author

Но заставлять его render'ить уже лишнее.

Спорно. ModelAndView вполне может делать render сам, чтобы соблюдать tell don't ask. В OnPHP вообще есть что-то типа FrontController'а? Иначе получается, что у многих будет повторение вида

$view = $mav->getView();
$headerCollection = $mav->getHeaderCollection();
$cookieCollection = $mav->getCookieCollection();
$status = $mav->getStatus();

if (is_string($view)) {
    $view = ...;
}

header($status->toString());
// send $headerCollection
$cookieCollection->httpSetAll();
echo $view->render($mav->getModel());

@AlexeyDsov
Copy link
Member

Мне кажется странным использовать ModelAndView для писем.

Ну MaV сейчас ни для чего не используется кроме передачи model и view. Странным может казаться любое его использование.

@DanielPlainview
Copy link
Contributor Author

Так ведь BC это не ломает, а странность его использования только подтолкнёт к рефакторингу.

Надо подумать по поводу FrontController.

@dovg
Copy link
Member

dovg commented Oct 3, 2013

Я положительно отношусь к необходимости где-то (не во вью) аккумулировать заголовки и все остальное, что связано с http, но ИМХО ModelAndView не совсем удачное место для этого.

Вот например, у меня сейчас в проекте почти везде два представления - html или json
поэтому заголовки в MaV мне не нужны, ибо они разные будут, и только фронт-контроллер знает какие отдавать.
А контроллеру приложения пофиг, он свою работу выполнил - данные собрал, объекты обновил.

Более того, есть случаи, когда контекст http вообще отсутствует, при этом стандартная MVC вполне может быть.

@DanielPlainview
Copy link
Contributor Author

Вот например, у меня сейчас в проекте почти везде два представления - html или json
поэтому заголовки в MaV мне не нужны

А откуда заголовки будут браться?

Более того, есть случаи, когда контекст http вообще отсутствует, при этом стандартная MVC вполне может быть.

Я не вижу реального кейса. Сейчас у нас есть HttpRequest, но внезапно нет HttpResponse. Я могу сделать отдельный класс, но это потребует существенной переработки существующего кода в проекте. Везде ожидается ModelAndView и с этим надо как-то жить.

фронт-контроллер знает какие отдавать

Не во FrontController, допустим. А в каком-то фильтре. Причём мне это видится как-то так:

class UserController implements Controller
{
    public function handleRequest(HttpRequest $request)
    {
        $user = User::dao()->getById($request->getGetVar('id'));

        return
            ModelAndView::create()->
                setView('viewUser')->
                setModel(
                    Model::create()->set('user', $user)
                );
    }
}

class ResponseFormatFilter extends DecoratorController
{
    private $view;

    public function __construct(CustomView $view)
    {
        $this->view = $view;
    }

    public function handleRequest(HttpRequest $request)
    {
        $response = parent::handleRequest($request);

        $format = $request->hasAttachedVar('format'); // Enumeration (HTML, JSON, XML, Protobuf, etc.)

        if (!$format->isHtml()) {
            $response->getHeaderCollection()->set('Content-Type', $format->getContentType());
            $response->setView($this->view->setFormat($format));
        }

        return $response;
    }
}

class CustomView implements View
{
    private $serializer;
    private $format;

    public function __construct(JMS\Serializer $serializer)
    {
        $this->serializer = $serializer;
    }

    public function setFormat(Format $format)
    {
        $this->format = $format;

        return $this;
    }

    public function render(Model $model)
    {
        echo $this->serializer($model->toArray(), $this->format->getName());

        return $this;
    }
}

GET /42.user.html => ...<span class="name">Blah</span>...
GET /42.user.json => {"user": {"id": 42, "name": "Blah"}}
GET /42.user.pb => бинарый protobuf response

@DanielPlainview
Copy link
Contributor Author

Кстати, а для ObjectNotFoundException можно запилить такой фильтр:

class NotFoundResourcesFilter extends DecoratorController
{
    private $logger;

    public function __construct(Logger $logger)
    {
        $this->logger = $logger;
    }

    public function handleRequest(HttpRequest $request)
    {
        try {
            $response = parent::handleRequest($request);
        } catch (ObjectNotFoundException $e) {
            $this->logger->logException($e);
            $response = $this->createNotFoundResponse();
        }

        return $response;
    }
}

Тогда в коде выше проверка наличия юзера не нужна и будет вполне RESTful.

@AlexeyDsov
Copy link
Member

Примеры выше это примеры лишь одного проекта. В других проектах другие решения. То что тут делает ResponseFormatFilter, в других местах делают другие классы подругому.

@stev
Copy link
Contributor

stev commented Oct 18, 2013

Идея поместить в MAV данные о заголовках, сомнительна

Может в мав помещать view а в в него уже заголовки ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants