influxdata / influxdb-java

Java client for InfluxDB
MIT License
1.18k stars 478 forks source link

InfluxDBResultMapper doesnt call POJO setter #670

Open sjoshid opened 4 years ago

sjoshid commented 4 years ago

It is puzzling that InfluxDBResultMapper doesnt call POJO setter. My POJO looks like

class MyPOJO
{
    @Column(name = "time")
    private Instant time;

    //getter ignored
    public void setTime(Instant time) {  
        this.time = time.truncatedTo(ChronoUnit.SECONDS);
    }
}

Then I do something like

    final List<MyPOJO> resultEvents =  resultMapper.toPOJO(result, MyPOJO.class);

Assumption is setTime would be called for each point fetched. But it doesnt. It somehow bypasses it. time is private. Reflection in play here? Expectation is that if I remove my setter, toPOJO call would fail. But it doesnt.

I have tried 2.17, 2.7 and 2.9 versions.

majst01 commented 4 years ago

Hi, please describe what problem are you facing, the questions above are more regarding implementation details of influxdb-java which can be seen in the source code obviously.

sjoshid commented 4 years ago

@majst01 Im trying to truncate (to seconds) the Instant I received from Influx. If setTime isnt called, I cant do this. I can iterate over the result of toPOJO but that will lead to other....nasty changes on my side. Im trying to avoid those.

fmachado commented 4 years ago

@sjoshid yes, you are right: setters and field visibility (default, private, protected or public) are ignored. The reason why I decided to ignore accessory methods were (1) simplicity and (2) the idea was to have a real mapping of the data stored.

Particularly I think it's a bad practice to pass a parameter to a setter (e.g. "foobar") and have it changed to something else (e.g. "foo") but I understand this is allowed by some ORMs and NoSQL drivers and used in some cases.

InfluxDB supports precisions different than ns like m (minutes) or s seconds. Can you store your points with one of these?

You are also welcome to contribute add this feature to the project. Create a PR and we'll review it. :)

sjoshid commented 4 years ago

@fmachado I agree it is a bad practice. And so is not following field visibility policies. No? Keeping aside my reasons to do this, we should follow getter/setter and leave the rest to end users. It's up to them to follow good or bad policies. I'll try to work on this and open a PR. Thank you.

fmachado commented 4 years ago

I'm closing this ticket. Feel free to create your PR and we'll reopen this one.

John9570 commented 3 years ago

I'd also like this functionality, whats the status on the review for this? This would be a super useful feature