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.

Sign up for GitHub

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

Jump to bottom

Make ValidationProblem implement Serializable #437

Merged
merged 4 commits into from
Mar 29, 2018
Merged

Make ValidationProblem implement Serializable #437

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ce6d096
Make ValidationProblem implement Serializable
Nov 2, 2016
3448ef5
Add custom serialization to ValidationProblem
Nov 2, 2016
3d44d90
Merge remote-tracking branch 'upstream/master'
iantabolt Oct 6, 2017
2692aa7
Add test case for serializing ValidationFailed
iantabolt Oct 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions config/src/main/java/com/typesafe/config/ConfigException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,10 @@ public NotResolved(String message) {
* {@link ConfigException.ValidationFailed} exception thrown from
* <code>checkValid()</code> includes a list of problems encountered.
*/
public static class ValidationProblem {
public static class ValidationProblem implements Serializable {

final private String path;
final private ConfigOrigin origin;
final private transient ConfigOrigin origin;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In discussion of #288 it looks like we are assuming ConfigOrigin is serializable (or at least, subclasses of it used in exceptions are). The transient here can't be the right fix in any case I don't think - we can't reconstruct this field after deserialization, and don't want to lose it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I wasn't sure what to do with that, but looking at your commit here b739f4b, it seems like ConfigOrigin is very intentionally not serializable. Maybe I should follow the lead of ConfigException, and add custom writeObject/readObject methods that call ConfigImplUtil.writeOrigin and ConfigImplUtil.readOrigin?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, good archaeology, I didn't remember that at all.

I guess you are right - we should copy what ConfigException does with the custom read/write.

Copy link
Contributor Author

@iantabolt iantabolt Nov 2, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I've updated it to use the custom serialization. I tried to extract a method to reuse some of the other code but maybe it would've been more straightforward to copy/paste the whole thing ¯_(ツ)_/¯. Let me know

final private String problem;

public ValidationProblem(String path, ConfigOrigin origin, String problem) {
Expand Down