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

fix npe when has null value tag field in pojo #598

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix npe when has null value tag field in pojo #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/influxdb/dto/Point.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ private void addFieldByAttribute(final Object pojo, final Field field, final Col
}

if (column.tag()) {
this.tags.put(fieldName, (String) fieldValue);
if (fieldValue != null && !((String) fieldValue).isEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand the null check, but why not adding if empty, that might be on pupose ?

Copy link
Author

Choose a reason for hiding this comment

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

sry I dont know what you mean exactly,can you give some code or example...

Copy link
Collaborator

Choose a reason for hiding this comment

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

the old behavior just added the value of the field, now it is only added if not empty ?? What is wrong with an empty value, this is a API breakage.

Copy link
Author

Choose a reason for hiding this comment

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

influx tag must be a string key and string value pair.if some field has column annotation :

  • This filed not a String value,I think it's just incorrect usage
  • This filed is a String value, but user havn't set or just give it "",i think we just need not to set this tag in Point.

if this field is an empty String,influxdb-java will also throw exception:
unable to parse 'example,tag1= value1=123i 1559035795995000000': missing tag value

Copy link
Author

@flashmouse flashmouse May 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

tag.getValue().replace("", "\\ ");

seems could resolved insert empty tag value.

Copy link
Author

Choose a reason for hiding this comment

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

hi @majst01 tell me if you have any idea, my application will use in production soon, i don't want to maintain another branch

Copy link

Choose a reason for hiding this comment

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

any news?

this.tags.put(fieldName, (String) fieldValue);
}
} else {
this.fields.put(fieldName, fieldValue);
}
Expand Down
27 changes: 27 additions & 0 deletions src/test/java/org/influxdb/dto/PointTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,33 @@ public void testAddFieldsFromPOJOWithTimeColumnNull() throws NoSuchFieldExceptio
pojo.time = null;
}

@Test
public void testAddFieldsFromPOJOWithNullTag() throws NoSuchFieldException, IllegalAccessException {
Pojo pojo = new Pojo();
pojo.booleanObject = true;
pojo.booleanPrimitive = false;
pojo.doubleObject = 2.0;
pojo.doublePrimitive = 3.1;
pojo.integerObject = 32;
pojo.integerPrimitive = 64;
pojo.longObject = 1L;
pojo.longPrimitive = 2L;
pojo.time = Instant.now();

Point p = Point.measurementByPOJO(pojo.getClass()).addFieldsFromPOJO(pojo).build();

Assertions.assertEquals(pojo.booleanObject, p.getFields().get("booleanObject"));
Assertions.assertEquals(pojo.booleanPrimitive, p.getFields().get("booleanPrimitive"));
Assertions.assertEquals(pojo.doubleObject, p.getFields().get("doubleObject"));
Assertions.assertEquals(pojo.doublePrimitive, p.getFields().get("doublePrimitive"));
Assertions.assertEquals(pojo.integerObject, p.getFields().get("integerObject"));
Assertions.assertEquals(pojo.integerPrimitive, p.getFields().get("integerPrimitive"));
Assertions.assertEquals(pojo.longObject, p.getFields().get("longObject"));
Assertions.assertEquals(pojo.longPrimitive, p.getFields().get("longPrimitive"));
Assertions.assertEquals(pojo.time, p.getFields().get("time"));
Assertions.assertEquals(null, p.getTags().get("uuid"));
}

@Test
public void testAddFieldsFromPOJOWithData() throws NoSuchFieldException, IllegalAccessException {
Pojo pojo = new Pojo();
Expand Down