Repository: influxdata/influxdb-java
Pull request #687 (2020-07-01 22:07:54)

Added version of query method
InfluxDBResultMapper was recently updated with this version of .toPojo:
  public <T> List<T> toPOJO(final QueryResult queryResult, final Class<T> clazz, final String measurementName)
      throws InfluxDBMapperException {
    return toPOJO(queryResult, clazz, measurementName, TimeUnit.MILLISECONDS);
  }

Adding  InfluxDBMapper.query(final Query query, final Class<T> clazz, String measurementName) to take advantage of the new toPojo variation to map a db with variable measurement names to a POJO. Without this pull, mapping Query to Pojo without @Measurement annotation requires both an InfluxDB instance to fetch the query result and an influxDBMapper instance to map to call toPojo directly.

codecov-commenter: # [Codecov](https://codecov.io/gh/influxdata/influxdb-java/pull/687?src=pr&el=h1) Report
> Merging [#687](https://codecov.io/gh/influxdata/influxdb-java/pull/687?src=pr&el=desc) into [master](https://codecov.io/gh/influxdata/influxdb-java/commit/01979bfa19d6ee2c05445ca2fbb201eda74593e7&el=desc) will **decrease** coverage by `0.07%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/influxdata/influxdb-java/pull/687/graphs/tree.svg?width=650&height=150&src=pr&token=QKip8tza8W)](https://codecov.io/gh/influxdata/influxdb-java/pull/687?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##             master     #687      +/-   ##
============================================
- Coverage     88.18%   88.11%   -0.08%     
  Complexity      722      722              
============================================
  Files            69       69              
  Lines          2513     2515       +2     
  Branches        268      268              
============================================
  Hits           2216     2216              
- Misses          208      210       +2     
  Partials         89       89              
```


| [Impacted Files](https://codecov.io/gh/influxdata/influxdb-java/pull/687?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...rc/main/java/org/influxdb/impl/InfluxDBMapper.java](https://codecov.io/gh/influxdata/influxdb-java/pull/687/diff?src=pr&el=tree#diff-c3JjL21haW4vamF2YS9vcmcvaW5mbHV4ZGIvaW1wbC9JbmZsdXhEQk1hcHBlci5qYXZh) | `85.71% <0.00%> (-2.53%)` | `18.00 <0.00> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/influxdata/influxdb-java/pull/687?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/influxdata/influxdb-java/pull/687?src=pr&el=footer). Last update [01979bf...603ee10](https://codecov.io/gh/influxdata/influxdb-java/pull/687?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Ahimsaka: Alright well, I don't know how to fix the Codecov situation but as long as the new toPojo signature has coverage this commit should be a-okay. Any humans have any questions or concerns, I am more than happy to talk :)

And fwiw, I have personally seen this Codecov character hold up a liquor store at gunpoint, so I don't know who you really want to be trusting around here. 
majst01: Codecov complains the fact that you added a new public facing method without adding a test. So overall this looks fine, but if you dont mind please add a Simple unit test for this new signature and a a CHANGELOG.md entry
Ahimsaka: Is there a reason why this signature needs a unit test but InfluxDBMapper.query(Class clazz) doesn't?

I'm not an experienced programmer by any means, but to me it would seem redundant to test a method that doesn't contain any new logic, and only calls 2 methods that are already tested elsewhere. 

I any case, I thought it would be more helpful to write a few lines myself than to open an issue to complain when I noticed that somebody forgot to update this method when they added the new-ish toPojo signature. I've already adjusted my code to work around that oversight, so I don't mind if you decide to close this pull request. Maybe I'll look into it again when I'm finished with my project and have a little more time. 
majst01: If the existing method doesnt have a unit test is this bad as well, i will ensure that i dont get any new functionality without coverage
Ahimsaka: As long as it's consistent, that's perfectly reasonable. I apologize for the impolite response. This is a very helpful library and I am grateful to you for maintaining it. 

I went ahead and added the test for the new InfluxDBMapper.query signature, and also for the existing signature that I didn't see tested previously. It looks like it has passed the CI tests again, and appeased codecov. Please let me know if there are any other issues and I'll do my best to correct them.
majst01: Hi, sorry for the delay, can you please fix the merge conflict, i will merge then
Ahimsaka: Merge conflict in Changelog is resolved
Ahimsaka: Test that failed is unrelated to the changes in this pull request and only failed in one out of 9 builds. 

[ERROR] Failures: 
[ERROR]   MessagePackBatchOptionsTest>BatchOptionsTest.testJitterDuration:250 expected: <null> but was: <[Series [name=weather, tags=null, columns=[time, humidity, temperature, uv_index], values=[[0, 0.0, 0.0, moderate], [3600000000000, 1.1, 1.0, moderate], [7200000000000, 2.2, 2.0, moderate], [10800000000000, 3.3000000000000003, 3.0, moderate], [14400000000000, 4.4, 4.0, moderate], [18000000000000, 5.5, 5.0, moderate], [21600000000000, 6.6000000000000005, 6.0, moderate], [25200000000000, 7.700000000000001, 7.0, moderate], [28800000000000, 8.8, 8.0, moderate], [32400000000000, 9.9, 9.0, moderate], [36000000000000, 11.0, 10.0, moderate], [39600000000000, 12.100000000000001, 11.0, moderate], [43200000000000, 13.200000000000001, 12.0, moderate], [46800000000000, 14.3, 13.0, moderate], [50400000000000, 15.400000000000002, 14.0, moderate], [54000000000000, 16.5, 15.0, moderate], [57600000000000, 17.6, 16.0, moderate], [61200000000000, 18.700000000000003, 17.0, moderate], [64800000000000, 19.8, 18.0, moderate], [68400000000000, 20.900000000000002, 19.0, moderate]]]]>
[INFO] 
[ERROR] Tests run: 376, Failures: 1, Errors: 0, Skipped: 3

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 534e59a..e6226c1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,11 @@
 
 ### Features
 - Add an option in `BatchOption` to prevent `InfluxDB#write` from blocking when actions queue is exhausted. [Issue #668](https://github.com/influxdata/influxdb-java/issues/688)
+- Added new signature to InfluxDBMapper.query() with params final Query query, final Class<T> clazz, final String measurementName to leverage InfluxDBResultMapper.toPojo method with identical signature.
 
+### Improvements
+
+- Test: Added test for new InfluxDBMapper.query() signature, as well as test for existing InfluxDBMapper.query(Class clazz) signature (previously only InfluxDBMapper.query(Query query, Class clazz) was tested).
 
 ## 2.19 [2020-05-18]
 
diff --git a/src/main/java/org/influxdb/impl/InfluxDBMapper.java b/src/main/java/org/influxdb/impl/InfluxDBMapper.java
index 73a7475..700a960 100644
--- a/src/main/java/org/influxdb/impl/InfluxDBMapper.java
+++ b/src/main/java/org/influxdb/impl/InfluxDBMapper.java
@@ -21,6 +21,11 @@ public class InfluxDBMapper extends InfluxDBResultMapper {
     this.influxDB = influxDB;
   }
 
+  public <T> List<T> query(final Query query, final Class<T> clazz, final String measurementName) {
+    QueryResult queryResult = influxDB.query(query);
+    return toPOJO(queryResult, clazz, measurementName);
+  }
+
   public <T> List<T> query(final Query query, final Class<T> clazz) {
     throwExceptionIfMissingAnnotation(clazz);
     QueryResult queryResult = influxDB.query(query);


Test patch:
diff --git a/src/test/java/org/influxdb/impl/InfluxDBMapperTest.java b/src/test/java/org/influxdb/impl/InfluxDBMapperTest.java
index 1bdb1e5..1246054 100644
--- a/src/test/java/org/influxdb/impl/InfluxDBMapperTest.java
+++ b/src/test/java/org/influxdb/impl/InfluxDBMapperTest.java
@@ -55,6 +55,24 @@ public class InfluxDBMapperTest {
     Assert.assertTrue(persistedMeasures.size()>0);
   }
 
+  @Test
+  public void testQueryWhenCalledWithClassOnly() {
+    ServerMeasure serverMeasure = createMeasure();
+    influxDBMapper.save(serverMeasure);
+
+    List<ServerMeasure> persistedMeasures = influxDBMapper.query(ServerMeasure.class);
+    Assert.assertTrue(persistedMeasures.size()>0);
+  }
+
+  @Test
+  public void testQueryWhenCalledWithQuery_Class_MeasurementName() {
+    ServerMeasure serverMeasure = createMeasure();
+    influxDBMapper.save(serverMeasure);
+
+    List<NonAnnotatedServerMeasure> persistedMeasures = influxDBMapper.query(new Query("SELECT * FROM server_measure",UDP_DATABASE), NonAnnotatedServerMeasure.class, "server_measure");
+    Assert.assertTrue(persistedMeasures.size()>0);
+  }
+
   @Test
   public void testIllegalField() {
     InvalidMeasure invalidMeasure = new InvalidMeasure();
@@ -185,6 +203,87 @@ public class InfluxDBMapperTest {
     }
   }
 
+  static class NonAnnotatedServerMeasure {
+
+    /** Check the instant conversions */
+    @Column(name = "time")
+    private Instant time;
+
+    @Column(name = "name", tag = true)
+    private String name;
+
+    @Column(name = "cpu")
+    private double cpu;
+
+    @Column(name = "healthy")
+    private boolean healthy;
+
+    @Column(name = "min")
+    private long uptime;
+
+    @Column(name = "memory_utilization")
+    private Double memoryUtilization;
+
+    @Column(name = "ip")
+    private String ip;
+
+    public Instant getTime() {
+      return time;
+    }
+
+    public void setTime(Instant time) {
+      this.time = time;
+    }
+
+    public String getName() {
+      return name;
+    }
+
+    public void setName(String name) {
+      this.name = name;
+    }
+
+    public double getCpu() {
+      return cpu;
+    }
+
+    public void setCpu(double cpu) {
+      this.cpu = cpu;
+    }
+
+    public boolean isHealthy() {
+      return healthy;
+    }
+
+    public void setHealthy(boolean healthy) {
+      this.healthy = healthy;
+    }
+
+    public long getUptime() {
+      return uptime;
+    }
+
+    public void setUptime(long uptime) {
+      this.uptime = uptime;
+    }
+
+    public Double getMemoryUtilization() {
+      return memoryUtilization;
+    }
+
+    public void setMemoryUtilization(Double memoryUtilization) {
+      this.memoryUtilization = memoryUtilization;
+    }
+
+    public String getIp() {
+      return ip;
+    }
+
+    public void setIp(String ip) {
+      this.ip = ip;
+    }
+  }
+
   @Measurement(name = "invalid_measure", database = UDP_DATABASE)
   static class InvalidMeasure {