Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Implemented
    • Affects Version/s: 3.1M2
    • Fix Version/s: 4.0.M3
    • Component/s: Database integration
    • Labels:
      None

      Description

      Here is some code that we wrote at Swrve to load/save jodatime DateTime objects with Cayenne

      1. 0001-Add-support-for-joda-time-attributes.patch
        19 kB
        John Huss
      2. 0002-Add-support-for-joda-time-attributes.patch
        31 kB
        John Huss
      3. 0003-CAY-1626-Add-JodaTime-DateTime-support.patch
        30 kB
        John Huss
      4. DateTimeType.java
        3 kB
        Marc O'Morain
      5. joda-patch.txt
        18 kB
        John Huss

        Activity

        Marc O'Morain created issue -
        Marc O'Morain made changes -
        Field Original Value New Value
        Attachment DateTimeType.java [ 12497671 ]
        John Huss made changes -
        Comment [ I'm trying to use this with Postgresql and it's not working. Are you defining the column as a varchar or as a timestamp? With postgres it doesn't like you using statement.setString to set a timestamp field. And for reading the value, it doesn't parse because the timezone offset is appended to the string returned by rs.getString; and even without that, that string only contains the value down to the second, omitting the milliseconds. ]
        Hide
        John Huss added a comment -

        The attached file has some issues:

        • The string format of the timestamp is database specific so it can't be relied upon. For example with Postgres this doesn't work. To avoid problems with the driver changing the time zone you should modify the driver or just change the default time zone of your app to use GMT.
        • Using a string with this format limits the precision of the time value since milliseconds are only given to 1 decimal place.

        I'm interested in the Joda time library myself, so I took a stab at doing this and also added support for some of the other useful joda classes.
        So my patch has support for: DateTime, LocalDate, and LocalTime. It also includes the custom serializers necessary for using these classes in an ROP app. The patch has new classes only. The new extended types and serializers aren't registered anywhere currently, so the user has to do that manually.

        Show
        John Huss added a comment - The attached file has some issues: The string format of the timestamp is database specific so it can't be relied upon. For example with Postgres this doesn't work. To avoid problems with the driver changing the time zone you should modify the driver or just change the default time zone of your app to use GMT. Using a string with this format limits the precision of the time value since milliseconds are only given to 1 decimal place. I'm interested in the Joda time library myself, so I took a stab at doing this and also added support for some of the other useful joda classes. So my patch has support for: DateTime, LocalDate, and LocalTime. It also includes the custom serializers necessary for using these classes in an ROP app. The patch has new classes only. The new extended types and serializers aren't registered anywhere currently, so the user has to do that manually.
        John Huss made changes -
        Attachment joda-patch.txt [ 12501163 ]
        John Huss made changes -
        Attachment joda-patch.txt [ 12501163 ]
        Hide
        John Huss added a comment -

        Updated patch with some bug fixes

        Show
        John Huss added a comment - Updated patch with some bug fixes
        John Huss made changes -
        Attachment joda-patch.txt [ 12516739 ]
        Hide
        John Huss added a comment -

        Patch to add joda support now prepped for inclusion - please review.

        Show
        John Huss added a comment - Patch to add joda support now prepped for inclusion - please review.
        John Huss made changes -
        Hide
        Andrus Adamchik added a comment -

        Looks pretty good. I like the idea to a use a Module to ensure it is entirely optional. A few notes:

        1. Joda dependency scope should probably be "provided" at framework/cayenne-core-unpublished/pom.xml. Doesn't matter much probablyas it is repackaged into cayenne-server eventually, but just to underscore that this is an extension.

        2. Let's use SERVER_DEFAULT_TYPES_LIST instead of SERVER_USER_TYPES_LIST in the JodaModule - after all these are framework classes.

        3. Maybe we can improve on 'isJodaAvailable' check in CommonsJdbcEventLogger ? Wonder if we can rewrite the whole 'sqlLiteralForObject' method and instead of guessing the print format of each object type, we take it from say ExtendedType. Extended types can be injected, like we are injecting them to DbAdapters.

        Show
        Andrus Adamchik added a comment - Looks pretty good. I like the idea to a use a Module to ensure it is entirely optional. A few notes: 1. Joda dependency scope should probably be "provided" at framework/cayenne-core-unpublished/pom.xml. Doesn't matter much probablyas it is repackaged into cayenne-server eventually, but just to underscore that this is an extension. 2. Let's use SERVER_DEFAULT_TYPES_LIST instead of SERVER_USER_TYPES_LIST in the JodaModule - after all these are framework classes. 3. Maybe we can improve on 'isJodaAvailable' check in CommonsJdbcEventLogger ? Wonder if we can rewrite the whole 'sqlLiteralForObject' method and instead of guessing the print format of each object type, we take it from say ExtendedType. Extended types can be injected, like we are injecting them to DbAdapters.
        Hide
        Andrus Adamchik added a comment -

        4. Also we probably need at least basic unit tests

        Show
        Andrus Adamchik added a comment - 4. Also we probably need at least basic unit tests
        Hide
        John Huss added a comment -

        Good thoughts. Regarding point 3 (rewriting sqlLiteralForObject) we can break up the existing switch statement by adding a new method to ExtendedType that each type can override. But the strings generated by those methods are still basically guesswork, although it doesn't matter much because these are only used for writing the sql debug log.

        Show
        John Huss added a comment - Good thoughts. Regarding point 3 (rewriting sqlLiteralForObject) we can break up the existing switch statement by adding a new method to ExtendedType that each type can override. But the strings generated by those methods are still basically guesswork, although it doesn't matter much because these are only used for writing the sql debug log.
        Hide
        John Huss added a comment -

        Updated patch following feedback. I haven't made any changes to sqlLiteralForObject. If we want to do that, I think it should go in a separate commit / task.

        Show
        John Huss added a comment - Updated patch following feedback. I haven't made any changes to sqlLiteralForObject. If we want to do that, I think it should go in a separate commit / task.
        John Huss made changes -
        Hide
        John Huss added a comment -

        Updated patch. No code changes, just re-integrated with current trunk. This is ready for review again.

        Show
        John Huss added a comment - Updated patch. No code changes, just re-integrated with current trunk. This is ready for review again.
        John Huss made changes -
        Hide
        John Huss added a comment -

        This has been outstanding for a long time. Any objections to merging it after rebasing it on master?

        Since Java 8 support is still a ways out and many people (me) have existing code using joda-time this is still a useful feature.

        Show
        John Huss added a comment - This has been outstanding for a long time. Any objections to merging it after rebasing it on master? Since Java 8 support is still a ways out and many people (me) have existing code using joda-time this is still a useful feature.
        Savva Kolbachev made changes -
        Assignee Savva Kolbachev [ savvakolbachev ]
        Savva Kolbachev made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Savva Kolbachev made changes -
        Status In Progress [ 3 ] Closed [ 6 ]
        Fix Version/s 4.0.M3 [ 12327645 ]
        Resolution Implemented [ 10 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        1308d 16h 21m 1 Savva Kolbachev 05/May/15 11:11
        In Progress In Progress Closed Closed
        1d 3h 37m 1 Savva Kolbachev 06/May/15 14:48

          People

          • Assignee:
            Savva Kolbachev
            Reporter:
            Marc O'Morain
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development