-
Notifications
You must be signed in to change notification settings - Fork 36
Solve ordering issue of record constructor parameters #169
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
Changes from 4 commits
c3fdd3b
4d0d628
16c9ff0
87a43d9
0ef1a88
4b26814
883c566
a800ee0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -452,7 +452,18 @@ protected BeanReader _resolveBeanForDeser(Class<?> raw, POJODefinition beanDef) | |
final Map<String, BeanPropertyReader> propMap; | ||
Map<String, String> aliasMapping = null; | ||
|
||
boolean isRecord = RecordsHelpers.isRecordType(raw); | ||
final Map<String, Integer> ctorParameterIndexMap = new HashMap<>(); | ||
final boolean isRecord = RecordsHelpers.isRecordType(raw); | ||
if (isRecord) { | ||
final Parameter[] parameters = constructors._recordCtor.getParameters(); | ||
for (int i = 0; i < parameters.length; i++) { | ||
Parameter parameter = parameters[i]; | ||
if (!parameter.getName().isEmpty()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why such check? Does this happen? Isn't it an error condition? And specifically if this happens wouldn't it just lead to an NPE when looking up the index? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, I think this is an irrelevant check, as you can not create a (record) instance variable without a name... It would lead to a NPE in the look-up part. I'd say just remove the check, as you can not test this as you'd get a compile error? WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, sounds good. But as per my note above, might be this part of code can be removed altogether. |
||
ctorParameterIndexMap.put(parameter.getName(), i); | ||
} | ||
} | ||
} | ||
|
||
if (len == 0) { | ||
propMap = Collections.emptyMap(); | ||
} else { | ||
|
@@ -496,7 +507,12 @@ protected BeanReader _resolveBeanForDeser(Class<?> raw, POJODefinition beanDef) | |
} | ||
} | ||
|
||
propMap.put(rawProp.name, new BeanPropertyReader(rawProp.name, field, setter, i)); | ||
if (isRecord) { | ||
final Integer ctorIndex = ctorParameterIndexMap.get(rawProp.name); | ||
propMap.put(rawProp.name, new BeanPropertyReader(rawProp.name, field, setter, ctorIndex)); | ||
} else { | ||
propMap.put(rawProp.name, new BeanPropertyReader(rawProp.name, field, setter, i)); | ||
} | ||
|
||
// 25-Jan-2020, tatu: Aliases are a bit different because we can not tie them into | ||
// specific reader instance, due to resolution of cyclic dependencies. Instead, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
package jr.failing; | ||
package jr; | ||
|
||
import com.fasterxml.jackson.jr.ob.JSON; | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.