-
Notifications
You must be signed in to change notification settings - Fork 14
Client re-architecture #47
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
base: main
Are you sure you want to change the base?
Conversation
This commit introduces a comprehensive and robust implementation for handling complex document structures within the `#[derive(Typesense)]` macro, enabling powerful schema generation directly from Rust structs. The macro now supports the full range of advanced indexing strategies offered by Typesense, including automatic object indexing, field flattening with prefix control, and patterns for manual flattening. ### Key Features & Implementation Details - **Automatic Object Indexing:** - A field containing a nested struct that also derives `Document` is now automatically mapped to a Typesense `object` (or `object[]` for `Vec<T>`). - This feature requires `#[typesense(enable_nested_fields = true)]` on the parent collection, which the macro now supports. - **Automatic Field Flattening with `#[typesense(flatten)]`:** - A field marked `#[typesense(flatten)]` has its sub-fields expanded into the parent schema using dot-notation. - By default, the Rust field's name is used as the prefix for all sub-fields (e.g., `details: ProductDetails` results in schema fields like `details.part_number`). - **Prefix Override for Flattening:** - The `flatten` attribute can be combined with `rename` to provide a custom prefix for the flattened fields. - Usage: `#[typesense(flatten, rename = "custom_prefix")]` - This provides powerful schema mapping flexibility, allowing the Rust struct's field name to differ from the prefix used in the Typesense schema. - **Manual Flattening Pattern (`skip` + `rename`):** - A new `#[typesense(skip)]` attribute has been introduced to completely exclude a field from the generated Typesense schema. - This enables the powerful pattern of sending both nested and flattened data to Typesense: the nested version can be used for display/deserialization, while a separate set of flattened fields is used for indexing. This is achieved by: 1. Marking the nested struct field (e.g., `details: Details`) with `#[typesense(skip)]`. 2. Adding corresponding top-level fields to the Rust struct, marked with `#[typesense(rename = "details.field_name")]`. - **Ergonomic Boolean Attributes:** - All boolean attributes (`facet`, `sort`, `index`, `store`, `infix`, `stem`, `optional`, `range_index`) now support shorthand "flag" syntax. - For example, `#[typesense(sort)]` is a valid and recommended equivalent to `#[typesense(sort = true)]`, dramatically improving readability and consistency. - **Robust Error Handling & Validation:** - The macro provides clear, compile-time errors for invalid or ambiguous attribute usage. - It correctly detects and reports duplicate attributes, whether they are in the same `#[typesense(...)]` block or across multiple attributes on the same field. ### Testing - **Comprehensive Integration Test (`derive_integration.rs`):** - A new, full-lifecycle integration test has been added to validate the entire feature set. - The test defines a complex struct using every new attribute and pattern, generates a schema, creates a real collection, and uses the generic client (`collection_of<T>`) to perform and validate a full Create, Read, Update, Delete, and Search lifecycle. - A second integration test was added to specifically validate the manual flattening pattern. - **UI Tests:** - `trybuild` UI tests have been added to verify that the macro produces the correct compile-time errors for invalid attribute combinations, such as duplicate attributes.
…bon; restructure library layout
fccf912
to
3bc15f7
Compare
Why would you want to wrap each API function into it's own struct implementation? That's something I have tried to avoid. Because it's hard to synchronize (support) manually with an OpenAPI implementation. It does not provide any benefits except not needing to write I would also propose to separate it to different PRs, because it's easier to review smaller PRs:
if let Some(ref apikey) = configuration.api_key {
let key = apikey.key.clone();
let value = match apikey.prefix {
Some(ref prefix) => format!("{} {}", prefix, key),
None => key,
}; I previously manually replaced with: if let Some(ref local_var_apikey) = local_var_configuration.api_key {
let local_var_key = &local_var_apikey.key;
let local_var_value = match local_var_apikey.prefix {
Some(ref local_var_prefix) => format!("{local_var_prefix} {local_var_key}"),
None => local_var_key.clone(),
}; to avoid extra clone. Also |
Hi @RoDmitry,
It's to support the multi-node configuration (retry to a different node), it also give us more control over the input and output schemas of each API method. One example is the // Handle the result: parse to raw SearchResult, then convert to generic SearchResult<Value>
match raw_result {
Ok(json_value) => {
// A union search returns a single SearchResult object, not a MultiSearchResult.
// First, parse into the non-generic, raw model.
let raw_search_result: raw_models::SearchResult =
serde_json::from_value(json_value).map_err(Error::from)?;
// Then, use our existing constructor to convert the raw result to the typed one,
// specifying `serde_json::Value` as the document type.
SearchResult::<serde_json::Value>::from_raw(raw_search_result).map_err(Error::from)
}
Err(e) => Err(e),
} And isn't this let search_requests = MultiSearchSearchesParameter {
// the `union` field is removed
searches: vec![
MultiSearchCollectionParameters {
collection: Some("company".to_owned()),
..Default::default()
},
],
};
let common_params = MultiSearchParameters {
limit: Some(1),
..Default::default()
};
let raw_response = client.multi_search().perform(&search_requests, &common_params).await?; looks cleaner than this? let request_body = raw_models::MultiSearchSearchesParameter {
union: Some(false),
searches: search_requests,
};
let params = MultiSearchParams {
// body
multi_search_searches_parameter: Some(request_body),
// common params
collection: "companies".to_owned()
}
documents_api::multi_search(&config, params).await
Sorry, I’m not sure I understand your point, could you clarify?
Yes, all tests have passed. This way we can minimize modifying the auto-generated code which will eventually be overwritten by future open api spec updates.
My bad, I'm fairly new to Rust, my thinking was simply 'I need a String,' so to_string() was the first method that came to mind 😅.
Thanks for the catch, I will get it updated!
Hmmm, the retry mechanism in the
Because of the
The rust client is too outdated right now, we should prioritize adding the missing features in my opinion. We can always optimize later and it won't be a breaking change. Thanks for the feedback @RoDmitry, how does this sound? I can start by opening a small PR to add the Xtasks. |
I did not notice the retry mechanism, my bad, it makes sense. Will take a look at it later. If you have multiple
|
Yes, will post up a PR for this shortly |
I have looked at the |
That That load-balance logic is similar to the logic in Typesense clients for other languages. More info in the official docs.
Could you clarify what the drawbacks are? This wrapper design works really well in the Typesense Go client. It gives us more control/flexibility over the input and output schemas (like I said above). Auto-generated API methods are not very clean, we should not expose it to the user, it should only be used internally to reduce boilerplate code.
I'm open to improve this in a separate smaller PR. |
For me it sounds like: "I don't care about performance". Not good for somebody who tries to re-architecture the code API. But all I was trying to say that this can be made simpler, without too much affect on performance. Why do you want to avoid modifying the auto-generated code? Do you have the same opinion towards the AI generated code? 😁😅
So make them clean. Don't rely that much on auto-generation. It does not make the code unmodifiable. Git allows you to review changes after re-generation, and revert the changed parts. Also you can cover these changes with tests, so all output types will be checked.
The thing that bothers me is that P.S. I'm not against a builder concept. |
That was my attempt to avoid premature optimization 😅, I do care about performance. That's why while working on this I always make sure that future improvement/optimization won't introduce breaking change for users.
I am not against modifying the auto-generated code, in fact, I did modify some of it out of necessity (for endpoints that accept JSONL like document import, the auto-generated code caused JSONL to be serialized into JSON). But we should avoid it whenever possible as this would make the developer experience much better overall: less auto-generated code modifications to keep track of and future updates to parameters can be made by running one single command.
Imagine a new contributor run the openapi codegen expecting to add just one new search parameter but it overwrite all previous changes we made to the auto-generated code and a bunch of tests start failing. Also, it would be painful for reviewers to keep track of all the modifications.
Really? I thought this was fast enough that it is negligible. In this case it might be worth it to directly modify the auto generated code. So the problem isn't about the wrapper API methods but the wrapper struct? I'm thinking of writing a simple script to find and replace the auto-generated code, and it will run after the codegen script.
One thing for sure is that it's a better Rust developer than I am 😂. Just picking up Rust as I work on this project and I'm amazed by how good the developer experience is. |
Change Summary
This PR modernizes the Rust client with multi-node support, a cleaner/ergonomic builder API, helpers for federated search, typed parsing, and a substantially improved derive workflow for complex/nested document schemas. It also replaces mocked HTTP tests with a real Typesense server in CI to increase reliability.
What’s in this PR
Xtasks : added
fetch
andcode-gen
tasks to fetch the open-api spec and run code generation with this new config. Command:cargo xtask fetch code-gen
Multi-node client with retry/load-balance logic. All API methods now live inside
typesense/src/client/*
.Ergonomic builder API for client config, search parameters, multi-search, collection schema & fields (
typesense/src/builders/*
).Federated search helpers and typed result parsing (
models/multi_search.rs
,traits/multi_search_ext.rs
).Derive improvements: advanced nested + flattened field handling with UI tests for invalid attributes. Previously only the
facet
flag is supported, this PR added the ability to specify attribute value (string, number, boolean,...)WASM support (but timeout and retry features are disabled because of
reqwest
andreqwest_middleware
)CI: run against a real Typesense server (Docker), removed API mocks, lint config updates (disable all lints for
typesense_codegen
).Usage highlights
Examples
For more examples including the multi-search, union-search typed parsing helpers, please see the integration tests.
Local quick start
Backward compatibility
This PR is a substantial re-architecture of the Typesense Rust client. There is no backward compatibility guarantee with previous versions. APIs have been redesigned around the new multi-node client. The APIs in
typesense_codegen
are no longer exported.Documentation
Inline docs added alongside the new builders and client modules. Follow-up PRs will update the README with examples.
Checklist
PR Checklist