Wolfgang-Schuetzelhofer / jcypher

Java access to Neo4J graph databases at multiple levels of abstraction
Apache License 2.0
86 stars 15 forks source link

AbstractEmbeddedDBAccess does not support ZonedDateTime properties #58

Open DonCziken opened 4 years ago

DonCziken commented 4 years ago

As in topic when using InMemoryDBAccess or EmbeddedDBAccess its not possible to retrieve properties of such type.

The problem lays within writeLiteral in AbstractEmbeddedDBAccess in both AbstractEmbeddedDBAccess.NodeHolder and AbstractEmbeddedDBAccess.RelationHandler as it simply omtis props with such type

public abstract class AbstractEmbeddedDBAccess implements IDBAccessInit {
[..]
        private static void writeLiteral(String key, Object val, JsonObjectBuilder props) {
            if (val instanceof String)
                props.add(key, val.toString());
            else if (val instanceof Number) {
                if (val instanceof Long)
                    props.add(key, ((Long) val).longValue());
                else if (val instanceof Integer)
                    props.add(key, ((Integer) val).intValue());
                else if (val instanceof Double)
                    props.add(key, ((Double) val).doubleValue());
                else if (val instanceof Float)
                    props.add(key, ((Float) val).floatValue());
            } else if (val instanceof Boolean) {
                props.add(key, ((Boolean) val).booleanValue());
            }
                 }

what I propose to do is to add following:

                        } else if (val instanceof ZonedDateTime) {
                props.add(key, ((ZonedDateTime) val).toString());
            }

and then slightly modify JSONContentHandler so it is capable to smartly parse those stringified dates.

public class JSONContentHandler
[..]
        @Override
    public Object convertContentValue(Object value) {
        if (value instanceof JsonValue) {
            JsonValue val = (JsonValue) value;
            Object ret = null;
            ValueType typ = val.getValueType();
            if (typ == ValueType.NUMBER)
                ret = ((JsonNumber)val).bigDecimalValue();
            else if (typ == ValueType.STRING) {
                ret = tryToParseObjectToDate(((JsonString)val).getString()); // so instead to string first we try to do to date
            } 
                        [..]
      [..]

      public static Object tryToParseObjectToDate(Object val) {
      // Try to parse to date, if failing just fallback to original value
      if (val instanceof String) {
      String potentialDate = (String) val;
      if (24 <= potentialDate.length()
          && potentialDate.length() <= 50) {
        try {
          return ZonedDateTime.parse(potentialDate);
        } catch (DateTimeParseException e) {
          // its not date so just return val as is
        }
      }
    }

    return val;
  }
Wolfgang-Schuetzelhofer commented 4 years ago

Hi, I am sorry that I am currently not very responsive with JCypher, due to heavy workload on customer projects. I would very much appreciate if you could make a pull request with your proposed changes, I will then merge it.

Best regards, Wolfgang

DonCziken commented 4 years ago

Wolfgang,

ok, will do it over the weekend.

cheers, Krzysztof

DonCziken commented 4 years ago

@Wolfgang-Schuetzelhofer

this is actually a bit more complex as https://github.com/Wolfgang-Schuetzelhofer/jcypher/issues/56 had not been solved (and what I did I manually switched dependency in my project).

So doing above and bumping version to >1.7.5 would allow code to handle ZonedDateTime in Query/Graph level, however I am not sure how this would affect Domain/DomainQuery level and if there are any gotchas.

Will try to look at this someday this month.

Wolfgang-Schuetzelhofer commented 4 years ago

Hi Krzysztof,

that's fine for me, whenever you make a pull request, I will have a look at it and merge it.

cheers, Wolfgang