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

[3.4.4] Fix data convertion for BigInteger values that stored in DB as String #8002

Merged

Conversation

ShvaykaD
Copy link
Contributor

@ShvaykaD ShvaykaD commented Jan 27, 2023

Pull Request description

issue: #8001

General checklist

  • You have reviewed the guidelines document.
  • PR name contains fix version. For example, "[3.3.4] Hotfix of some UI component" or "[3.4] New Super Feature".
  • The milestone is specified and corresponds to fix version.
  • Description references specific issue.
  • Description contains human-readable scope of changes.
  • Description contains brief notes about what needs to be added to the documentation.
  • No merge conflicts, commented blocks of code, code formatting issues.
  • Changes are backward compatible or upgrade script is provided.
  • Similar PR is opened for PE version to simplify merge. Required for internal contributors only.

Front-End feature checklist

  • Screenshots with affected component(s) are added. The best option is to provide 2 screens: before and after changes;
  • If you change the widget or other API, ensure it is backward-compatible or upgrade script is present.
  • Ensure new API is documented here

Back-End feature checklist

  • Added corresponding unit and/or integration test(s). Provide written explanation in the PR description if you have failed to add tests.
  • If new dependency was added: the dependency tree is checked for conflicts.
  • If new service was added: the service is marked with corresponding @TbCoreComponent, @TbRuleEngineComponent, @TbTransportComponent, etc.
  • If new REST API was added: the RestClient.java was updated, issue for Python REST client is created.

@ShvaykaD ShvaykaD added the bug label Jan 27, 2023
@ShvaykaD ShvaykaD added this to the 3.4.4 milestone Jan 27, 2023
@@ -25,6 +25,7 @@ public class EntityDataAdapterTest {
public void testConvertValue() {
assertThat(EntityDataAdapter.convertValue("500")).isEqualTo("500");
assertThat(EntityDataAdapter.convertValue("500D")).isEqualTo("500D"); //do not convert to Double !!!
assertThat(EntityDataAdapter.convertValue("0101010521130565")).isEqualTo("0101010521130565");
assertThat(EntityDataAdapter.convertValue("0101010521130565")).isEqualTo("0101010521130565"); //do not convert to Double !!!
Copy link
Member

Choose a reason for hiding this comment

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

You have added check for "Simple double" that contained ".", but I don't see new tests values that contain ".". When do you return Double as double?

Copy link
Contributor Author

@ShvaykaD ShvaykaD Jan 30, 2023

Choose a reason for hiding this comment

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

Hi @ashvayka,

EntityDataAdapter.convertValue value method always accepts strings as input parameter.

Simple double added to verify that BigInteger value like 89010303310033979663 will not be converted to: 8.901030331003398E19.

Please check the example added in the github issue:

#8001

@ashvayka ashvayka merged commit b22d31d into thingsboard:master Jan 30, 2023
@@ -92,8 +92,9 @@ static String convertValue(Object value) {
}
try {
double dblVal = Double.parseDouble(strVal);
if (!Double.isInfinite(dblVal)) {
return Double.toString(dblVal);
String doubleAsString = Double.toString(dblVal);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey! @ShvaykaD

Look at a more optimal way to identify a double type (have a . char).
Another improvement is to verify the string length to asses for long or double string content (let's say 64 chars)
I hope you can add more use cases for conversion as well.
It is essential code that affects data representation.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants