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

Commit on not changed Date filed causes creating a snapshot (and commit row) #208

Closed
floresek opened this Issue Sep 25, 2015 · 10 comments

Comments

Projects
None yet
2 participants
@floresek

floresek commented Sep 25, 2015

Hi,

I noticed that every commit (without audited object changes) on Date/Timestamp field cases creation of timestamp.
Use following code to reproduce the bug.

class SimpleTypes {
    @Id
    String id;

    enum TestEnum { ONE, TWO }

    Date date = new Date();
    java.sql.Date dateSql = new java.sql.Date(date.getTime());
    Timestamp ts = new Timestamp(date.getTime());
    String text = "test";
    Boolean bool = true;
    Long longNumber = 1l;
    Integer integerNumber = 1;
    Short shortNumber = 1;
    Double doubleNumber = 1.0;
    Float floatNumber = 1.0f;
    BigDecimal bigDecimalNumber = BigDecimal.ONE;
    byte byteFiled = 0x1;
    TestEnum testEnum = TestEnum.ONE;

    public SimpleTypes(String id) {
        this.id = id;
    }
}

        ...
        SimpleTypes obj = new SimpleTypes("1");
        javers.commit("anonymous", obj);
        javers.commit("anonymous", obj);
        javers.commit("anonymous", obj);
        List<Change> changes = javers
                .findChanges(QueryBuilder.byInstanceId("1", SimpleTypes.class)
                        .withNewObjectChanges(true)
                        .build());
        String changeLog = javers.processChangeList(changes, getChangeLogger());
        log.info("\n{}", changeLog);
        ...

Cheers
Mariusz

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Sep 30, 2015

are you using java.sql.Timestamp and java.util.Date ?

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Sep 30, 2015

I dont see nothing wrong here, I got 15 changes (14 ValueChanges + 1 NewObject change), all bounded to the first commit - Commit 1.00

@floresek

This comment has been minimized.

floresek commented Oct 1, 2015

Yes, I tested with:

  • java.sql.Timestamp
  • java.util.Date
  • java.sql.Date

I ran the test with 1.3.10 javers version and java: jdk1.7.0_79 (64 bit)
What java version do you use?

Below I attached exported 3 rows from jv_snapshot, which were created during commits:

[{
  "snapshot_pk": 1300,
  "type": "INITIAL",
  "state": "{  "bool": true,  "bigDecimalNumber": 1,  "longNumber": 1,  "doubleNumber": 1.0,  "integerNumber": 1,  "id": "1",  "byteFiled": 1,  "floatNumber": 1.0,  "date": "2015-10-01T08:13:18+0200",  "dateSql": "2015-10-01T08:13:18+0200",  "shortNumber": 1,  "testEnum": "ONE",  "text": "test",  "ts": "2015-10-01T08:13:18+0200"}",
  "changed_properties": "[  "bool",  "bigDecimalNumber",  "longNumber",  "doubleNumber",  "integerNumber",  "id",  "byteFiled",  "floatNumber",  "date",  "dateSql",  "shortNumber",  "testEnum",  "text",  "ts"]",
  "global_id_fk": 1100,
  "commit_fk": 1400
 },
 {
  "snapshot_pk": 1301,
  "type": "UPDATE",
  "state": "{  "bool": true,  "bigDecimalNumber": 1,  "longNumber": 1,  "doubleNumber": 1.0,  "integerNumber": 1,  "id": "1",  "byteFiled": 1,  "floatNumber": 1.0,  "date": "2015-10-01T08:13:18+0200",  "dateSql": "2015-10-01T08:13:18+0200",  "shortNumber": 1,  "testEnum": "ONE",  "text": "test",  "ts": "2015-10-01T08:13:18+0200"}",
  "changed_properties": "[  "date",  "dateSql",  "ts"]",
  "global_id_fk": 1100,
  "commit_fk": 1401
 },
 {
  "snapshot_pk": 1302,
  "type": "UPDATE",
  "state": "{  "bool": true,  "bigDecimalNumber": 1,  "longNumber": 1,  "doubleNumber": 1.0,  "integerNumber": 1,  "id": "1",  "byteFiled": 1,  "floatNumber": 1.0,  "date": "2015-10-01T08:13:18+0200",  "dateSql": "2015-10-01T08:13:18+0200",  "shortNumber": 1,  "testEnum": "ONE",  "text": "test",  "ts": "2015-10-01T08:13:18+0200"}",
  "changed_properties": "[  "date",  "dateSql",  "ts"]",
  "global_id_fk": 1100,
  "commit_fk": 1402
 }
]
@floresek

This comment has been minimized.

floresek commented Oct 1, 2015

Also exists running with jdk1.8.0_60 (64 bit)

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 1, 2015

I need a failing test with repro of this case.
For now, the test I've wrote, passes on my side ...

package org.javers.core.cases;

import org.fest.assertions.api.Assertions;
import org.javers.core.Javers;
import org.javers.core.JaversBuilder;
import org.javers.core.diff.Change;
import org.javers.repository.jql.QueryBuilder;
import org.junit.Test;

import javax.persistence.Id;
import java.math.BigDecimal;
import java.sql.Date;
import java.sql.Timestamp;
import java.util.List;


/**
 * see https://github.com/javers/javers/issues/213
 * @author bartosz.walacik
 */
public class Case208 {

    @Test
    public void shouldCalculateChangedPropertyNamesForNullifiedValues() {
        //given
        Javers javers = JaversBuilder.javers().build();
        SimpleTypes obj = new SimpleTypes("1");

        //when
        javers.commit("anonymous", obj);
        javers.commit("anonymous", obj);
        javers.commit("anonymous", obj);

        List<Change> changes = javers.findChanges(QueryBuilder.byInstanceId("1", SimpleTypes.class).andProperty("date")
                .withNewObjectChanges(true)
                .build());

        //then
        Assertions.assertThat(changes).hasSize(1);
        Assertions.assertThat(changes.get(0).getCommitMetadata().get().getId().getMajorId()).isEqualTo(1);

    }

    static class SimpleTypes {
        @Id
        String id;

        enum TestEnum { ONE, TWO }

        Date date = new Date(new java.util.Date().getTime());
        java.sql.Date dateSql = new java.sql.Date(date.getTime());
        Timestamp ts = new Timestamp(date.getTime());
        String text = "test";
        Boolean bool = true;
        Long longNumber = 1l;
        Integer integerNumber = 1;
        Short shortNumber = 1;
        Double doubleNumber = 1.0;
        Float floatNumber = 1.0f;
        BigDecimal bigDecimalNumber = BigDecimal.ONE;
        byte byteFiled = 0x1;
        TestEnum testEnum = TestEnum.ONE;

        public SimpleTypes(String id) {
            this.id = id;
        }

        public void nullify() {
            date = null;
            ts = null;
            text = null;
            bool = null;
            longNumber = null;
            integerNumber = null;
            shortNumber = null;
            doubleNumber = null;
            floatNumber = null;
            bigDecimalNumber = null;
            testEnum = null;
        }
    }

}

@floresek

This comment has been minimized.

floresek commented Oct 2, 2015

The test you attached also passes on my side - but when you provide sql repository you can see, that with every commit a snapshot is created with date/time columns reported as changed.

I used sql (Postgres) repository created with following code:

    public static Javers createSqlRepository(final Connection dbConnection, final DialectName dialectName) throws SQLException {
        ConnectionProvider connectionProvider = new ConnectionProvider() {
            @Override
            public Connection getConnection() {
                return dbConnection;
            }
        };

        JaversSqlRepository sqlRepository = SqlRepositoryBuilder
                .sqlRepository()
                .withConnectionProvider(connectionProvider)
                .withDialect(dialectName)
                .build();

        return JaversBuilder.javers()
                .registerJaversRepository(sqlRepository)
                .build();
    }

With such repository 3 snaphots is returned by:

List<CdoSnapshot> snapshoths = javers.findSnapshots(QueryBuilder.byInstanceId("1", SimpleTypes.class)
                .withNewObjectChanges(true)
                .build());
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 2, 2015

looks like there is a problem with timezones serialization/deserialization

@bartoszwalacik bartoszwalacik added bug and removed question labels Oct 2, 2015

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 2, 2015

actually, it's a problem with millis, in the new version, all datetimes will be serialized with millis, using ISO datetime format yyyy-MM-dd'T'HH:mm:ss.SSS

bartoszwalacik added a commit that referenced this issue Oct 2, 2015

bartoszwalacik added a commit that referenced this issue Oct 2, 2015

bartoszwalacik added a commit that referenced this issue Oct 3, 2015

@bartoszwalacik bartoszwalacik added the fixed label Oct 3, 2015

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Oct 3, 2015

fixed in 1.3.12

Added support for legacy date types: java.util.Date, java.sql.Date, java.sql.Timestamp and java.sql.Time.
Added milliseconds to JSON datetime format.
All local datetimes are now serialized using ISO format yyyy-MM-dd'T'HH:mm:ss.SSS.

@floresek

This comment has been minimized.

floresek commented Oct 6, 2015

Thanks, works perfectly now ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment