refact: Isolate FeatureSnapshot and PropertySnapshot from crate::models types
#56
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Javier G. Sogo <[email protected]>
jgsogo
commented
Jan 20, 2025
| } | ||
|
|
||
| impl FeatureSnapshot { | ||
| pub(crate) fn new( | ||
| feature: crate::models::Feature, | ||
| segments: HashMap<String, crate::models::Segment>, | ||
| ) -> Self { | ||
| Self { feature, segments } | ||
| let segment_rules = SegmentRules::new(segments, feature.segment_rules, feature.kind); |
There was a problem hiding this comment.
Probable in a following PR the new method should just take the members and really remove all the references to crate::models from here.
rainerschoe
approved these changes
Feb 4, 2025
Merged
jgsogo
added a commit
that referenced
this pull request
Feb 12, 2025
Following #56, this PR moves the `FeatureSnapshot` and `PropertySnapshot` inside `Configuration` (closer to JSON data), this way the json-models doesn't leak into the application. --------- Signed-off-by: Javier G. Sogo <[email protected]>
rainerschoe
added a commit
that referenced
this pull request
May 30, 2025
Squashed commit of the following: commit 8f8cc2c Author: Rainer Schoenberger <[email protected]> Date: Fri May 30 16:21:38 2025 +0200 formatting commit b54ad25 Author: Rainer Schoenberger <[email protected]> Date: Fri May 30 16:20:43 2025 +0200 make the test pass: basic thread setup, nothign more commit 08b4bfa Author: Rainer Schoenberger <[email protected]> Date: Fri May 30 15:25:21 2025 +0200 error handling commit 53f920e Author: Rainer Schoenberger <[email protected]> Date: Fri May 30 14:36:18 2025 +0200 documentation + naming commit f3da41b Author: Rainer Schoenberger <[email protected]> Date: Fri May 30 14:18:12 2025 +0200 clean up warnings commit 2235b5c Author: Rainer Schoenberger <[email protected]> Date: Fri May 30 14:02:11 2025 +0200 provide default implementation for push_metering_data() for easier mocking commit d7752a3 Merge: fed9035 b443a3e Author: Rainer Schoenberger <[email protected]> Date: Fri May 30 13:51:19 2025 +0200 Merge remote-tracking branch 'github/main' into metering commit b443a3e Author: Javier G. Sogo <[email protected]> Date: Fri May 30 13:49:10 2025 +0200 chore: Renames associated to `SegmentRule` and `TargetingRule` (#80) * For the JSON fields (`appconfiguration::models`): the name in Rust matches the name in the JSON. There is no need to rename, the more they look like each other, the easier it will be to understand. We can (should) document them a little bit, but they will be _private_ (🤞 ), so it should be internal documentation only. * In the `segment_evaluation` module, I changed the names to `TargetingRule`, as suggested by @rainerschoe is previous comments. Better documentation is still pending. --------- Signed-off-by: Javier G. Sogo <[email protected]> commit 0cec63b Author: Javier G. Sogo <[email protected]> Date: Fri May 23 23:05:28 2025 +0200 feat: Introduce `ConfigurationProvider` trait (#74) I want to hide the concept of a `Configuration` as an implementation detail and instead introduce a trait (`ConfigurationProvider`) that offers the functionality to provide features and properties. This trait will then be implemented by many of the structs we already have: app-configuration clients, the `Configuration` itself,... and it will be easier to mock (/dependency injection). I also think this will help us to narrow the API of the crate: we can hide `Configuration` and some other structs together with it. --------- Signed-off-by: Javier G. Sogo <[email protected]> commit c417b8d Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Mon May 19 16:13:33 2025 +0200 chore(deps): update actions/setup-python action to v5 (#79) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [actions/setup-python](https://redirect.github.com/actions/setup-python) | action | major | `v3` -> `v5` | --- ### Release Notes <details> <summary>actions/setup-python (actions/setup-python)</summary> ### [`v5`](https://redirect.github.com/actions/setup-python/compare/v4...v5) [Compare Source](https://redirect.github.com/actions/setup-python/compare/v4...v5) ### [`v4`](https://redirect.github.com/actions/setup-python/compare/v3...v4) [Compare Source](https://redirect.github.com/actions/setup-python/compare/v3...v4) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/IBM/appconfiguration-rust-sdk). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC4xMS4xOCIsInVwZGF0ZWRJblZlciI6IjQwLjExLjE4IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> commit 2bb697b Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Mon May 19 13:55:57 2025 +0000 chore(deps): update actions/checkout action to v4 (#78) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [actions/checkout](https://redirect.github.com/actions/checkout) | action | major | `v3` -> `v4` | --- ### Release Notes <details> <summary>actions/checkout (actions/checkout)</summary> ### [`v4`](https://redirect.github.com/actions/checkout/blob/HEAD/CHANGELOG.md#v422) [Compare Source](https://redirect.github.com/actions/checkout/compare/v3...v4) - `url-helper.ts` now leverages well-known environment variables by [@​jww3](https://redirect.github.com/jww3) in [https://github.com/actions/checkout/pull/1941](https://redirect.github.com/actions/checkout/pull/1941) - Expand unit test coverage for `isGhes` by [@​jww3](https://redirect.github.com/jww3) in [https://github.com/actions/checkout/pull/1946](https://redirect.github.com/actions/checkout/pull/1946) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/IBM/appconfiguration-rust-sdk). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC4xMS4xOCIsInVwZGF0ZWRJblZlciI6IjQwLjExLjE4IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Javier G. Sogo <[email protected]> commit 04f216c Author: Javier G. Sogo <[email protected]> Date: Mon May 19 14:26:40 2025 +0200 fix: Iterate all rules in the targeting segment (#75) Iterate all rules in the targeting segment looking for the match. Only return `Ok(None)` at the end if there is no match --------- Signed-off-by: Javier G. Sogo <[email protected]> commit a20ef72 Author: Javier G. Sogo <[email protected]> Date: Mon May 19 11:25:46 2025 +0200 chore: Run pre-commit in PRs (all files) (#76) Create another github workflow to run pre-commit hooks in all PRs. * I'm running all pre-commits in all the files regardless of the files impacted in the PR. Our crate is small, we don't have too many files (we pay the price of cloning the repo one more time, though) * I'm using a different workflow because I only want pre-commits to run in pull-requests. We don't want to fail in `main` because of them. --------- Signed-off-by: Javier G. Sogo <[email protected]> commit fed9035 Author: Rainer Schoenberger <[email protected]> Date: Mon May 19 09:53:12 2025 +0200 compiles commit ff06b62 Author: Rainer Schoenberger <[email protected]> Date: Sun May 18 18:53:07 2025 +0200 still some way to go to get to compile commit ed14067 Author: Rainer Schoenberger <[email protected]> Date: Sun May 18 18:33:58 2025 +0200 initial metering sketch commit b07b945 Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Thu May 8 11:54:57 2025 +0200 chore(deps): update rust crate rstest to 0.25.0 (#72) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [rstest](https://redirect.github.com/la10736/rstest) | dev-dependencies | minor | `0.23.0` -> `0.25.0` | --- ### Release Notes <details> <summary>la10736/rstest (rstest)</summary> ### [`v0.25.0`](https://redirect.github.com/la10736/rstest/blob/HEAD/CHANGELOG.md#0250-202532) [Compare Source](https://redirect.github.com/la10736/rstest/compare/v0.24.0...v0.25.0) ##### Changed - Append generated test macro so next test macros are aware of it (see [#​291](https://redirect.github.com/la10736/rstest/pull/291) thanks to [@​kezhuw](https://redirect.github.com/kezhuw)). ##### Add - Added a `#[mode = ...]` attribute to be used with the `#[files(...)]` attribute to change the way the files get passed to the test. (see [#​295](https://redirect.github.com/la10736/rstest/issues/295) thanks to [@​lucascool12](https://redirect.github.com/lucascool12)) ### [`v0.24.0`](https://redirect.github.com/la10736/rstest/blob/HEAD/CHANGELOG.md#0240-202511) [Compare Source](https://redirect.github.com/la10736/rstest/compare/v0.23.0...v0.24.0) ##### Changed - MSRV to 1.70.0 (see [#​284](https://redirect.github.com/la10736/rstest/issues/284) thanks to [@​rnbguy](https://redirect.github.com/rnbguy)) ##### Add - `#![no_std]` support: now you can use `rstest` also in `no_std` lib (see [#​282](https://redirect.github.com/la10736/rstest/issues/282) thanks to [@​rnbguy](https://redirect.github.com/rnbguy)) - `#[context]` to have test function name and other useful thighs on the tip of your fingers (see [#​177](https://redirect.github.com/la10736/rstest/issues/177)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/IBM/appconfiguration-rust-sdk). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC43LjEiLCJ1cGRhdGVkSW5WZXIiOiI0MC43LjEiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> commit a9b38d7 Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Thu May 8 10:03:59 2025 +0200 chore: Configure Renovate (#71) Welcome to [Renovate](https://redirect.github.com/renovatebot/renovate)! This is an onboarding PR to help you understand and configure settings before regular Pull Requests begin. 🚦 To activate Renovate, merge this Pull Request. To disable Renovate, simply close this Pull Request unmerged. --- ### Detected Package Files * `Cargo.toml` (cargo) * `.github/workflows/main.yml` (github-actions) * `.github/workflows/release-plz.yml` (github-actions) ### Configuration Summary Based on the default config's presets, Renovate will: - Start dependency updates only once this onboarding PR is merged - Hopefully safe environment variables to allow users to configure. - Show all Merge Confidence badges for pull requests. - Enable Renovate Dependency Dashboard creation. - Use semantic commit type `fix` for dependencies and `chore` for all others if semantic commits are in use. - Ignore `node_modules`, `bower_components`, `vendor` and various test/tests (except for nuget) directories. - Group known monorepo packages together. - Use curated list of recommended non-monorepo package groupings. - Apply crowd-sourced package replacement rules. - Apply crowd-sourced workarounds for known problems with packages. 🔡 Do you want to change how Renovate upgrades your dependencies? Add your custom config to `renovate.json` in this branch. Renovate will update the Pull Request description the next time it runs. --- ### What to Expect With your current configuration, Renovate will create 1 Pull Request: <details> <summary>chore(deps): update rust crate rstest to 0.25.0</summary> - Schedule: ["at any time"] - Branch name: `renovate/rstest-0.x` - Merge into: `main` - Upgrade [rstest](https://redirect.github.com/la10736/rstest) to `0.25.0` </details> --- ❓ Got questions? Check out Renovate's [Docs](https://docs.renovatebot.com/), particularly the Getting Started section. If you need any further assistance then you can also [request help here](https://redirect.github.com/renovatebot/renovate/discussions). --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/IBM/appconfiguration-rust-sdk). <!--renovate-config-hash:e80b4e42a3043bc12fa0640db4bac392d2bf770acf841360d7c8ceeeac2ec1a9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> commit e808eaa Author: Javier G. Sogo <[email protected]> Date: Wed May 7 15:49:40 2025 +0200 feat: Implement offline mode (#68) The user can provide offline mode behavior to the http-based clients: `AppConfigurationClientHttp` and `AppConfigurationClientIBMCloud` to control the behavior of the client while it is not connected to the server: * `OfflineMode::Fail`: the client will return an error. * `OfflineMode::Cache`: the client will return the latest configuration available, if any. * `OfflineMode::FallbackData`: the client will use the provided fallback data while it's not connected to the server. closes #63 (implements the feature) closes #60 (PR is superseeded) --------- Signed-off-by: Javier G. Sogo <[email protected]> commit 33759b7 Merge: 36a6735 102e8b5 Author: Rainer Schoenberger <[email protected]> Date: Wed May 7 11:35:34 2025 +0200 feat: Provide a way to get the matching `Segment` after Feature/Property evaluation (#69) For Metering/Billing (#15 ) we need to collect and send (in batches) data for all evaluations of features/properties. In this PR I am preparing for this by making segment information available as a result of evaluation, as `segment_id` is one of the things we need to report for metering. commit 102e8b5 Merge: 3f2e8b0 36a6735 Author: Rainer Schoenberger <[email protected]> Date: Tue May 6 12:19:23 2025 +0200 Merge branch 'main' into metering commit 3f2e8b0 Author: Rainer Schoenberger <[email protected]> Date: Tue May 6 11:35:18 2025 +0200 better doc Signed-off-by: Rainer Schoenberger <[email protected]> commit 3cecc7b Author: Rainer Schoenberger <[email protected]> Date: Tue May 6 11:26:57 2025 +0200 comment Signed-off-by: Rainer Schoenberger <[email protected]> commit bd459d3 Author: Rainer Schoenberger <[email protected]> Date: Tue May 6 11:14:12 2025 +0200 make test work (some renames on the way to make function signatures match the function name) Signed-off-by: Rainer Schoenberger <[email protected]> commit 66dfbd1 Author: Rainer Schoenberger <[email protected]> Date: Tue May 6 11:01:38 2025 +0200 make test more robust Signed-off-by: Rainer Schoenberger <[email protected]> commit e746783 Author: Rainer Schoenberger <[email protected]> Date: Tue May 6 10:56:44 2025 +0200 test for communicating the segment back from segment eval Signed-off-by: Rainer Schoenberger <[email protected]> commit 36a6735 Author: Javier G. Sogo <[email protected]> Date: Mon May 5 11:11:57 2025 +0200 Move `Configuration` sync to `network::live_configuration` module, outside `AppConfigurationHttp` object (#66) Taken from #60 -- This PR basically extracts everything related to the refactor, and **leaves the offline modes implementation** for a future PR. * Moves the logic to keep the local `Configuration` updated from the `AppConfigurationHttpClient` client into a dedicated `network::live_configuration` module: * Core logic to retrieve the configuration goes to `UpdateThreadWorker` * Nice wrapper to handle offline modes goes to `LiveConfigurationImpl` (**note that offline mode feature is not yet in this PR) * `AppConfigurationIBMClient` is just a wrapper over `AppConfigurationHttpClient` using a _default` live configuration. * New `utils/thread_handle.rs` file with a nice utility to implement scoped-threads and be able to check its status * New functionality has high test coverage 🧪 --- Technical debt (added to #60 description): - Review `pub` visibility of `models.rs` (basically go through all the crate again) --> If we don't expose the `LiveConfiguration` it will be possible to make all of them _private_. - Review traits `PartialEq` and `Eq` added to `models.rs` - Are the tests in `app_configuration_client_http.rs` actually testing something? - Go through all the errors hierarchy --------- Signed-off-by: Javier G. Sogo <[email protected]> commit bf306e7 Author: Javier G. Sogo <[email protected]> Date: Tue Apr 29 16:41:35 2025 +0200 [refact] Use traits to narrow the objects that we are passing around (#65) Extracted from #60 Working on #63 We are introducing a couple of traits: `WebsocketReader` and `ServerClient` with to main purposes: * be explicit about the functionality required by the objects that we are passing around * make it easier to write tests when using mocks/dependency injection (see #60) --------- Signed-off-by: Javier G. Sogo <[email protected]> commit e0a2a12 Author: Javier G. Sogo <[email protected]> Date: Mon Apr 28 10:11:34 2025 +0200 [refact] Introduce NetworkResult and NetworkError types (#64) Extracted from #60 Working on #63 In this PR we are just introducing new types `NetworkResult` and `NetworkError` that will be used inside the `network` module. Our purpose is to isolate inside this `network` module everything related to outside-world communications and provide a clear (and small) interface that can be reused from the other modules in the crate. Signed-off-by: Javier G. Sogo <[email protected]> commit 6a653cf Author: Javier G. Sogo <[email protected]> Date: Tue Apr 15 15:05:54 2025 +0200 [refact] Reduce public surface + Move responsibilities closer (#62) Signed-off-by: Javier G. Sogo <[email protected]> commit bff6477 Author: Javier G. Sogo <[email protected]> Date: Wed Feb 12 10:08:37 2025 +0100 [refact] Refact/models (#61) Following #56, this PR moves the `FeatureSnapshot` and `PropertySnapshot` inside `Configuration` (closer to JSON data), this way the json-models doesn't leak into the application. --------- Signed-off-by: Javier G. Sogo <[email protected]> Signed-off-by: Rainer Schoenberger <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A refactor going in the direction of isolating our logic from the JSON data returned by the server:
crate::models::...members fromFeatureSnapshotandPropertySnapshotTargetingRules andSegments intoSegmentRulesobject. This new type handles the logic related tofind_applicable_segment_rule_for_entityand also the one to get theValuefrom the foundTargetingRule(now hidden into aSegmentRule)