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

bugfix 319 fix invalid BigDecimal serialization because of locale #320

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

bugfix 319 fix invalid BigDecimal serialization because of locale #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f75b4f
bugfix 319 fix invalid BigDecimal serialization because of locale
Simulant87 Aug 29, 2019
f91b6db
bugfix 319 set grouping on BigDecimal always to false
Simulant87 Aug 30, 2019
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: 3 additions & 1 deletion src/main/java/org/eclipse/yasson/internal/serializer/AbstractNumberDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.text.ParseException;
import java.util.Locale;
import java.util.Optional;

/**
Expand Down Expand Up @@ -49,9 +50,10 @@ protected final Optional<Number> deserializeFormatted(String jsonValue, boolean

final JsonbNumberFormatter numberFormat = getCustomization().getDeserializeNumberFormatter();
//consider synchronizing on format instance or per thread cache.
final NumberFormat format = NumberFormat.getInstance(jsonbContext.getConfigProperties().getLocale(numberFormat.getLocale()));
final NumberFormat format = NumberFormat.getInstance((Locale.ENGLISH));
((DecimalFormat)format).applyPattern(numberFormat.getFormat());
format.setParseIntegerOnly(integerOnly);
Copy link
Member

Choose a reason for hiding this comment

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

going with the same train of thought not allowing commas in JSON numbers, I think we should also call format.setGroupingUsed(false) here, because people may try to use a number format like ###,##0.00 which could output a number like 123,456.78 and should be serialized as 123456.78 according to RFC 7159

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated this and also the test to cover it. But there are other test failing:
org.eclipse.yasson.customization.NumberFormatTest#testDeserializer
org.eclipse.yasson.customization.NumberFormatTest#testDeserializeWithoutClassLevelFormatter

I am unsure if I should just adopt the expected behavior/input data to my new implementation to let them pass as this changes current behavior.

Copy link
Member

Choose a reason for hiding this comment

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

while I think this behavior proposed in this PR is technically correct, some existing users may be depending on the behavior and it would be good to not break such users in a micro release.

Since we have a major/minor release of JSON-B on the horizon, I am thinking we should defer this PR to a later release. Is that OK with you @Simulant87 or do you urgently need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is OK with me and I totally agree that because of the changed behavior it should be part of a major- and not part of a minor-release. I am not depending an a fix.

format.setGroupingUsed(false);
try {
return Optional.of(format.parse(jsonValue));
} catch (ParseException e) {
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/eclipse/yasson/internal/serializer/AbstractNumberSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import javax.json.stream.JsonGenerator;
import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.util.Locale;

/**
* Common serializer for numbers, using number format.
Expand Down Expand Up @@ -52,8 +53,9 @@ public AbstractNumberSerializer(Customization customization) {
@Override
protected void serialize(T obj, JsonGenerator generator, Marshaller marshaller) {
if (formatter != null) {
final NumberFormat format = NumberFormat.getInstance(marshaller.getJsonbContext().getConfigProperties().getLocale(formatter.getLocale()));
final NumberFormat format = NumberFormat.getInstance(Locale.ENGLISH);
((DecimalFormat)format).applyPattern(formatter.getFormat());
format.setGroupingUsed(false);
generator.write(format.format(obj));
} else {
serializeNonFormatted(obj, generator);
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/org/eclipse/yasson/documented/DocumentationExampleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ public static class Person9 {
@JsonbDateFormat("dd.MM.yyyy")
public LocalDate birthDate;

@JsonbNumberFormat("#0.00")
@JsonbNumberFormat(value = "###,##0.00", locale = "de-de")
public BigDecimal salary;
}

Expand All @@ -367,15 +367,15 @@ public void testDateNumberFormats1() {
Person9 p = new Person9();
p.name = "Jason Bourne";
p.birthDate = LocalDate.of(1999, 8, 7);
p.salary = new BigDecimal("123.45678");
p.salary = new BigDecimal("1234.5678");
Jsonb jsonb = JsonbBuilder.create();
String json = jsonb.toJson(p);
assertEquals("{\"birthDate\":\"07.08.1999\",\"name\":\"Jason Bourne\",\"salary\":\"123.46\"}", json);
assertEquals("{\"birthDate\":\"07.08.1999\",\"name\":\"Jason Bourne\",\"salary\":\"1234.57\"}", json);

Person9 after = jsonb.fromJson("{\"birthDate\":\"07.08.1999\",\"name\":\"Jason Bourne\",\"salary\":\"123.46\"}", Person9.class);
Person9 after = jsonb.fromJson("{\"birthDate\":\"07.08.1999\",\"name\":\"Jason Bourne\",\"salary\":\"1234.57\"}", Person9.class);
assertEquals(p.name, after.name);
assertEquals(p.birthDate, after.birthDate);
assertEquals(new BigDecimal("123.46"), after.salary);
assertEquals(new BigDecimal("1234.57"), after.salary);
}

public static class Person10 {
Expand Down