Skip to content

Commit 63654bf

Browse files
iamdanfoxiamdanfox
authored
[fix] Builders check primitives are initialized (#36)
## Before this PR Conjure POJOs containing primitives (e.g. int, boolean) would be coerced from JSON in which the fields were actually missing. This was not compliant with the wire spec and could lead to nasty bugs, where a field was unintentionally initialized to `0` or `false`. For example, consider this type: ```yaml UpdateSalaryRequest: salary: integer shares: integer ``` The following request would result in dfox's salary being set to 0, because the the 'salary' field was missing and is initialized to 0!! ``` curl -XPOST https://some-api/update-salary/dfox --data '{"shares":20}' ``` ## After this PR Trying to deserialize a type with booleans, integers or doubles from `{}` will now fail. ## Concerns People may have been relying on this behaviour, so there's a chance this might break people's servers... In theory, we could put this behaviour behind a feature flag, and then phase it in slowly?? EDIT not gonna do this as it's not actually a wire-break and people's servers can be trivially fixed by updating their conjure defs: ```diff UpdateSalaryRequest: - salary: integer + salary: optional<integer> shares: integer ``` _Future work: unify this `validatePrimitivesHaveBeenInitialized` method with the other `validateFields` method_ fixes #14
1 parent 4c49d22 commit 63654bf

File tree

10 files changed

+245
-23
lines changed

10 files changed

+245
-23
lines changed

‎conjure-java-core/src/integrationInput/java/com/palantir/product/BooleanExample.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import com.fasterxml.jackson.annotation.JsonProperty;
55
import com.fasterxml.jackson.annotation.JsonSetter;
66
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
7+
import java.util.ArrayList;
8+
import java.util.List;
79
import java.util.Objects;
810
import javax.annotation.Generated;
911

@@ -65,6 +67,8 @@ public static Builder builder() {
6567
public static final class Builder {
6668
private boolean coin;
6769

70+
private boolean _coinInitialized = false;
71+
6872
private Builder() {}
6973

7074
public Builder from(BooleanExample other) {
@@ -75,10 +79,33 @@ public Builder from(BooleanExample other) {
7579
@JsonSetter("coin")
7680
public Builder coin(boolean coin) {
7781
this.coin = coin;
82+
this._coinInitialized = true;
7883
return this;
7984
}
8085

86+
private void validatePrimitiveFieldsHaveBeenInitialized() {
87+
List<String> missingFields = null;
88+
missingFields = addFieldIfMissing(missingFields, _coinInitialized, "coin");
89+
if (missingFields != null) {
90+
throw new IllegalArgumentException(
91+
"Some required fields have not been set: " + missingFields);
92+
}
93+
}
94+
95+
private static List<String> addFieldIfMissing(
96+
List<String> prev, boolean initialized, String fieldName) {
97+
List<String> missingFields = prev;
98+
if (!initialized) {
99+
if (missingFields == null) {
100+
missingFields = new ArrayList<>(1);
101+
}
102+
missingFields.add(fieldName);
103+
}
104+
return missingFields;
105+
}
106+
81107
public BooleanExample build() {
108+
validatePrimitiveFieldsHaveBeenInitialized();
82109
return new BooleanExample(coin);
83110
}
84111
}

‎conjure-java-core/src/integrationInput/java/com/palantir/product/DoubleExample.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import com.fasterxml.jackson.annotation.JsonProperty;
55
import com.fasterxml.jackson.annotation.JsonSetter;
66
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
7+
import java.util.ArrayList;
8+
import java.util.List;
79
import java.util.Objects;
810
import javax.annotation.Generated;
911

@@ -64,6 +66,8 @@ public static Builder builder() {
6466
public static final class Builder {
6567
private double doubleValue;
6668

69+
private boolean _doubleValueInitialized = false;
70+
6771
private Builder() {}
6872

6973
public Builder from(DoubleExample other) {
@@ -74,10 +78,34 @@ public Builder from(DoubleExample other) {
7478
@JsonSetter("doubleValue")
7579
public Builder doubleValue(double doubleValue) {
7680
this.doubleValue = doubleValue;
81+
this._doubleValueInitialized = true;
7782
return this;
7883
}
7984

85+
private void validatePrimitiveFieldsHaveBeenInitialized() {
86+
List<String> missingFields = null;
87+
missingFields =
88+
addFieldIfMissing(missingFields, _doubleValueInitialized, "doubleValue");
89+
if (missingFields != null) {
90+
throw new IllegalArgumentException(
91+
"Some required fields have not been set: " + missingFields);
92+
}
93+
}
94+
95+
private static List<String> addFieldIfMissing(
96+
List<String> prev, boolean initialized, String fieldName) {
97+
List<String> missingFields = prev;
98+
if (!initialized) {
99+
if (missingFields == null) {
100+
missingFields = new ArrayList<>(1);
101+
}
102+
missingFields.add(fieldName);
103+
}
104+
return missingFields;
105+
}
106+
80107
public DoubleExample build() {
108+
validatePrimitiveFieldsHaveBeenInitialized();
81109
return new DoubleExample(doubleValue);
82110
}
83111
}

‎conjure-java-core/src/integrationInput/java/com/palantir/product/IntegerExample.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import com.fasterxml.jackson.annotation.JsonProperty;
55
import com.fasterxml.jackson.annotation.JsonSetter;
66
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
7+
import java.util.ArrayList;
8+
import java.util.List;
79
import java.util.Objects;
810
import javax.annotation.Generated;
911

@@ -65,6 +67,8 @@ public static Builder builder() {
6567
public static final class Builder {
6668
private int integer;
6769

70+
private boolean _integerInitialized = false;
71+
6872
private Builder() {}
6973

7074
public Builder from(IntegerExample other) {
@@ -75,10 +79,33 @@ public Builder from(IntegerExample other) {
7579
@JsonSetter("integer")
7680
public Builder integer(int integer) {
7781
this.integer = integer;
82+
this._integerInitialized = true;
7883
return this;
7984
}
8085

86+
private void validatePrimitiveFieldsHaveBeenInitialized() {
87+
List<String> missingFields = null;
88+
missingFields = addFieldIfMissing(missingFields, _integerInitialized, "integer");
89+
if (missingFields != null) {
90+
throw new IllegalArgumentException(
91+
"Some required fields have not been set: " + missingFields);
92+
}
93+
}
94+
95+
private static List<String> addFieldIfMissing(
96+
List<String> prev, boolean initialized, String fieldName) {
97+
List<String> missingFields = prev;
98+
if (!initialized) {
99+
if (missingFields == null) {
100+
missingFields = new ArrayList<>(1);
101+
}
102+
missingFields.add(fieldName);
103+
}
104+
return missingFields;
105+
}
106+
81107
public IntegerExample build() {
108+
validatePrimitiveFieldsHaveBeenInitialized();
82109
return new IntegerExample(integer);
83110
}
84111
}

‎conjure-java-core/src/integrationInput/java/com/palantir/product/ManyFieldExample.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ public static final class Builder {
226226

227227
private StringAliasExample alias;
228228

229+
private boolean _integerInitialized = false;
230+
231+
private boolean _doubleValueInitialized = false;
232+
229233
private Builder() {}
230234

231235
public Builder from(ManyFieldExample other) {
@@ -251,13 +255,15 @@ public Builder string(String string) {
251255
@JsonSetter("integer")
252256
public Builder integer(int integer) {
253257
this.integer = integer;
258+
this._integerInitialized = true;
254259
return this;
255260
}
256261

257262
/** docs for doubleValue field */
258263
@JsonSetter("doubleValue")
259264
public Builder doubleValue(double doubleValue) {
260265
this.doubleValue = doubleValue;
266+
this._doubleValueInitialized = true;
261267
return this;
262268
}
263269

@@ -345,7 +351,31 @@ public Builder alias(StringAliasExample alias) {
345351
return this;
346352
}
347353

354+
private void validatePrimitiveFieldsHaveBeenInitialized() {
355+
List<String> missingFields = null;
356+
missingFields = addFieldIfMissing(missingFields, _integerInitialized, "integer");
357+
missingFields =
358+
addFieldIfMissing(missingFields, _doubleValueInitialized, "doubleValue");
359+
if (missingFields != null) {
360+
throw new IllegalArgumentException(
361+
"Some required fields have not been set: " + missingFields);
362+
}
363+
}
364+
365+
private static List<String> addFieldIfMissing(
366+
List<String> prev, boolean initialized, String fieldName) {
367+
List<String> missingFields = prev;
368+
if (!initialized) {
369+
if (missingFields == null) {
370+
missingFields = new ArrayList<>(2);
371+
}
372+
missingFields.add(fieldName);
373+
}
374+
return missingFields;
375+
}
376+
348377
public ManyFieldExample build() {
378+
validatePrimitiveFieldsHaveBeenInitialized();
349379
return new ManyFieldExample(
350380
string, integer, doubleValue, optionalItem, items, set, map, alias);
351381
}

‎conjure-java-core/src/integrationInput/java/com/palantir/product/ReservedKeyExample.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ public static final class Builder {
136136

137137
private int memoizedHashCode_;
138138

139+
private boolean _memoizedHashCodeInitialized = false;
140+
139141
private Builder() {}
140142

141143
public Builder from(ReservedKeyExample other) {
@@ -169,10 +171,35 @@ public Builder fieldNameWithDashes(String fieldNameWithDashes) {
169171
@JsonSetter("memoizedHashCode")
170172
public Builder memoizedHashCode_(int memoizedHashCode_) {
171173
this.memoizedHashCode_ = memoizedHashCode_;
174+
this._memoizedHashCodeInitialized = true;
172175
return this;
173176
}
174177

178+
private void validatePrimitiveFieldsHaveBeenInitialized() {
179+
List<String> missingFields = null;
180+
missingFields =
181+
addFieldIfMissing(
182+
missingFields, _memoizedHashCodeInitialized, "memoizedHashCode");
183+
if (missingFields != null) {
184+
throw new IllegalArgumentException(
185+
"Some required fields have not been set: " + missingFields);
186+
}
187+
}
188+
189+
private static List<String> addFieldIfMissing(
190+
List<String> prev, boolean initialized, String fieldName) {
191+
List<String> missingFields = prev;
192+
if (!initialized) {
193+
if (missingFields == null) {
194+
missingFields = new ArrayList<>(1);
195+
}
196+
missingFields.add(fieldName);
197+
}
198+
return missingFields;
199+
}
200+
175201
public ReservedKeyExample build() {
202+
validatePrimitiveFieldsHaveBeenInitialized();
176203
return new ReservedKeyExample(
177204
package_, interface_, fieldNameWithDashes, memoizedHashCode_);
178205
}

0 commit comments

Comments
 (0)