jcrygier / graphql-jpa

JPA Implementation of GraphQL (builds on graphql-java)
MIT License
165 stars 46 forks source link

Custom scalar types #52

Open uded opened 6 years ago

uded commented 6 years ago

As far as I was able to see, there is no easy way to introduce new scalar types into your handler. Currently, the method seems to be limited to statically coded in the list of types, as follows:

 private GraphQLType getAttributeType(Attribute attribute) {
        if (attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.BASIC) {
            if (String.class.isAssignableFrom(attribute.getJavaType()))
                return Scalars.GraphQLString;
            else if (UUID.class.isAssignableFrom(attribute.getJavaType()))
                return JavaScalars.GraphQLUUID;
            else if (Integer.class.isAssignableFrom(attribute.getJavaType()) || int.class.isAssignableFrom(attribute.getJavaType()))
                return Scalars.GraphQLInt;
            else if (Short.class.isAssignableFrom(attribute.getJavaType()) || short.class.isAssignableFrom(attribute.getJavaType()))
                return Scalars.GraphQLShort;
            else if (Float.class.isAssignableFrom(attribute.getJavaType()) || float.class.isAssignableFrom(attribute.getJavaType())
                    || Double.class.isAssignableFrom(attribute.getJavaType()) || double.class.isAssignableFrom(attribute.getJavaType()))
                return Scalars.GraphQLFloat;
            else if (Long.class.isAssignableFrom(attribute.getJavaType()) || long.class.isAssignableFrom(attribute.getJavaType()))
                return Scalars.GraphQLLong;
            else if (Boolean.class.isAssignableFrom(attribute.getJavaType()) || boolean.class.isAssignableFrom(attribute.getJavaType()))
                return Scalars.GraphQLBoolean;
            else if (Date.class.isAssignableFrom(attribute.getJavaType()))
                return JavaScalars.GraphQLDate;
            else if (LocalDateTime.class.isAssignableFrom(attribute.getJavaType()))
                return JavaScalars.GraphQLLocalDateTime;
            else if (LocalDate.class.isAssignableFrom(attribute.getJavaType()))
                return JavaScalars.GraphQLLocalDate;
            else if (attribute.getJavaType().isEnum()) {
                return getTypeFromJavaType(attribute.getJavaType());
            } else if (BigDecimal.class.isAssignableFrom(attribute.getJavaType())) {
                return Scalars.GraphQLBigDecimal;
            }
        }
//[...]
}

Please, do correct me if I am wrogn here and misunderstood something.

It would be beneficial to be able to register this way or another new type for which there is an implementation of GraphQLScalarType made. We had several (to say the least) such types in our systems, without them we can't use this library at the current stage of development.

I am happy to help if you will let me, but I would like to know what kind of implementation you would like to see here? We can do it in several different ways IMHO:

  1. Annotation to find and register all classes implementing GraphQLScalarType automatically - it will require a bit of work, but it's doable nevertheless. It would be way easier if this would be a Spring library, not a general library...
  2. Static map or singleton to hold all the types as your application will boot up. In this case, it would enable further extension via annotations based on frameworks and their prefered way of handling them.
  3. ...? Waiting to any proposals...

I think a combination of 2. followed by 1. per framework makes most of sense here...

jcrygier commented 6 years ago

Thanks for the idea, I think it will go a long way on flexibility. I'm leaning towards adding a way to pass additional Custom Scalar types into the constructor...so we stay as 'basic' as possible. So this is decently close to what you're suggesting from bullet 2 above, but not using static members.

That being said, I'd like to introduce a functional interface for handling these things, in an attempt to reduce verbosity. I should get a chance to work on some of this in the near future, but am more than willing to accept pull requests as well.

BTW, agreed on the Spring comment...but I'm trying to ensure this library is usable outside of a Spring environment as much as possible, as it's not really an explicit dependency for what's trying to be done here.

jcrygier commented 6 years ago

I put together something quickly...let me know if it helps w/ your problem. I pushed all of the 'custom' types into this new framework, mostly to show it working.

Let me know if you want to take it in a different direction.

uded commented 6 years ago

I will take a look, but at the first glance, it should work. I am more than happy to add some Spring Boot starter with annotations to make it more automated on that level...