Comments (5)
The following is severely sarcastic and I apologize in advance, but I couldn't find any reasonable way to express my thoughts. Please try to see past the form and understand the idea behind. (Unless by "current approach" you mean the approach I suggested in my previous comment? In which case the following is probably invalid - pretend I didn't write it.)
API call validation is the job of the server, not the client. The server can change its mind any day and make a field optional/required, or change the format of that field.
Yeah let's also just drop all SDKs and just let everyone write their requests raw, because after all the servers could also add new endpoints or remove them.
It's trivial to make many mistakes when dealing with an API, e.g. typo in the email which Sendgrid will reject correctly but the client can't validate easily.
Yeah the client if they try really hard can make at least one mistake which is difficult to prevent so let's just try to enable as many alternate mistakes as possible so that it's more challenge to write code that works.
If you don't agree with the sarcastic stuff above (which would be very legitimate), ask yourself : where do you set the limit, and why? I think there basically shouldn't be one.
providing typing for an API is the whole point of this kind of library. Otherwise if I'm going to have to read the Sendgrid spec anyway to figure which fields are mandatory and where I should put them, I almost might as well write my own serde SDK.
That won't be validated until the request actually runs in production
Indeed, this would dodge a request in case of bug, but not avoid it, so that doesn't help. The correct thing to do if the field is not optional is... simply to not use an option.
from sendgrid-rs.
As far as omitting required fields, we could leverage the Option type and check that there aren't any None values before sending a request.
That won't be validated until the request actually runs in production. I think you are right overall; API call validation is the job of the server, not the client. The server can change its mind any day and make a field optional/required, or change the format of that field. It's trivial to make many mistakes when dealing with an API, e.g. typo in the email which Sendgrid will reject correctly but the client can't validate easily.
I think the current approach is the best one. Enums for content type are definitely doable, and there is precedent for that in crates like reqwest, although they always have "break glass" ability to set any arbitrary header the user needs and that's important to keep imo.
P.S. Really great library, thank you for writing it!
from sendgrid-rs.
While it would be nice to ensure that a type has all the necessary fields, I'm not sure there is a feasible way to do this for all the various combinations. There are potentially lots of valid messages that would be hard to check against.
I don't see how your example Content
type would handle this either. Could you expand on that?
from sendgrid-rs.
While it would be nice to ensure that a type has all the necessary fields, I'm not sure there is a feasible way to do this for all the various combinations. There are potentially lots of valid messages that would be hard to check against.
If I understand correctly, you mean "It may not be possible to enforce everything by typing, while we should let every possible use of Sendgrid accessible". I agree, and there are a lot of examples of this impossibility to type every constraint. (Template id may be invalid, or corresponding template may not specify a body while body is an Option in the model I suggested...). I even agree that there may be a point where the added complexity in the interface may not be worth the risk of putting incoherent values in the fields. However I don't see how that could be a reason for not preventing mistakes we can easily prevent, e.g. no body, no target email addresses, invalid body content type...
I don't see how your example Content type would handle this either. Could you expand on that?
When specifying a content type with the current interface, it's easy to make mistakes:
Content::new().set_content_type("text/htlm").set_value(body))
You can forget to set one of the fields, or you can typo the content type.
The representation with the enum
would prevent that typo. It could still implement serde::Serialize
by serializing into a valid content-type string:
#[derive(Clone, Serialize)]
pub struct Body<'a> {
#[serde(rename = "type")]
pub content_type: ContentType,
pub body: &'a str,
}
#[derive(Clone, Copy)]
pub enum ContentType {
Html,
PlainText,
}
impl Serialize for ContentType {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
let content_type_str: &str = match self {
Self::Html => "text/html",
Self::PlainText => "text/plain",
};
content_type_str.serialize(serializer)
}
}
As presented, this new representation of Body
avoids most mistakes one could make when creating this object, while serializing in the exact same way as before, while also being faster (not reallocating String
s all the time), and (correct me if I'm wrong) does not prevent any previously possible valid usage of Sendgrid (the doc seems to specify only text/html
and text/plain
are valid values for this field. If that wasn't the case, nothing prevents us from adding a Custom
variant one would use for specific edge-cases).
from sendgrid-rs.
I am open to adding in typed APIs around things like content type. It would be very easy to just interpret those values at construction time and set a string accordingly. I would welcome changes like this. But I do agree with you that I don't want to have too many types to remember when adding fields, etc. As far as omitting required fields, we could leverage the Option
type and check that there aren't any None
values before sending a request.
Perhaps we can start small and incrementally add guard rails for things like this?
from sendgrid-rs.
Related Issues (20)
- Setting for sandbox mode HOT 2
- Sandbox mode HOT 1
- Async feature violates additive property HOT 6
- Keep changelogs somewhere HOT 1
- How to disable tracking? HOT 2
- Docs.rs showing outdated information HOT 1
- Use Rust's new asynchronous features HOT 1
- Make Sender own a reqwest::Client HOT 1
- Test Feature Gates
- `RequestNotSuccessful` lacks error details HOT 1
- Authorization failed HOT 6
- "rustls" feature now depends on "native-tls" HOT 2
- Personalization requires at least one email which isn't convenient HOT 2
- Support for either `pub add_field` or `add_template_id` HOT 2
- Update to `reqwest` 0.11 HOT 2
- dynamic_template_data uses SGMap instead of a more general JSON friendly type
- Can't set reply_to in the V3 API HOT 1
- Move to GitHub CI
- Set Subject for Personalization
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from sendgrid-rs.