jklingsporn / vertx-jooq

A jOOQ-CodeGenerator to create vertx-ified DAOs and POJOs.
MIT License
384 stars 53 forks source link

java.time.LocalDateTime in JsonObject during update #31

Closed quen2404 closed 6 years ago

quen2404 commented 6 years ago

Hi, i have currently an issue when i'm trying to update one entity. It's a row of a simple SQL table:

-- auto-generated definition
CREATE TABLE channel
(
  uid           INT AUTO_INCREMENT PRIMARY KEY,
  external_id   VARCHAR(255)                                                 NOT NULL,
  extenal_href  VARCHAR(255)                                                 NOT NULL,
  name          VARCHAR(255)                                                 NULL,
  status        ENUM ('ACTIVE', 'SUSPENDED', 'DEACTIVATED') DEFAULT 'ACTIVE' NOT NULL,
  data          LONGTEXT                                                     NULL,
  creation_date DATETIME DEFAULT CURRENT_TIMESTAMP                           NOT NULL
)
  ENGINE = InnoDB;

to use java.time classes, i'm using this option in maven generator configuration:

            <plugin>
                <configuration>
                    <generator>
                        <generate>
                            <!-- ... -->
                            <javaTimeTypes>true</javaTimeTypes>
                        </generate>
                    </generator>
                </configuration>
            </plugin>

and i'm using this custom generator.

Here is my code:

        channelDao.findOneByCondition(CHANNEL.UID.eq(1)).setHandler(result -> {
            if (result.succeeded()) {
                Channel channel = result.result();
                channel.setName("TEST");
                channelDao.update(channel).setHandler(resultUpdate -> {
                    if (!resultUpdate.succeeded()) {
                        LOG.error("UPDATE exception", resultUpdate.cause());
                    }
                });
            } else {
                LOG.error("GET exception", result.cause());
            }
        });

The stack trace:

2018-02-23 14:56:25.461 [vert.x-eventloop-thread-1] ERROR com.actility.thingpark.enterprise.channel.portal.controller.operator.ChannelController - UPDATE exception
java.lang.IllegalStateException: Illegal type in JsonObject: class java.time.LocalDateTime
    at io.vertx.core.json.Json.checkAndCopy(Json.java:210) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.vertx.core.json.JsonArray.add(JsonArray.java:439) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.github.jklingsporn.vertx.jooq.shared.internal.async.AbstractAsyncQueryExecutor.getBindValues(AbstractAsyncQueryExecutor.java:56) ~[vertx-jooq-shared-3.0.0.jar:?]
    at io.github.jklingsporn.vertx.jooq.classic.async.AsyncClassicGenericQueryExecutor.lambda$execute$0(AsyncClassicGenericQueryExecutor.java:38) ~[vertx-jooq-classic-async-3.0.0.jar:?]
    at io.vertx.core.Future.lambda$compose$1(Future.java:265) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.vertx.core.impl.FutureImpl.tryComplete(FutureImpl.java:121) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.vertx.core.impl.FutureImpl.complete(FutureImpl.java:83) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.vertx.core.impl.FutureImpl.handle(FutureImpl.java:147) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.vertx.core.impl.FutureImpl.handle(FutureImpl.java:18) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.vertx.ext.asyncsql.impl.BaseSQLClient.lambda$getConnection$0(BaseSQLClient.java:72) ~[vertx-mysql-postgresql-client-3.5.1.jar:3.5.1]
    at io.vertx.ext.asyncsql.impl.pool.AsyncConnectionPool.lambda$createConnection$0(AsyncConnectionPool.java:69) ~[vertx-mysql-postgresql-client-3.5.1.jar:3.5.1]
    at io.vertx.ext.asyncsql.impl.ScalaUtils$3.apply(ScalaUtils.java:91) ~[vertx-mysql-postgresql-client-3.5.1.jar:3.5.1]
    at io.vertx.ext.asyncsql.impl.ScalaUtils$3.apply(ScalaUtils.java:87) ~[vertx-mysql-postgresql-client-3.5.1.jar:3.5.1]
    at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:60) ~[scala-library-2.12.4.jar:?]
    at io.vertx.ext.asyncsql.impl.VertxEventLoopExecutionContext.lambda$execute$0(VertxEventLoopExecutionContext.java:59) ~[vertx-mysql-postgresql-client-3.5.1.jar:3.5.1]
    at io.vertx.core.impl.ContextImpl.lambda$wrapTask$2(ContextImpl.java:339) ~[vertx-core-3.5.1.jar:3.5.1]
    at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:163) [netty-common-4.1.19.Final.jar:4.1.19.Final]
    at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:404) [netty-common-4.1.19.Final.jar:4.1.19.Final]
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:463) [netty-transport-4.1.19.Final.jar:4.1.19.Final]
    at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:886) [netty-common-4.1.19.Final.jar:4.1.19.Final]
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [netty-common-4.1.19.Final.jar:4.1.19.Final]
    at java.lang.Thread.run(Thread.java:745) [?:1.8.0_92]

I don't understand why it doesn't works. I have no problem in my code when i'm retrieving or creating an entity.

jklingsporn commented 6 years ago

When using the async-driver, all datatypes have to be valid io.vertx.core.json.JsonObject-types (and LocalDateTime isn't). This is not a restriction of vertx-jooq, but of the choice to use JsonObject to fetch results and update params in AsyncSQLClient.

I have no problem in my code when i'm retrieving or creating an entity.

Maybe because those values are null in that case? For a quick test: can you please switch to JDBC to see if it works?

jklingsporn commented 6 years ago

So I dug a bit deeper into the issue and discovered that the AsyncSQLClient already converts some values like this, using ScalaUtils-class:

private static void convertValue(JsonArray array, Object value) {
    if (value == null) {
      array.addNull();
    } else if (value instanceof scala.math.BigDecimal) {
      array.add(value.toString());
    } else if (value instanceof LocalDateTime) {
      array.add(value.toString());
    } else if (value instanceof LocalDate) {
      array.add(value.toString());
    } else if (value instanceof DateTime) {
      array.add(Instant.ofEpochMilli(((DateTime) value).getMillis()));
    } else if (value instanceof UUID) {
      array.add(value.toString());
    } else if (value instanceof scala.collection.mutable.ArrayBuffer) {
      scala.collection.mutable.ArrayBuffer<Object> arrayBuffer = (scala.collection.mutable.ArrayBuffer<Object>) value;
      JsonArray subArray = new JsonArray();
      arrayBuffer.foreach(new AbstractFunction1<Object, Void>() {

        @Override
        public Void apply(Object subValue) {
          convertValue(subArray, subValue);
          return null;
        }

      });
      array.add(subArray);
    } else {
      array.add(value);
    }
  }

So when fetching LocalDateTimes they are automatically converted into Strings in the ResultSet. This is the reason why it works for you when you fetch the data.

Could you try replacing the method getBindValues(Query query) in AbstractAsyncQueryExecutor with

    protected JsonArray getBindValues(Query query) {
        ArrayList<Object> bindValues = new ArrayList<>();
        for (Param<?> param : query.getParams().values()) {
            Object value = convertToDatabaseType(param);
            bindValues.add(value);
        }
        return new JsonArray(bindValues);
    }
quen2404 commented 6 years ago

The value is not null. I will try to use JDBC. But it's not easy to change in my project, a lot of impacts. Ok, thank you for your investigation. Ok, i will clone your project and test with this update.

jklingsporn commented 6 years ago

It would have worked as I suggested if the driver would use java.time.LocalDateTime, but they use org.joda.time.LocalDateTime. There is a pending change request to replace it, but I don't think this will happen anytime soon. I guess I will hook in a type-conversion for the time types in AbstractAsyncQueryExecutor in the next version.

jklingsporn commented 6 years ago

Also I figured out the reason why inserts worked for you: the code for insertReturning inlines the SQL instead of using prepared statements. To me, this is a bug and will also be fixed in the next release.

quen2404 commented 6 years ago

Ok, thank you, I understand. Any idea about the date of the next release ? So, are you recommending me to use the JDBC driver instead?

jklingsporn commented 6 years ago

Either switch or try something like this: AbstractAsyncQueryExecutor replace/add

protected JsonArray getBindValues(Query query) {
        ArrayList<Object> bindValues = new ArrayList<>();
        for (Param<?> param : query.getParams().values()) {
            Object value = convertJodaTimeTypes(convertToDatabaseType(param));
            bindValues.add(value);
        }
        return new JsonArray(bindValues);
    }

    protected Object convertJodaTimeTypes(Object object){
        return object;
    }

And in AsyncClassicGenericQueryExecutor overwrite like this:

@Override
    protected Object convertJodaTimeTypes(Object object){
        if(object instanceof java.time.LocalDateTime){
            java.time.LocalDateTime convert = (java.time.LocalDateTime) object;
            return new org.joda.time.LocalDateTime(convert.getYear(),convert.getMonthValue(),convert.getDayOfMonth(),convert.getHour(),convert.getMinute(),convert.getSecond(),convert.getNano()/1000);
        }
        return object;
    }

This should work -.-

jklingsporn commented 6 years ago

Regarding the release: next week.

quen2404 commented 6 years ago

Ok. I forked your project. I have one question: By default, some tests are not passing because it needs a database access. Could you give me the SQL schema ? Do you want a pull request for this code ?

edit: I forgot to mention that your code works 💯

jklingsporn commented 6 years ago

You can find the SQL schema and credentials in the AsyncDatabaseConfigurationProvider. No need for a PR. Thx, Jens

quen2404 commented 6 years ago

Ok, thank you. Another question related to update, did you try to update an entity with an enum type (always with mauricio/postgresql-async library) ? It seems that an instance of enum class is sent to async lib instead of sending the value.

cala.MatchError: ACTIVE (of class com.actility.thingpark.enterprise.channel.portal.storage.enums.ChannelStatus)
    at com.github.mauricio.async.db.mysql.binary.BinaryRowEncoder.encoderFor(BinaryRowEncoder.scala:88) ~[mysql-async_2.12-0.2.21.jar:0.2.21]
    at com.github.mauricio.async.db.mysql.encoder.PreparedStatementExecuteEncoder.encodeValue(PreparedStatementExecuteEncoder.scala:78) ~[mysql-async_2.12-0.2.21.jar:0.2.21]
    at com.github.mauricio.async.db.mysql.encoder.PreparedStatementExecuteEncoder.encodeValues(PreparedStatementExecuteEncoder.scala:61) ~[mysql-async_2.12-0.2.21.jar:0.2.21]
    at com.github.mauricio.async.db.mysql.encoder.PreparedStatementExecuteEncoder.encode(PreparedStatementExecuteEncoder.scala:39) ~[mysql-async_2.12-0.2.21.jar:0.2.21]
    at com.github.mauricio.async.db.mysql.codec.MySQLOneToOneEncoder.encode(MySQLOneToOneEncoder.scala:75) ~[mysql-async_2.12-0.2.21.jar:0.2.21]
    at com.github.mauricio.async.db.mysql.codec.MySQLOneToOneEncoder.encode(MySQLOneToOneEncoder.scala:35) ~[mysql-async_2.12-0.2.21.jar:0.2.21]
    at io.netty.handler.codec.MessageToMessageEncoder.write(MessageToMessageEncoder.java:88) ~[netty-codec-4.1.19.Final.jar:4.1.19.Final]
    ... 35 more
jklingsporn commented 6 years ago

I've created #32 for this.