EntityAutocomplete should pass the original URI to the selection handler

And the fix.

Obvious. Now all tests involving entity reference require the keyvalue.expirable service in KernelTestBase container... and it's not there. I added the parameter in KernelTestBase but the table schema is only created on demand. Also I temporary stole #2623568: Config schema of argument_default plugins is incorrect just to see this passing.

And for 8.1.x.

The "test only" patch failed, the other is green. So, back to NR.

@amateescu, that was my first impulse. But on normal site, the actual implementation, will produce a reasonable variance in \Drupal::keyValue('entity_autocomplete'). How many specific autocomplete configurations you might have on a site? Dozens is already too much. But if we put there also the URI you can end up with hundreds or even thousands of items in \Drupal::keyValue('entity_autocomplete') (or even more). This is because we cannot predict on how many URIs with the same autocomplete will be displayed. Just imagine a page with an autocomplete where the URI has a node id as query parameter (e.g. /node/add/page?catalog={node}. For each {node} you'll have a new entry.

But adding the URI to \Drupal::service('keyvalue.expirable')->get('entity_autocomplete') with a 24 hrs expiry time can work as the garbage collection will keep a reasonable number of entries there. Of course the TTL we have to discuss and it's now an arbitrary value.

@amateescu, alternative solution:

1. We pack everything (selection settings + entire original symfony Request object) in a single array.
2. Build a hash on that package
3. Store the package keyed with the hash in session.
4. Pass the hash in the URL (as we do now)
5. Retrieve on the other side, in EntityAutocompleteController::handleAutocomplete(), the whole package
6. Clear the session entry.

What about that?

EDIT: Ignore the whole comment, this doesn't work.

This is another approach that doesn't require Expirable Key/Value. The URI is sent with a POST, instead of GET, in autocomplete.js. If it's reasonable we can expand and send also all the selection settings through the same post an drop completely the \Drupal::keyValue('entity_autocomplete') storage. What about that?

@dawehner, that was my first thought. But now I think I can extend the POST by adding also the $selection_settings to be posted. In this way we can get rid of the 'entity_autocomplete' Key/Value storage. That key/value stuff seem to me a little bit weird because has no mechanism for garbage collection. For example when the settings of a field are changed, a dead entry will still remain in the storage. And conceptually, I don't think that the key/value is for passing data between requests. But GET or POST are exactly for passing data across the requests.

Well the problem is that GET requests are cacheable, while POST requests arent'

@dawehner, that counts!

In this case what is your opinion, should I:

1. Go with #16 but switch to GET
2. Go with #16, switch to GET and, additionally, pass also serialised (PHP, Json?) $selection_settings as query param?

Changed back to use GET request. Now the original URI is passed as query parameter.

@amateescu, why cluttering key/value storage, even the expirable one, when we can pass it along with a very light GET request?

Credits.

Small improvements and doc fixes.

@amateescu,

Because we're trading cluttering the key/value storage for cluttering cache-able GET requests :)

You're right. But personally I like more to store them in the cache for 2 reasons: it has better invalidation mechanism and it benefits for faster storages (Redis, Memcache, etc)

@dawehner,

Can we somehow make that an opt in behaviour for some form elements?

Sure we can. But that means we need to add also an UI option in entity reference autocomplete widget so that we can take the benefit. There is the first place where this would be used.

Another idea I just had, we could store the URI in session, if needed.

I had also this idea in #15 but the values should live in the session till the end. The session data can increase dramatically for a single user.

How about storing a flag "pass_original_uri" in selection handler settings? Then we do the whole logic only if a specific selection handler will require that. ViewsSelection will require the original uri while DefaultSelection not.

EDIT: At least this narrows the cases to the selection handlers that requires it. But adding also the opt-in could narrow more.

If we let selection handlers to declare if they want to get the original URI (idea from #29) then we need a base class for all selection handlers in order to assure BC compatibility. I opened #2787873: Add a base class for entity reference selection handlers and fix the structure of their configuration also to deal with #2786841: Entity reference fields should add selection handler config dependencies. That will provide the foundation to add a method such as needsOriginalUri() to all selection handlers. Then we implement that method in the base class returning FALSE and we are covered as BC.