Hey there! I just ""finished"" an API crate for working with a download service for the Swedish traffic authority and I'd like some both code and design suggestions.
Some pointers/particular areas of interest:
I use Reqwest internally for requests. Is this good practice? Should I let the user handle those details or does it really matter?
I've got a couple of doc tests, and I know I'll have to avoid calling the API during tests when possible. I have a decent enough idea of how I'll go about it, but suggestions are welcome.
You can see on line 148 and 156 that I have two functions for getting data package files. One from a DataPackageFolder type and another from an arbitrary id. Is this the right play?
Whilst the published datasets (ie the results from get_published_packages) are pretty consistent, I'm not trusting them to remain consistent enough for me to directly reference or catalog them in code. Is this the right call? I could probably automate this to some extent otherwise.
Use thiserror for removing the boilerplate from the definition of custom error types.
Writing a separate method for every endpoint is not great design. You are repeating yourself all the time. Do this instead.
Do not force trait objects (or indirection) in your API. There's no reason for download_file() (etc.) to take &mut dyn Write. You should take a generic writer W by-value (which will work with mutable references too).
I'll disagree here - removing dependencies, even at the cost of a little extra boilerplate, is often quite worthwhile.
I'll comment on the public APIs below:
Document everything! The picture below is likely to be users' first introduction to the crate via docs.rs. The root level documentation is a chance for you to explain what this crate is for, how it's used, why it's better than other crates, and so on. Additionally, make sure to add a core motivating example of how the crate ought to be used to solve a problem.
Flatten out your namespace hierarchy. There's no need to hide all the types in the types module, and it just makes things more verbose for your users.
Add more doctests! They're the best way to communicate what the intended use-case is for your users. I try to have one doctest for every public API item in my own crates.
No need to prefix Lastkajen on your error type. Additionally, think about how your users will actually interact with the error. I'm a big fan of how std::io::Error does it in terms of maximizing your flexibility.
You might want to avoid having public fields on your data types and instead using methods to access them. This means changing the internal data layout of your values isn't a breaking change, and you can also add more fields without breaking users' code.
This is perhaps a little bikesheddy, but a Lastkajen isn't really a great name for the structure. Perhaps call it a Session or a Client to more accurately describe its use? Additionally, perhaps you should provide some functionality for users to determine whether the session has expired without manually accessing data.
That's just my initial thoughts from looking at the public API; feel free to avoid some of this advice if you have a better idea :).
Hand-rolling the error type won't remove any substantial dependencies. OP is already depending on serde with the derive feature, which means that the heavy proc-macro related dependencies (syn and quote) are already there.
