Skip to content
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

validate primitive and non-primitive fields simultaneously #1400

Open
rzpt opened this issue Jun 26, 2021 · 3 comments
Open

validate primitive and non-primitive fields simultaneously #1400

rzpt opened this issue Jun 26, 2021 · 3 comments

Comments

@rzpt
Copy link
Contributor

rzpt commented Jun 26, 2021

What happened?

Conjure objects validate primitive fields and non-primitive fields separately. The primitives are validated in the builder before the constructor is called, and the non-primitives are validated in the constructor. When creating an object without specifying all the fields, an exception will be thrown about the missing primitive fields. After fixing those, an exception will be thrown about the missing non-primitive fields.

What did you want to happen?

I'd like to know about all the missing fields at the same time.

@carterkozak
Copy link
Contributor

I agree with your assessment that all validation should occur at the same time. If memory serves, early versions of conjure didn’t validate primitives at all and ended up using their java default values rather than failing, so I can understand why the code might not be as consistent as we’d like.

have you considered opting into staged builders? It’s an abi break for existing apis, however we have rolled out automation to reorder call sites so that a recompile is sufficient in most cases. It’s certainly preferred for new modules because the type system handles validation for us at build time rather than failing at runtime.

@rzpt
Copy link
Contributor Author

rzpt commented Jun 26, 2021

How do I opt into staged builders? I'm working in a certain large internal repo. We didn't turn it on for this module.

@carterkozak
Copy link
Contributor

https://github.com/palantir/conjure-java#usage useStagedBuilders option allows us to opt on on a per-module basis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants