Inserting features creates catalogs

[qgau]

Hello,

This line in FeaturesFunctions.php store/create catalogs when a user insert a feature in an existing collections.

It has at least four consequences that could be undesirable:

1. It creates confusion as developer when understanding the API: why creating a give resource type (feature) creates another one (catalog) that looks very similar to yet another existing one (collection)
2. It creates confusion as user because now from the root we have:

• /collections/xxxx
• /catalogs/collections/xxxx
  while, for a given "xxxx" the two contents seem the same

3. It requests people that can can insert feature to have the rights to create catalogs, which in not necessary desired for now, as at the moment, newly created catalogs by a lambda user ends at catalog root (/catalogs), and hence has a major impacts on all users UX.
4. Enabling some options, such as catalogMinMatch, impacts /catalogs/collections but not /collections.

Could you highlight why are these catalogs created on-the-fly needed?
Could it makes sense to keep these hidden to the users?
If they are really needed, then is there any sense to have different rights for insertfeatures and createcatalogs? As the second require necessarily the first in case collections are managed by others users than the ones insterting features.

[jjrom]

During feature ingestion process, resto computes automatic classifications based on the feature properties. For instance, it computes geographical classification on the feature location (i.e. "Europe", "France", "Haute-Garonne", etc.).

All these classifications are provided as catalogs and thus this line is here to reflect this behavior. Since these catalogs are "internals" catalogs, there is no ownership check when they are created. So this is transparent to the user and so you point 3 is not true i.e. it does not require user to have the rights to create catalog.

The point 2 is an issue - the /catalogs/collections should returns a 404
The point 4 is an issue (see #430)

Concerning point 1. There is however, a way for a user to create catalog during feature ingestion by setting a "rest:catalogs" property within the feature to be inserted. This is a very useful behavior that should be kept in resto. See the code here (

resto/app/resto/core/addons/Cataloger.php

Lines 438 to 479 in e45793e

	/**
	* Return a catalogs array from feature properties
	*
	* @param array $properties
	* @param RestoCollection $collection
	*/
	private function catalogsFromProperties($properties, $collection)
	{
	/*
	* [IMPORTANT] If input properties contains a resto:catalogs property then use it
	* [SECURITY] The isExternal is automatically set to true for these catalogs to
	* avoid non authorized user to create a catalog if he doesn't have rights to do so
	*/
	$catalogs = $properties['resto:catalogs'] ?? array();
	for ($i = count($catalogs); $i--;) {
	$exploded = explode('/', $catalogs[$i]['id']);
	$catalogs[$i]['isExternal'] = true;
	$catalogs[$i]['rtype'] = 'catalog';
	$catalogs[$i]['hashtag'] = 'catalog' . RestoConstants::TAG_SEPARATOR . array_pop($exploded);
	}
	/*
	* Roll over facet categories
	*/
	$model = isset($collection) ? $collection->model : new DefaultModel();
	foreach (array_values($model->facetCategories) as $facetCategory) {
	$catalogs = array_merge($catalogs, $this->catalogsFromFacetCategory($properties, $facetCategory, $model));
	}
	/*
	* Compute hashtags catalogs from description
	*/
	$hashtags = $this->catalogsFromText($properties['description'] ?? '');
	if ( !empty($hashtags) ) {
	$catalogs = array_merge($catalogs, $hashtags);
	}
	/*
	* Finnaly date catalogs
	*/
	return array_merge($catalogs, $this->getDateCatalogs($properties, $model));
	}

)

Since this is not a documented behavior, I don't see any issue here. The probability that a user set a "resto:catalogs" property in input feature is quite low unless the user understand it in the code and thus do it on purpose (which means that this functionality is useful to him ;) )

[qgau]

Hi!

Thank for the answer, it is clearer to me now.

About point 3, you're probably right, this is not something that I experienced, but assumed because of this line. But I didn't test it.

So point 2 is a bug, I will try to fix it, but I wont' be able to during the next three weeks, so don't be suprised if you have no news.