Suggestion for improvement regarding entity/collection handling in next major release

[Wilt]

I noticed this recent change #96 where specific filter configuration for collections is introduced:
I missed this change, otherwise I would have commented sooner, but wouldn't it have been nicer to make the configuration array like so:

'zf-content-validation' => [
    'Application\Controller\HelloWorld' => [
        'input_filter' => 'Application\Controller\HelloWorld\Validator',
        'collection' => [
            'PUT' => 'Application\Controller\HelloWorld\UpdateValidator',
        ],
        'entity' => [
            'POST' => 'Application\Controller\HelloWorld\CreateValidator',
        ]
    ],
],

Like that the default input filter could be set for all requests (if available) and then for each case either entity or collection it can be collected and overwritten.
It would be more readable IMO instead of introducing all these additional METHOD_COLLECTION keys in the configuration.

I understand this is now too late, but might be worth reconsidering reorganizing keys in Apigilty next major release 2.0. I noticed more collection and entity specific keys are being added to the application while they do the same thing:

METHOD/METHOD_COLLECTION
collection_http_methods/entity_http_methods
collection_class/entity_class

some are specific for collection only
collection_name, collection_query_whitelist

They could also be nested in a key entity, or collection and then there could an interface for the common/shared elements but specific class instance is used depending on whether we are handling a entity or collection request.

This entity/collection key organization would be in line with the configuration of for example MvcAuth:

'authorization' => [
    'Controller\Service\Name' => [
        'actions' => [
            'action' => [
                'default' => boolean,
                'GET' => boolean,
                'POST' => boolean,
                // etc.
            ],
        ],
        'collection' => [
            'default' => boolean,
            'GET' => boolean,
            'POST' => boolean,
            // etc.
        ],
        'entity' => [
            'default' => boolean,
            'GET' => boolean,
            'POST' => boolean,
            // etc.
        ],
    ],
],

Then even the metadata_map could be reorganized the same way. Now the difference is made using a 'is_collection' key set to true/false, but even there there the entries could be grouped by collection or entity:

'metadata_map' => [
    'entity' => [
    ],
    'collection' => [
    ]
]

The MetadataMap has those organized in two groups, metadata for entities and collections.
Those could even be stored in two different Metadata classes, one for collections and one for entities.

Those metadata objects for collections and entities are not the same, only a part of their interface is common. For example EntityMetadata holds entity specific properties and the hydrators for the entity,
CollectionMetadata holds the collection specific data

This differentiation between work done separately for collections and entities currently works all the way down into the HalJsonRenderer logic:

    public function render($nameOrModel, $values = null)
    {
        if (! $nameOrModel instanceof HalJsonModel) {
            return parent::render($nameOrModel, $values);
        }

        if ($nameOrModel->isEntity()) {
            $helper  = $this->helpers->get('Hal');
            $payload = $helper->renderEntity($nameOrModel->getPayload());
            return parent::render($payload);
        }

        if ($nameOrModel->isCollection()) {
            $helper  = $this->helpers->get('Hal');
            $payload = $helper->renderCollection($nameOrModel->getPayload());

            if ($payload instanceof ApiProblem) {
                return $this->renderApiProblem($payload);
            }
            return parent::render($payload);
        }
    }

Should it not be:

public function render($nameOrModel, $values = null)
{
    if (! $nameOrModel instanceof HalJsonModel) {
        return parent::render($nameOrModel, $values);
    }
    $helper  = $this->helpers->get('Hal');
    $payload = $helper->render($nameOrModel);
    return parent::render($payload);
}

Getting the payload can be handled inside the render method.
Or am I maybe missing something here?

Please don't see this as criticism, I love the work everyone does in the zf-campus repositories. Merely see this as suggestion for possible future improvement. I am not a great software architect myself, but just wanted to share my thoughts and hope it will lead to some food for thought and/or discussion.

[weierophinney]

This repository has been closed and moved to laminas-api-tools/api-tools-content-validation; a new issue has been opened at laminas-api-tools/api-tools-content-validation#3.