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

Fix parsing of numbers of Locales with "non-standard"-grouping and/or decimal separators #3778

Closed
wants to merge 6 commits into from
Closed

Fix parsing of numbers of Locales with "non-standard"-grouping and/or decimal separators #3778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
973c672
Set foreground color for the limit exceeded widget as well and use
kimmerin Feb 1, 2024
136b9dd
added a test as starting point for more
kimmerin Feb 1, 2024
be5013d
removed system out println
kimmerin Feb 1, 2024
075b868
Fixed the parsing of numbers with "non-standard" formatting Locales,
kimmerin Feb 5, 2024
a557a97
removed (again)
kimmerin Feb 5, 2024
96ae5c4
call init-methods in setup as well
kimmerin Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
181 changes: 172 additions & 9 deletions name.abuchen.portfolio.tests/src/name/abuchen/portfolio/model/AttributeTypeTest.java
View file
Open in desktop
@@ -1,34 +1,118 @@
package name.abuchen.portfolio.model;

import static org.hamcrest.Matchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;

import java.text.MessageFormat;
import java.text.ParseException;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;

import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import name.abuchen.portfolio.Messages;
import name.abuchen.portfolio.model.AttributeType.AmountConverter;
import name.abuchen.portfolio.model.AttributeType.Converter;
import name.abuchen.portfolio.model.AttributeType.LimitPriceConverter;
import name.abuchen.portfolio.model.AttributeType.LongConverter;
import name.abuchen.portfolio.model.AttributeType.PercentConverter;
import name.abuchen.portfolio.model.LimitPrice.RelationalOperator;
import name.abuchen.portfolio.model.proto.v1.PAnyValue;
import name.abuchen.portfolio.money.Values;
import name.abuchen.portfolio.money.ValuesBuilder;

@SuppressWarnings("nls")
public class AttributeTypeTest
{
private static Locale DEFAULT_LOCALE = Locale.getDefault();

@BeforeClass
public static void setupLocale()
@Before
public void setupLocale()
{
DEFAULT_LOCALE = Locale.getDefault();
Locale.setDefault(Locale.GERMAN);
Locale.setDefault(Locale.GERMANY);
ValuesBuilder.initQuoteValuesDecimalFormat();
AttributeType.initPatterns();
}

@AfterClass
public static void resetLocale()
@After
public void resetLocale()
{
Locale.setDefault(DEFAULT_LOCALE);
ValuesBuilder.initQuoteValuesDecimalFormat();
AttributeType.initPatterns();
}

@Test
public void testLongShareConverter_deDE() throws Exception
{
Locale.setDefault(Locale.GERMANY);
ValuesBuilder.initQuoteValuesDecimalFormat();
AttributeType.initPatterns();
performLongConverterTest(
Values.Share,
Arrays.asList(" 12.345,67890123 ", "12.345,6789012345", "12.345,678901236", "12345,67890123",
"1.2.345,678901236", "-12.345,67890123"),
Arrays.asList(1234567890123L, 1234567890123L, 1234567890123L, 1234567890123L, 1234567890123L,
-1234567890123L),
Arrays.asList("12.345,67890123", "12.345,67890123", "12.345,67890123", "12.345,67890123",
"12.345,67890123", "-12.345,67890123"));
}

@Test
public void testLongShareConverter_deCH() throws Exception
{
Locale.setDefault(new Locale("de", "CH"));
ValuesBuilder.initQuoteValuesDecimalFormat();
AttributeType.initPatterns();
performLongConverterTest(
Values.Share,
Arrays.asList(" 12’345.67890123 ", "12’345.6789012345", "12’345.678901236", "12345.67890123",
"1’2’345.678901236"),
Arrays.asList(1234567890123L, 1234567890123L, 1234567890123L, 1234567890123L, 1234567890123L),
Arrays.asList("12’345.67890123", "12’345.67890123", "12’345.67890123", "12’345.67890123",
"12’345.67890123"));
}

@Test
public void testLimitPriceConverter_deDE()
{
Locale.setDefault(Locale.GERMANY);
ValuesBuilder.initQuoteValuesDecimalFormat();
AttributeType.initPatterns();

performLimitPriceTests(
Arrays.asList(" < 123,45 ", "<= 123,45", ">= 1.123,45", " > 1.234", " > 1.2.34,5678"),
Arrays.asList(new LimitPrice(RelationalOperator.SMALLER, 12345000000L),
new LimitPrice(RelationalOperator.SMALLER_OR_EQUAL, 12345000000L),
new LimitPrice(RelationalOperator.GREATER_OR_EQUAL, 112345000000L),
new LimitPrice(RelationalOperator.GREATER, 123400000000L),
new LimitPrice(RelationalOperator.GREATER, 123456780000L)),
Arrays.asList("< 123,45", "<= 123,45", ">= 1.123,45", "> 1.234,00", "> 1.234,5678"));
}

@Test
public void testLimitPriceConverter_deCH()
{
Locale.setDefault(new Locale("de", "CH"));
ValuesBuilder.initQuoteValuesDecimalFormat();
AttributeType.initPatterns();

performLimitPriceTests(
Arrays.asList(" < 123.45 ", "<= 123.45", ">= 1’123.45", " > 1’234", " > 1’2’34.5678"),
Arrays.asList(new LimitPrice(RelationalOperator.SMALLER, 12345000000L),
new LimitPrice(RelationalOperator.SMALLER_OR_EQUAL, 12345000000L),
new LimitPrice(RelationalOperator.GREATER_OR_EQUAL, 112345000000L),
new LimitPrice(RelationalOperator.GREATER, 123400000000L),
new LimitPrice(RelationalOperator.GREATER, 123456780000L)),
Arrays.asList("< 123.45", "<= 123.45", ">= 1’123.45", "> 1’234.00", "> 1’234.5678"));
}

@Test
Expand All @@ -50,8 +134,87 @@ public void testPercentParsing()
assertThat(converter.fromString("22"), is(0.22));
assertThat(converter.fromString("12,34%"), is(0.1234));
assertThat(converter.fromString("12,34567%"), is(0.1234567));

assertThat(converter.toString(converter.fromString("22%")), is("22,00%"));
assertThat(converter.toString(converter.fromString("22")), is("22,00%"));
}

private void performLongConverterTest(Values<Long> values, List<String> validParseTexts, List<Long> parseResults,
List<String> toStringResult)
{
Iterator<String> it = validParseTexts.iterator();
Iterator<Long> valIt = parseResults.iterator();
Iterator<String> toStringIt = toStringResult.iterator();
while (it.hasNext())
{
String validParseText = it.next();
System.out.println(validParseText);
Long val = valIt.next();
LongConverter sc = new LongConverter(ValuesBuilder.createNumberValues(values));
assertThat(sc.toString(null), is(""));
assertThat(sc.toString(val), is(toStringIt.next()));
assertThrows(NullPointerException.class, () -> sc.fromString(null));
assertNull(sc.fromString(" "));
assertThat(sc.fromString(validParseText), is(val));
assertThat(sc.fromString(validParseText), is(values.factorize(val.doubleValue() / values.factor())));
checkParseException(sc, "notanumber", Messages.MsgNotANumber, false);

PAnyValue pav;
pav = sc.toProto(null);
assertNotNull(pav);
assertThat(pav.hasNull(), is(true));
assertNull(sc.fromProto(pav));
pav = sc.toProto(val);
assertNotNull(pav);
assertThat(pav.hasInt64(), is(true));
assertThat(sc.fromProto(pav), is(val));
}
}

private void performLimitPriceTests(List<String> validParseTexts, List<LimitPrice> parseResults,
List<String> toStringResult)
{
Iterator<String> it = validParseTexts.iterator();
Iterator<LimitPrice> resultIt = parseResults.iterator();
Iterator<String> toStringIt = toStringResult.iterator();
while (it.hasNext())
{
String validParseText = it.next();
System.out.println(validParseText);
LimitPrice val = resultIt.next();
LimitPriceConverter sc = new LimitPriceConverter();
assertThat(sc.toString(null), is(""));
assertThat(sc.fromString(validParseText), is(val));
assertThat(sc.toString(val), is(toStringIt.next()));
assertThrows(NullPointerException.class, () -> sc.fromString(null));
assertNull(sc.fromString(" "));
checkParseException(sc, "notanumber", Messages.MsgNotAComparator, false);

PAnyValue pav;
pav = sc.toProto(null);
assertNotNull(pav);
assertThat(pav.hasNull(), is(true));
assertNull(sc.fromProto(pav));
pav = sc.toProto(val);
assertNotNull(pav);
assertThat(pav.hasString(), is(true));
assertThat(sc.fromProto(pav), is(val));
}
}

private void checkParseException(Converter sc, String toParse, String expectedResourceKey, boolean expectCause)
{
IllegalArgumentException iae;
iae = assertThrows(IllegalArgumentException.class, () -> sc.fromString(toParse));
assertThat(iae.getMessage(), is(MessageFormat.format(expectedResourceKey, toParse)));
if (expectCause)
{
assertNotNull(iae.getCause());
assertThat(iae.getCause().getClass().getName(), is(ParseException.class.getName()));
}
else
{
assertNull(iae.getCause());
}
}
}
44 changes: 44 additions & 0 deletions name.abuchen.portfolio.tests/src/name/abuchen/portfolio/money/ValuesBuilder.java
View file
Open in desktop
@@ -0,0 +1,44 @@
package name.abuchen.portfolio.money;

import java.text.DecimalFormat;

import name.abuchen.portfolio.money.Values.QuoteValues;

public class ValuesBuilder
{
public static <T extends Number> Values<T> createNumberValues(Values<T> orgValues)
{
return new Values<T>(orgValues.pattern(), orgValues.precision())
{
private final DecimalFormat format = new DecimalFormat(pattern());

@Override
public String format(Number share)
{
if (DiscreetMode.isActive())
return DiscreetMode.HIDDEN_AMOUNT;
else
return format.format(share.doubleValue() / divider());
}
};
}

public static <T extends Number> Values<T> createNumberValues(String pattern, int precision)
Copy link
Member

Choose a reason for hiding this comment

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

I think this (test) method is not used anywhere in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, I've created it and then realized that the code that is involved in the parsing of monetary values didn't use the parse-methods in the Values-instance but above DecimalFormat in QUOTE_FORMAT. I've left the method in because it might come handy in future testcases where this might be needed.

{
return new Values<T>(pattern, precision)
{

@Override
public String format(T amount)
{
DecimalFormat df = new DecimalFormat(pattern);
return df.format(amount);
}
};
}

public static void initQuoteValuesDecimalFormat()
{
QuoteValues.QUOTE_FORMAT.set(new DecimalFormat(QuoteValues.QUOTE_PATTERN));
}
}
13 changes: 7 additions & 6 deletions name.abuchen.portfolio.ui/src/name/abuchen/portfolio/ui/editor/ClientInput.java
View file
Open in desktop
Expand Up @@ -557,30 +557,31 @@ private void scheduleOnlineUpdateJobs()
if (preferences.getBoolean(UIConstants.Preferences.UPDATE_QUOTES_AFTER_FILE_OPEN, true))
{
Predicate<Security> onlyActive = s -> !s.isRetired();
Client c = getClient();

Job initialQuoteUpdate = new UpdateQuotesJob(client, onlyActive,
Job initialQuoteUpdate = new UpdateQuotesJob(c, onlyActive,
EnumSet.of(UpdateQuotesJob.Target.LATEST, UpdateQuotesJob.Target.HISTORIC));
initialQuoteUpdate.schedule(1000);

CreateInvestmentPlanTxJob checkInvestmentPlans = new CreateInvestmentPlanTxJob(client,
CreateInvestmentPlanTxJob checkInvestmentPlans = new CreateInvestmentPlanTxJob(c,
exchangeRateProviderFacory);
checkInvestmentPlans.startAfter(initialQuoteUpdate);
checkInvestmentPlans.schedule(1100);

int thirtyMinutes = 1000 * 60 * 30;
Job job = new UpdateQuotesJob(client, onlyActive, EnumSet.of(UpdateQuotesJob.Target.LATEST))
Job job = new UpdateQuotesJob(c, onlyActive, EnumSet.of(UpdateQuotesJob.Target.LATEST))
.repeatEvery(thirtyMinutes);
job.schedule(thirtyMinutes);
regularJobs.add(job);

int sixHours = 1000 * 60 * 60 * 6;
job = new UpdateQuotesJob(client, onlyActive, EnumSet.of(UpdateQuotesJob.Target.HISTORIC))
job = new UpdateQuotesJob(c, onlyActive, EnumSet.of(UpdateQuotesJob.Target.HISTORIC))
.repeatEvery(sixHours);
job.schedule(sixHours);
regularJobs.add(job);

new SyncOnlineSecuritiesJob(client).schedule(5000);
new UpdateDividendsJob(getClient()).schedule(7000);
new SyncOnlineSecuritiesJob(c).schedule(5000);
new UpdateDividendsJob(c).schedule(7000);
}
}

Expand Down
8 changes: 4 additions & 4 deletions name.abuchen.portfolio.ui/src/name/abuchen/portfolio/ui/util/Colors.java
View file
Open in desktop
Expand Up @@ -21,8 +21,8 @@ public static class Theme
private Color defaultForeground = Colors.BLACK;
private Color defaultBackground = Colors.WHITE;
private Color warningBackground = getColor(254, 223, 107); // FEDF6B
private Color redBackground = Colors.GREEN;
private Color greenBackground = Colors.RED;
private Color redBackground = Colors.RED;
private Color greenBackground = Colors.GREEN;
private Color redForeground = Colors.DARK_RED;
private Color greenForeground = Colors.DARK_GREEN;
private Color grayForeground = getColor(112, 112, 112); // 707070
Expand Down Expand Up @@ -225,8 +225,8 @@ public static Color getTextColor(Color color)
{
// http://stackoverflow.com/questions/596216/formula-to-determine-brightness-of-rgb-color

double luminance = 1 - (0.299 * color.getRed() + 0.587 * color.getGreen() + 0.114 * color.getBlue()) / 255;
return luminance < 0.4 ? BLACK : WHITE;
double luminance = (0.299 * color.getRed() + 0.587 * color.getGreen() + 0.114 * color.getBlue()) / 255;
return luminance > 0.55 ? BLACK : WHITE;
}

public static Color brighter(Color base)
Expand Down
8 changes: 5 additions & 3 deletions ...uchen.portfolio.ui/src/name/abuchen/portfolio/ui/views/dashboard/LimitExceededWidget.java
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@
import java.util.function.Supplier;

import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.layout.FormAttachment;
import org.eclipse.swt.layout.FormLayout;
import org.eclipse.swt.widgets.Composite;
Expand Down Expand Up @@ -103,10 +104,11 @@ protected Composite createItemControl(Composite parent, LimitItem item)

// determine colors
LimitPriceSettings settings = new LimitPriceSettings(item.attributeType.getProperties());
price.setBackdropColor(item.limit.getRelationalOperator().isGreater()
Color bgColor = item.limit.getRelationalOperator().isGreater()
? settings.getLimitExceededPositivelyColor(Colors.theme().greenBackground())
: settings.getLimitExceededNegativelyColor(Colors.theme().redBackground()));

: settings.getLimitExceededNegativelyColor(Colors.theme().redBackground());
price.setBackdropColor(bgColor);
price.setForeground(Colors.getTextColor(bgColor));
price.setText(Values.Quote.format(item.getSecurity().getCurrencyCode(), item.price.getValue()));

Label limit = new Label(composite, SWT.NONE);
Expand Down