Skip to content
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

REST: limit fields list for index and view actions #7098

Closed
RomeroMsk opened this issue Jan 30, 2015 · 37 comments
Closed

REST: limit fields list for index and view actions #7098

RomeroMsk opened this issue Jan 30, 2015 · 37 comments

Comments

@RomeroMsk
Copy link
Contributor

I'm using yii\rest\ActiveController and want to limit list of resource fields returned by index and view actions. I can use yii\rest\ViewAction::$findModel to set scenario of found model - it can help to restrict fields for view, but for index it is not so easy.
Why not add something like defaultFields (and deafultExtraFields) property to these actions and pass it to yii\rest\Serializer::getRequestedFields() to intersect with arrays from $_GET? Name defaultFields is not good for restriction, but you can suggest another names.

@fernandezekiel
Copy link
Contributor

what do you mean by restrict?
can it not be accomplished by configuring the fields() or extraFields()?

@RomeroMsk
Copy link
Contributor Author

Model has attr1, attr2, attr3, attr4. I need to show only attr1, attr2 and attr3 by default in index and view. If consumer will add ?fields=attr1,attr2 to URL, he will get only those. But if he will ask for ?fields=attr2,attr3,attr4, he will get an intersection (attr2 and attr3), because attr4 is not allowed.

@fernandezekiel
Copy link
Contributor

i probably would just deal with the model to do this, override the
toArray()

public function toArray(array $fields = [], array $expand = [], $recursive = true)
{
    if (empty($fields)) {
        $fields = ['username', 'id'];
    }

    return parent::toArray($fields, $expand, $recursive);
}

this would work for me because i have models for my api that extends my common models, i don't know if this would be ok to you

@fernandezekiel
Copy link
Contributor

if not, i suppose your solution about defaultFields in the Serializer would work better,
this will be declared in the controller though

@RomeroMsk
Copy link
Contributor Author

My idea is to make it easier than extending each model for API. BTW, you can do the same by overriding model's fields() method.

@fernandezekiel
Copy link
Contributor

yeah i guess

class UserController extends ActiveController
{
    public $serializer = [
        'class' => 'api\\Serializer',
        'defaultFields' => ['id', 'name']
    ];
...

@RomeroMsk
Copy link
Contributor Author

Also these new action properties can accept a callback to set different fields arrays according to API user. Then you can do it like this:

    public function actions()
    {
        $actions = parent::actions();
        $actions['view']['defaultFields'] = function () {
            if (Yii::$app->user->identity->username == 'testuser') {
                return ['attr1', 'attr2', 'attr3'];
            }
            return null;
        };
        return $actions;
    }

@RomeroMsk
Copy link
Contributor Author

You can set different arrays for different actions, so it could be better to use properties of Action, not of Serializer.

@fernandezekiel
Copy link
Contributor

but from an API user's perspective it wouldn't make sense to them if something is a field and doesn't show up by default, and then you have these extraFields thing which the name implies as something to add on what you get by default

@RomeroMsk
Copy link
Contributor Author

extraFields is usually using to get relations of AR model. Of course, we need to limit it too, as I mentioned.

@creocoder
Copy link
Contributor

I suggest to not overcomplicate things and not add ANY excess methods to framework. If you need restrictions make it standart way. Standart way in that case 1) using scenarios, 2) have base model and 2 children models with different fields(). If it will not help only yii\rest\* classes should be enhanced, Model should be untouched since everything fine with current architecture in it.

@RomeroMsk
Copy link
Contributor Author

Can you show how can I use scenarios in index action to implement this?

@creocoder
Copy link
Contributor

@RomeroMsk

(new MyModel(['scenario' => 'index']))->find()->all();

@creocoder
Copy link
Contributor

@RomeroMsk In case of yii\rest\IndexAction...

You can achive what you want by providing prepareDataProvider property for it.

@RomeroMsk
Copy link
Contributor Author

It doesn't work like this...

@creocoder
Copy link
Contributor

@RomeroMsk Yes, seems you can't manipulate scenarios of received models. Ok, than go subclassing way. Base model + 2 children models with different fields().

@creocoder
Copy link
Contributor

@RomeroMsk On other side there is another scenarios way. You can provide prepareDataProvider where before returning data provider you can change received models scenarios. So both methodics i suggested still works.

@RomeroMsk
Copy link
Contributor Author

Using subclassing of models - is the best way (without enhancing rest classes). But I think that limiting fields list for API is common use case, and we can make it a feature of Yii2 REST.

@creocoder
Copy link
Contributor

@RomeroMsk There is scenarios way with just different implementation (as i shown in my message above). So 2 ways without any additional core code.

@creocoder
Copy link
Contributor

@RomeroMsk But to be honest i do not like scenarios way since scenarios only for validation purposes and use it for anything else is IMO dirty hack. So from legal ways we have only subclassing way which will work. But since you said that different fields is common... Ok, than i suggest to look at this situation different way. And change fields() signature to theoretical fields($case = null).

@RomeroMsk
Copy link
Contributor Author

There are many features of framework which could be implemented by other ways (subclassing etc). But when it is common case, why not add new feature which can help to not write much code? :)
Thanks for your advices, now I want to see another opinions. If other devs will not like this, I'll use model subclassing.

@RomeroMsk
Copy link
Contributor Author

Yes, I thought about fields($case = null) too. And I don't like to use scenarios in this case too.

@creocoder
Copy link
Contributor

@RomeroMsk Lets think from that way. This will lead us to thinking around Serializer enhancements and probably will give more clean solution than mess new methods into Model or enhancing yii\rest\* classes. Looks like trouble you issued is relationship between Model and serializator. So by changing field() signature to fields($case = null) you'll get possibility to set up some case...

@creocoder
Copy link
Contributor

@RomeroMsk Seems everything is obvious now... We need yii\rest\Serializer::fieldsCase and yii\rest\Serializer::expandCase. Inside controller you can change its serializer property by choosing proper case. In your case fields case changed by different action. In another case it can be different. Very agile i think.

@creocoder
Copy link
Contributor

For example i want that my fields() configuration changes only when moon is full... etc. You want it only for index and view actions... You got idea :)

@RomeroMsk
Copy link
Contributor Author

My arguments for dealing with rest classes:
You have a base Model (in common application) and need to show it to partners via REST API, but not all attributes: partner needs to see only specific fields. And to implement this you suggest to change base model (to set different scenarios and change fields())... Why do I need to change base model to play with another application (api)? That's why subclassing of model is better. But if it is common for other devs, we can make an enhancement for rest classes to implement this only by setting some properties of action inside API controller - nice solution, isn't it?
Yes' I've got your idea. We can try to invent something flexible. But since Serializer is calling toArray(), it is the first place which must be refined. Then we need to give a flexible way for developer to set this props from ActiveController: by changing Serializer props in configuration array (as @fernandezekiel mentioned above; and this is good solution too, if I decide to not set different fields lists for two actions) or by passing fields list to Serializer from action somehow...

@fernandezekiel
Copy link
Contributor

funny enough, it seems that i had the need of something like this

@RomeroMsk
Copy link
Contributor Author

Any thoughts from core devs?

@cebe cebe added this to the 2.0.x milestone Feb 15, 2015
@EvgenyOrekhov
Copy link

Upvote, I need this feature.

@RomeroMsk
Copy link
Contributor Author

I'm using this solution for now:

  1. Extended yii\rest\Serializer to add fieldsConf property and change serializeModel() and serializeModels() methods to "intersect" attributes and extra fields from fieldsConf and request params. Also I added formatting functionality to change the format of filed in Serializer. It is useful, for example, to show datetime values in special format (ISO 8601 etc).
  2. In controller I'm configuring the Serializer by setting appropriate fieldsConf (it depends on API user in my case).

So, my changes to framework code are minimal: only in yii\rest\Serializer. I can share the code or make a PR if someone is interesting in it.

@creocoder
Copy link
Contributor

There is much more better solution called transformers approach used in every modern REST api implementation on php. It allows not only reduce fields for different actions. It allows differently represent one model to several resources.

P.S. Working on it. PR will be little later.

@fernandezekiel
Copy link
Contributor

@creocoder look forward on that one,
currently Yii's fields() implementation is a per-model instance thing, i take this one is gonna be different?

@blacksesion
Copy link

this is my solution for this problems, using Yii::$app()

    public function fields()
    {
        if(Yii::$app->controller->action->uniqueId == 'controller/action'){
            return ['field_1','field_2','field_3','field_4'];
        }else{
            return ['field_1','field_3'];
        }
    }

hope works for you 👯

@mahendran-sakkarai
Copy link

Thanks @blacksesion. It helps me.

@MarcosNavarroS
Copy link

This is my solution:

app\components\Serializer

<?php

namespace app\components;


class Serializer extends \yii\rest\Serializer {

    public $defaultFields;
    public $defaultExpand;

    public function init() {
        parent::init();
        $this->defaultFields = !is_null($this->defaultFields) ? implode(",", $this->defaultFields) : $this->defaultFields;
        $this->defaultExpand = !is_null($this->defaultExpand) ? implode(",", $this->defaultExpand) : $this->defaultExpand;
    }

    protected function getRequestedFields() {
        $fields = is_null($this->request->get($this->fieldsParam)) ? $this->defaultFields : $this->request->get($this->fieldsParam);
        $expand = is_null($this->request->get($this->expandParam)) ? $this->defaultExpand : $this->request->get($this->expandParam);

        return [
            preg_split('/\s*,\s*/', $fields, -1, PREG_SPLIT_NO_EMPTY),
            preg_split('/\s*,\s*/', $expand, -1, PREG_SPLIT_NO_EMPTY),
        ];
    }

}

app\modules\v1\controllers\CustomerController

<?php

namespace app\modules\v1\controllers;

use Yii;
use yii\data\ActiveDataProvider;
use use yii\rest\Controller;;
use app\modules\v1\models\RestCustomer;

class CustomerController extends Controller {

    public $serializer = [
        'class' => 'app\components\Serializer',
        'defaultFields' => ['cust_id', 'cust_first_name', 'cust_last_name', 'cust_email']
    ];

    public function actionIndex() {
        return new ActiveDataProvider([
            'query' => RestCustomer::find(),
        ]);
    }

}

OR

<?php

namespace app\modules\v1\controllers;

use Yii;
use yii\data\ActiveDataProvider;
use use yii\rest\Controller;;
use app\modules\v1\models\RestCustomer;

class CustomerController extends Controller {

    public $serializer = ['class' => 'app\components\Serializer'];

    public function actionIndex() {
        $this->serializer['defaultFields'] = ['cust_id', 'cust_first_name', 'cust_last_name', 'cust_email'];
        return new ActiveDataProvider([
            'query' => RestCustomer::find(),
        ]);
    }

    public function actionXXXXXXX() {
        $this->serializer['defaultFields'] = ['cust_first_name', 'cust_email'];
        return new ActiveDataProvider([
            'query' => RestCustomer::find(),
        ]);
    }

}

@pgyf
Copy link

pgyf commented Aug 15, 2017

    public function actionXXXXXXX() {
        $_GET['fields'] = 'username,full_name';
        return new ActiveDataProvider([
            'query' => RestCustomer::find(),
        ]);
    }

@samdark samdark removed this from the 2.0.x milestone Dec 18, 2017
@ryakoviv
Copy link

ryakoviv commented Jan 5, 2019

You've forgotten about events

public function actionPublic()
{
    \yii\base\Event::on(Thing::class, Thing::EVENT_AFTER_FIND, function ($event) {
        $event->sender->scenario = Thing::SCENARIO_SEARCH_PUBLIC;
    });
    return new ActiveDataProvider([
        'query' => Thing::find(),
    ]);
}


public function actionPrivate()
{
    \yii\base\Event::on(Thing::class, Thing::EVENT_AFTER_FIND, function ($event) {
        $event->sender->scenario = Thing::SCENARIO_SEARCH_PRIVATE;
    });
    return new ActiveDataProvider([
        'query' => Thing::find(),
    ]);
}

and inside of ActiveRecord (Thing in my case) check the scenario in fields() method

 public function fields()
 {
    $fields = parent::fields();

    if ($this->scenario === self::SCENARIO_SEARCH_PUBLIC) {
        unset($fields['field1'], $fields['field2'], $fields['field3'], $fields['field4']);
    }

    return $fields;
  }

Check my answer in stackoverflow

@samdark samdark closed this as completed Jan 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests