Bug: For some reason exported dump of bootstrap file is not working by default

tranxton commented 3 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

Problem Description

I am encountering three errors when attempting to connect a bootstrap file in the Unleash PHP SDK. These errors occur during the import process of the bootstrap file, which was exported from the Unleash admin panel.

Note: The bootstrap file itself is valid, as confirmed by re-importing it into the Unleash admin panel without issues.

Errors Encountered

Warning: Undefined array key "constraints"
Location: vendor/unleash/client/src/Repository/DefaultUnleashRepository.php:438
Adding a check using the Null coalescing operator (??) resolves this error, but leads to another issue.

Code Example:
```
foreach ($segmentsRaw as $segmentRaw) {
   $result[$segmentRaw['id']] = new DefaultSegment(
       $segmentRaw['id'],
       $this->parseConstraints($segmentRaw['constraints'] ?? []),
   );
}
```

Warning: Undefined array key "strategies"
Location: vendor/unleash/client/src/Repository/DefaultUnleashRepository.php:184
Adding a check for the key's existence fixes this issue but introduces a third error.

Code Example:

foreach ($feature['strategies'] ?? [] as $strategy) {
   $constraints = $this->parseConstraints($strategy['constraints'] ?? []);
   $strategyVariants = $this->parseVariants($strategy['variants'] ?? []);

   $hasNonexistentSegments = false;
   $segments = [];

Warning: Undefined array key "enabled"
Location: vendor/unleash/client/src/Repository/DefaultUnleashRepository.php:213
Adding a Null coalescing operator here resolves the issue.

Code Example:
```
$featureDto = new DefaultFeature(
   $feature['name'],
   $feature['enabled'] ?? false,
   $strategies,
   $featureVariants,
   $feature['impressionData'] ?? false,
   $dependencies,
);
```

Conclusion

After addressing all three errors by adding checks for the existence of the respective keys, I am able to successfully connect the bootstrap file to the Unleash PHP SDK.

Question:
Am I approaching this problem correctly? If my analysis is accurate, I can proceed to create a Pull Request (PR) with a proposed solution.

Environment Details:

PHP: 8.3
Symfony: 7.1
Unleash: 6
unleash/client: 2.5.283
unleash/symfony-client-bundle: 0.11.382
Bootstrap File: Link to Bootstrap File

To reproduce

Download the build from this repository.
Follow the initial setup steps outlined in the README.md file to configure the project.
Navigate to config/packages/unleash_client/unleash_client.yaml:21.
Replace fetching_enabled: true with fetching_enabled: false on line 12.
Replace bootstrap: null with bootstrap: file://%kernel.project_dir%/config/packages/unleash_client/bootstrap.json on line 21.
Open localhost:8080/feature-toggle/default in your browser to view the error.

Sample code (optional)

No response

Version

2.5.283

Expected behavior

After replacing the bootstrap configuration with the path to the bootstrap file, the application should load the feature toggles without any errors. The feature toggles should be correctly initialized based on the content of the bootstrap.json file, and the page at localhost:8080/feature-toggle/default should display the expected feature toggle states without triggering any warnings or errors in the PHP logs.

Logs (optional)

[2024-08-14T08:20:19.812288+00:00] request.CRITICAL: Uncaught PHP Exception ErrorException: "Warning: Undefined array key "constraints"" at DefaultUnleashRepository.php line 438 {"exception":"[object] (ErrorException(code: 0): Warning: Undefined array key \"constraints\" at /app/vendor/unleash/client/src/Repository/DefaultUnleashRepository.php:438)"} [] [2024-08-14T08:20:32.509411+00:00] request.CRITICAL: Uncaught PHP Exception ErrorException: "Warning: Undefined array key "strategies"" at DefaultUnleashRepository.php line 184 {"exception":"[object] (ErrorException(code: 0): Warning: Undefined array key \"strategies\" at /app/vendor/unleash/client/src/Repository/DefaultUnleashRepository.php:184)"} [] [2024-08-14T08:22:36.615117+00:00] request.CRITICAL: Uncaught PHP Exception ErrorException: "Warning: Undefined array key "enabled"" at DefaultUnleashRepository.php line 213 {"exception":"[object] (ErrorException(code: 0): Warning: Undefined array key \"enabled\" at /app/vendor/unleash/client/src/Repository/DefaultUnleashRepository.php:213)"} []

Additional context (optional)

No response

RikudouSage commented 3 months ago

That export doesn't look like the format that's expected for a bootstrap. In the tests you can see an example bootstrap: https://github.com/Unleash/unleash-client-php/blob/main/tests/data/bootstrap-file.json

It's fairly minimal, but notice that unlike in the feature export you did, the strategies are part of the feature itself.

Though if you want to provide a helper that can take the exported json and convert it to the expected format (something like BootstrapHelper::fromUnleashExport(array $exported): array), I'll be happy to merge.

tranxton commented 3 months ago

Thank you for your quick response.

I would be happy to assist in adding a converter for the exported JSON to the expected format.

sighphyre commented 3 months ago

Hey @tranxton, I think there's a mix up here. The file you've provided is an export of the features for the admin UI. That isn't intended to be used as a bootstrap file for any of the SDKs, that's purely for a UI representation of the features and its definitely missing some important information for the SDK to do its job.

If you want a valid bootstrap, you should call the endpoint at /api/client/features and use the result from that rather

tranxton commented 2 months ago

Apologies for the delay.

I’ve been quite busy and haven’t had the time to prepare the PR yet. I will send it in the next few days

sighphyre commented 2 months ago

Apologies for the delay. I’ve been quite busy and haven’t had the time to prepare the PR yet. I will send it in the next few days

Hey @tranxton, this doesn't need a fix, it isn't a bug, you just need to use the data from the correct endpoint. A conversion won't work here, the endpoint you're using doesn't hold all the information that's needed

tranxton commented 2 months ago

Alright, I understand you.

How do you feel about automating this process: getting rid of the need to manually add the bootstrap file, for example, at some regular interval or based on TTL?

Since PHP doesn’t have the ability to store global state by default, we could take the following approaches:

Utilize the existing caching mechanism, which wouldn’t require a radical overhaul of the SDK or rewriting the integration.
Use APCu, but this adds a dependency on the extension.
Store the state on disk, which would impact request performance, and the method itself isn’t quite applicable.
Add a helper that will create the bootstrap file based on the response from the Unleash server, and users will be able to independently create background tasks to update the bootstrap file.

I’m ready to help with implementation :)

tranxton commented 2 months ago

What do you think, @sighphyre?

RikudouSage commented 2 months ago

I personally don't have an opinion on how to do it, but what I'm sure about is that we definitely don't want to do 2. or 3. Locking in to APCu is a no-go for me (if you're so inclined, you can write a cache adapter that uses APCu) and storing on disk is a no-go as well, there are many use-cases where you can't write to disk outside /tmp (like AWS Lambda) and storing cache in /tmp is not the best idea.

If we're to do it, we definitely want to use a cache abstraction (like we currently do for all other parts).

Anyway, what you're proposing sounds to me more like a stale cache, which is already present. If you don't care how outdated it is, just set it to a really large number (like 100 years or so) and as long as you fetch the toggles at least once, you'll always have at least some fallback.

Bootstrap is for cases where you don't have even the stale cache populated. But if you wish to provide the stale cache as a bootstrap, you can do that already (though it's not very straightforward, but we could add some events to hook into to update the cache).

tranxton commented 2 months ago

I'm not suggesting replacing the bootstrap file with a cache, since in the first case, it serves as a mechanism for graceful degradation, while in the second case, we are optimizing query speed and reducing the load on the Unleash server.

The idea is to automate the updating (or refreshing) of the bootstrap file with minimal development effort.

In the first option, the SDK updates the bootstrap file automatically at regular intervals or based on a TTL, which can be stored in the cache.

In the second option, the SDK simply provides a mechanism to download the current state from the Unleash server and generate the bootstrap file based on it, leaving it to the users to decide when and where they want to update the bootstrap file.

Unleash / unleash-client-php