[Autocomplete] Allow passing extra options to the autocomplete fields

[jakubtobiasz]

Q	A
Bug fix?	no
New feature?	yes
Issues	n/a
License	MIT

This PR still needs some tweaks and tests, but I want to validate my idea. Currently, if we pass anything as a third argument in the ->add() method, it is only used on the form render, and is fully ignored during the AJAX call.

While implementing UX's Autocomplete in Sylius, I wanted to complete the following user story

Given I want to edit a taxon
When I want to assign a parent taxon to it
And I check a list of available taxa
Then I should see all taxa except the edited one

So basically, I have to pass a current taxon's ID to the autocomplete's query. As we know, currently it's not possible. So after contacting @weaverryan, I decided to implement a mechanism similar to the one from Live Components.

When you pass an array of options as a 3rd argument, you can use a special extra_options key, which is an array consisting scalars/arrays/nulls.

Next, when the form is rendered, I get these values, calculate a checksum for them, then pass them through json_encode and base64_encode functions. In the end, I glue them to the url values in the \Symfony\UX\Autocomplete\Form\AutocompleteChoiceTypeExtension::finishView method.

So, basically with the following configuration:

we end up with the following HTML code:

I decided to "glue" the extra_options to the URL, as I didn't have to deal with JS. Of course, I do not exclude a chance to refactor it, as a whole method should be refactored anyway.

Finally, the controller decodes the data, checks the checksum and passes the values to the autocomplete's form.

[alexander-schranz]

As I'm currently facing a similar issue in recreating some more complex Selection Type which is doing similar things like autocomplete. I wanted to share it here also here. https://gist.github.com/alexander-schranz/b1cb8843bc54dc368001375fe6998e61#file-mycontactchoicetype-php-L39 In my case I did also go with providing a list of options, so the loading part will have possibility todo such things. But instead of duplicating the options I just provide a list of which options should be forwarded (string[], instead of array<string, mixed>`. So I don't have to duplicate the value of the option inside the extra_options, item_loader_options in my case.

[alexander-schranz]

I try currently some magic to find out automatically my extra_options without have the need to define them in my custom Selection Type wanted to share it here, not sure if its too much magic:

    public function buildView(FormView $view, FormInterface $form, array $options): void
    {
        $extraOptions = [];
        $formType = $form->getConfig()->getType();

        $innerType = $formType->getInnerType();
        $defaultOptionsResolver = new OptionsResolver();
        $innerType->configureOptions($defaultOptionsResolver);
        $definedOptions = $defaultOptionsResolver->getDefinedOptions();
        foreach ($defaultOptionsResolver->getMissingOptions() as $missingOption) {
            $defaultOptionsResolver->remove($missingOption);
        }
        // Form, ChoiceType options which we keep always in mind and are important for form widget rendering e.g. required, disabled, ...
        foreach ($options['extra_options'] as $extra_option) {
            $definedOptions[] = $extra_option;
        }

        $defaultOptions = $defaultOptionsResolver->resolve([]);
        foreach ($options as $key => $value) {
            if (!\is_scalar($value)) {
                // ignore all options which are non-scalar values
                continue;
            }

            if (!\in_array($key, $definedOptions)) {
                // we ignore all options which were not defined specifically for the inner type
                continue;
            }

            if (isset($defaultOptions[$key]) && $defaultOptions[$key] === $value) {
                // we ignore all options which have the same value as the default value
                continue;
            }

            $extraOptions[$key] = $value;
        }

        $view->vars['extra_options'] = $extraOptions;
    }
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'extra_options' => ['required', 'disabled'],
        ]);
    }

---

What could also be used to avoid a seperate extra_options is the $resolver->getRequiredOptions(); used as extra_options, and so via $resolver->setRequired('some_value'); can be defined what is required:

    public function buildView(FormView $view, FormInterface $form, array $options): void
    {
        $extraOptions = [];
        $formType = $form->getConfig()->getType();

        $innerType = $formType->getInnerType();
        $defaultOptionsResolver = new OptionsResolver();
        $requiredOptions = $defaultOptionsResolver->getRequiredOptions();
        foreach ($defaultOptionsResolver->getMissingOptions() as $missingOption) {
            $defaultOptionsResolver->remove($missingOption);
        }

        $defaultOptions = $defaultOptionsResolver->resolve([]);
        foreach ($options as $key => $value) {
            if (!\is_scalar($value)) {
                // ignore all options which are non-scalar values
                continue;
            }

            if (!\in_array($key, $requiredOptions)) {
                // we ignore all options which are not required
                continue;
            }

            if (isset($defaultOptions[$key]) && $defaultOptions[$key] === $value) {
                // we ignore all options which have the same value as the default value
                continue;
            }

            $extraOptions[$key] = $value;
        }

        $view->vars['extra_options'] = $extraOptions;
    }

[alexander-schranz]

In my own selection type I did instead of using a checksum go with the UriSigner but ignore all query parameters which I don't know, so maybe we could do here the avoid own checksum calculator:

if (!$this->uriSigner->check(
    // ignore unrelated query parameters as they can be search filters which are allowed
    $request->getSchemeAndHttpHost() . $request->getPathInfo() . '?_config=' . \rawurlencode($request->query->get('_config')) . '&_hash=' . \rawurlencode($request->query->get('_hash')),
)) {
    throw new AccessDeniedHttpException('Invalid signature.');
}

/**
 * @var array{
 *     options: array<string, mixed>,
 *     name: string,
 *     theme: string,
 * }|false
 */
$config = \json_decode(\base64_decode($request->query->get('_config'), true), true, 512, \JSON_THROW_ON_ERROR);

[smnandre]

Do you want some help/feedback/other on this ?

[jakubtobiasz]

Hi @smnandre,
I've refactored one of two places I marked as "needs to be refactored". I have to write some functional tests for it, and... basically it will be ready to be fully reviewed.

I'll let you know once ready to be reviewed :D. It should be ready today :D.

[smnandre]

Great! Good luck then :)

[jakubtobiasz]

I guess I haven't missed anything, and it's ready to be reviewed 👍🏻. AFAIR LiveComponent build failures are not related to my PR.

[stloyd]

Maybe use some lighter hash algo?

Suggested change

	return base64_encode(hash_hmac('sha256', json_encode($data), $this->secret, true));
	return base64_encode(hash_hmac('xxh128', json_encode($data), $this->secret, true));

Ref: https://php.watch/articles/php-hash-benchmark

[smnandre]

Would the base64 still be necessary ?

[jakubtobiasz]

Right, I guess the base64_encode is kind of a leftover before I started using hash_hmac (or I used it to get a shorter string) 🤔. We can remove it and increase the string length from 44 to 64.

In terms of the algorithm:

[stloyd]

The question here would be how we treat that data, for me it's checksum which is not a password but should be kinda of cryptographic, but it doesn't need to be perfect as a password and it doesn't need to be as safe as it is.

[jakubtobiasz]

Ah, you're right. I thought (idk why :D) that we encrypt data here, but the data is not encrypted at all.

So the answer here is simple, I used sha256, because that's exactly the same way as we calculate the checksum in the LiveComponent. I'm open to changing it, I just copied the code from the LiveComponent and didn't bother to think whether sha256 is the correct one here :D.

[jakubtobiasz]

Now I'm thinking about extracting checksum calculating inside the LiveComponent component, and using it in the autocomplete component. Autocomplete depends on the LiveComponent so it might make sense.

[smnandre]

That would make perfect sense to me.

Feel free to do it in a second/later PR if you want to release this one first... in all maner this should be "internal" so there is no BC involved there :)

[jakubtobiasz]

I was 100% sure LiveComponent is required by the Autocomplete, but it is not.

So, for now, I leave the Calculator where it lives. I can mark the class as internal but I'll leave the interface as it is, to allow people to create their own implementation of checksum calculator. If anyone finds they need a lighter version of hash algorithm, it'll make sense to create their own.

[weaverryan]

but I'll leave the interface as it is,

I'd prefer to remove the interface. I can't see a legit reason to need to write your own. If there is a problem with our class, then we want them to tell us so we can fix it :)

[jakubtobiasz]

@weaverryan I've removed it!

[smnandre]

Somewhere in the future documentation, should we add a small warning about the visibility of extra_options ? Something like "the given extra options can be retrieved and therefore should not contain sensible or personal information"

[smnandre]

I'd rather have separate Interfaces, what do you think ?

[jakubtobiasz]

I do like creating "subtype" interfaces like this, but it's not a thing I'd fight for :D. I'm open for changing it.

[smnandre]

No strong opinion on this.

i'm often more concerned about "what could happen later" than i probably should... i'm working on it but habits are hard to change :)

That's why i ask / raise questions but would never "require" changes :))

[weaverryan]

A few comments, but this is really great! Could you also bootstrap docs for this? Thanks!

[weaverryan]

but I'll leave the interface as it is,

I'd prefer to remove the interface. I can't see a legit reason to need to write your own. If there is a problem with our class, then we want them to tell us so we can fix it :)

[weaverryan]

What if we used Symfony's UriSigner instead of the checksum? If extra options exist, we sign the URL when generating it and passing it to the frontend. Here, we validate the signed URL.

[weaverryan]

Friendly ping :) - I'd love to get this across the finish line. Thanks!

[jakubtobiasz]

Hi @weaverryan,
I'll try to get back to it this weekend 😅.

[smnandre]

Suggested change

	/** @internal */
	/**
	* @author .... ?
	*
	* @internal
	*/

(same thing on OptionsAware..Interface ?)

[smnandre]

Compared to symfony/form changelog, i'd write it maybe more like

• Add extra_options to XXX and YYY types (those options are .... to ...)

[weaverryan]

Thanks for your hard work on this Jacob!

[arlenreyb]

Not sure if this is bad etiquette, but if anyone came to this thread looking for how to use this new feature, an example of how to use it is in this thread, it's just hidden by GitHub. It's right here.

[weaverryan]

Oh yea, it looks like we missed creating a docs PR. @jakubtobiasz are you able to create a quick docs PR for this feature?