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

[5.x] Provide search index name callback #10435

Open
wants to merge 6 commits into
base: 5.x
Choose a base branch
from

Conversation

ajnsn
Copy link
Contributor

@ajnsn ajnsn commented Jul 11, 2024

Currently Statamic always generates search index names like this $locale ? $name.'_'.$locale : $name.

When using different environments (staging, production) and a cloud search provider (e.g. Algolia) index names tpyically collide. Staging updates overwrite those from production and vice versa.

At least in the lower pricing plans Algolia has a limit of 1 app per account and per app isolation is also often a configuration overhead. So an environment prefix for the search index name would be handy?

Afaik there a two approaches to get this solved in user land:

  1. With a dirty env access within search.php
'indexes' => [
    env('APP_ENV') . '_pages' => [
        'driver' => 'algolia',
        'searchables' => ['collection:pages'],
        'fields' => ['title'],
        'sites' => 'all',
    ],
]
  1. By overriding the default index within a ServiceProvider's boot method:
use Algolia\AlgoliaSearch\SearchClient;
use App\Statamic\Search\Algolia\EnvironmentIndex;
use Illuminate\Foundation\Application;
use Statamic\Facades\Search;

Search::extend(
    driver: 'algolia',
    callback: fn (Application $app, array $config, string $name, string $locale): EnvironmentIndex => new EnvironmentIndex(
        client: SearchClient::create(
            appId: $config['credentials']['id'],
            apiKey: $config['credentials']['secret']
        ),
        name: $name,
        config: $config,
        locale: $locale
    )
);

//

namespace App\Statamic\Search\Algolia;

use Algolia\AlgoliaSearch\SearchClient;
use Statamic\Search\Algolia\Index as BaseIndex;

class EnvironmentIndex extends BaseIndex
{
    public function __construct(SearchClient $client, string $name, array $config, string $locale)
    {
        parent::__construct(
            client: $client,
            name: app()->environment() . '_' . $name,
            config: $config,
            locale: $locale
        );
    }
}

With this PR it would possible to override the generating of index names by setting a callback within a ServiceProvider's boot method e.g. like this:

use Statamic\Search\Algolia\Index as AlgoliaIndex;

AlgoliaIndex::resolveNameUsing(fn (string $name): string => app()->environment() . '_' . $name);

No breaking chances, default name generation is preserved.

Any chance that this is getting merged? :)
Thanks!

Closes statamic/ideas#1202

src/Search/Index.php Outdated Show resolved Hide resolved
@jasonvarga
Copy link
Member

@ajnsn Would #10536 work for you?

@ajnsn
Copy link
Contributor Author

ajnsn commented Jul 31, 2024

Ah yes, sure. I am not that config-file fan any more but as Statamic has a lot of config files it would fit too. Also keep in mind that there are other search providers as well. Narrowing down this to Algolia could lead to other PR's :)

@jasonvarga
Copy link
Member

Also keep in mind that there are other search providers as well. Narrowing down this to Algolia could lead to other PR's

Good point!

@bastihilger
Copy link
Contributor

I went the config file road in the other PR because that's the way it is done in Laravel Scout. In the end it's more about having the opportunity to have the value in your env file. It doesn't have to be in the drivers.algolia array, but it would be great to have it in the config file, maybe at a higher level, so we can use the same value from the env file.

@aaronbushnell
Copy link
Contributor

I'm sure some folks are tied up with Laracon/traveling, but happy to help support this in any way!

We're about to work on a project that needs this type of capability. Any documentation necessary here? I can write that up if so if it'll help get this merged sooner!

@edalzell
Copy link
Contributor

I'm sure some folks are tied up with Laracon/traveling, but happy to help support this in any way!

We're about to work on a project that needs this type of capability. Any documentation necessary here? I can write that up if so if it'll help get this merged sooner!

You can always composer patch it in.

@aaronbushnell
Copy link
Contributor

Good idea, @edalzell! In the meantime, if the core team could use an extra pair of hands I'm happy to jump in!

@ajnsn
Copy link
Contributor Author

ajnsn commented Sep 12, 2024

Hi @jasonvarga. I added a test for the search index name callback. This somehow took longer as expected as the getIndex() approach seems to be unused or broken? But this could be the right place for this type of test and maybe provides a basis for further search index testing?

Please let me know if this works for you and if there is anything missing. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option for Algolia search index prefix
6 participants