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.

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

BeanMapper unable to resolve BigInteger #182

Closed
Sxtanna opened this issue Jan 11, 2021 · 3 comments
Closed

BeanMapper unable to resolve BigInteger #182

Sxtanna opened this issue Jan 11, 2021 · 3 comments
Milestone

Comments

@Sxtanna
Copy link
Contributor

Sxtanna commented Jan 11, 2021

It took me quite some time to figure out why this was just not working.

Upon export, the bean mapper correctly stores the values in the map as BigInteger. This map is then again able to resolve back to the object.

The issue arises when the mapper pulls values from a config, where numbers are stored as primitive values.

Properties like BigInteger are loaded as Integer, the mapper fails to convert it to the object, so it just returns null.

example:

class Range {
    BigInteger min;
    BigInteger max;
    // bean stuff
}
@ljacqu
Copy link
Member

ljacqu commented Jan 12, 2021

Thanks for the report! I can gladly add support for BigDecimal and BigInteger in the upcoming version. Probably defaulting to exporting it as a text (rather than an integer) if you agree since the advantage of those types is that they can represent numbers with "arbitrary" precision, which would get lost using the default numerical data types.

For the time being, there is a workaround, although it is a bit verbose. You can extend the default mapper to teach it how to handle BigInteger values. But I'll definitely support BigDecimal & BigInteger in the next version out of the box. :)

public class BeanWithBigIntegerTest {

    @TempDir
    public Path tempDir;

    @Test
    void shouldLoadMap() throws IOException {
        // given
        String yaml = "rangeByName:"
            + "\n  growth:"
            + "\n    min: 5"
            + "\n    max: 10"
            + "\n  depth:"
            + "\n    min: 1"
            + "\n    max: 10"
            + "\n  speed:"
            + "\n    min: 2"
            + "\n    max: 7";
        Path tempFile = TestUtils.createTemporaryFile(tempDir);
        Files.write(tempFile, yaml.getBytes());

        // when
        SettingsManager settingsManager = SettingsManagerBuilder.withYamlFile(tempFile)
            .configurationData(MyTestSettings.class)
            .create();

        // then
        RangeCollection ranges = settingsManager.getProperty(MyTestSettings.RANGES);
        assertThat(ranges.getRangeByName().keySet(), contains("growth", "depth", "speed"));
        assertThat(ranges.getRangeByName().get("growth"), equalTo(new Range(5, 10)));
        assertThat(ranges.getRangeByName().get("depth"), equalTo(new Range(1, 10)));
        assertThat(ranges.getRangeByName().get("speed"), equalTo(new Range(2, 7)));
    }

    public static final class MyTestSettings implements SettingsHolder {

        public static final Property<RangeCollection> RANGES =
            new BeanProperty<>(RangeCollection.class, "", new RangeCollection(), new MapperWithBigIntSupport());

        private MyTestSettings() {
        }
    }

    public static final class MapperWithBigIntSupport extends MapperImpl {

        MapperWithBigIntSupport() {
            super(new BeanDescriptionFactoryImpl(),
                new CombiningLeafValueHandler(StandardLeafValueHandlers.getDefaultLeafValueHandler(),
                    new BigIntegerLeafValueHandler()));
        }
    }

    public static final class BigIntegerLeafValueHandler extends AbstractLeafValueHandler {

        @Override
        protected Object convert(Class<?> clazz, Object value) {
            if (clazz.equals(BigInteger.class) && value instanceof Number) {
                return BigInteger.valueOf(((Number) value).longValue());
            }
            return null;
        }

        @Nullable
        @Override
        public Object toExportValue(@Nullable Object value) {
            if (value instanceof BigInteger) {
                return ((BigInteger) value).longValue();
            }
            return null;
        }
    }

    public static class RangeCollection {

        private Map<String, Range> rangeByName;

        public Map<String, Range> getRangeByName() {
            return rangeByName;
        }

        public void setRangeByName(Map<String, Range> rangeByName) {
            this.rangeByName = rangeByName;
        }
    }

    public static class Range {

        private BigInteger min;
        private BigInteger max;

        public Range() {
        }

        public Range(int min, int max) {
            this.min = BigInteger.valueOf(min);
            this.max = BigInteger.valueOf(max);
        }

        public BigInteger getMin() {
            return min;
        }

        public void setMin(BigInteger min) {
            this.min = min;
        }

        public BigInteger getMax() {
            return max;
        }

        public void setMax(BigInteger max) {
            this.max = max;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;
            Range range = (Range) o;
            return Objects.equals(min, range.min)
                && Objects.equals(max, range.max);
        }

        @Override
        public int hashCode() {
            return Objects.hash(min, max);
        }
    }
}

@ljacqu
Copy link
Member

ljacqu commented Jan 13, 2021

Additional input: throw an exception if a type is not supported? Current behavior with BigInteger is confusing since it will "silently" return null but exports OK

ljacqu added a commit that referenced this issue Feb 6, 2021
ljacqu added a commit that referenced this issue Apr 19, 2021
- Add default support for BigInteger and BigDecimal to the bean mapper
- Change test demonstrating custom type support to use custom class instead of BigInteger, which is now supported out of the box
@ljacqu
Copy link
Member

ljacqu commented Apr 19, 2021

Minor breaking changes:

  • Leaf value handlers are each in their own class now and have been renamed from [Type]Handler to [Type]LeafValueHandler, e.g. StandardLeafValueHandlers.BooleanHandler is now BooleanLeafValueHandler
  • NumberLeafValueHandler will no longer export values for any Number types it does not support

@ljacqu ljacqu closed this as completed Apr 19, 2021
ljacqu added a commit that referenced this issue Apr 20, 2021
…scientific notation

Address remarks by @Sxtanna:
- Do not export values of a type that cannot be imported (BigDecimal and BigInteger are not final)
- Increase value threshold determining whether scientific notation should be used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants