Skip to content
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

Map scalar type #91

Closed
luisobo opened this issue Oct 29, 2015 · 2 comments
Closed

Map scalar type #91

luisobo opened this issue Oct 29, 2015 · 2 comments

Comments

@luisobo
Copy link

luisobo commented Oct 29, 2015

Hi there,

I'm trying to model something and I might be at the edge of graphql itself. Here is the thing: I have an abstract type Customization and many (dozens) concrete subtypes, each one with its own attributes. This list of subtypes will grow over time.

This is okay to model on the query side, an Customization Interface and many object types, one per subtype, the problem is the mutation side, since input types cannot be abstract (addCustomizationToNode(nodeId, customizationInput)), I have to define one mutation per concrete subtype (addSubtypeAToNode, addSubtypeBToNode) etc.

I opened an issue on graphql but I don't anticipate this being implemented soon or at all. In the mean time I need to solve this problem so I'm trying to implement a MapType that models an Map[String, Any] so I can use it as the input param, escape the type system and check the input attributes at runtime myself.

This is what I got so far:

  val MapType = ScalarType[Map[String, Any]]("Map",
    Some("A standard representation of map"),
    coerceOutput = s => {
      val fields = s.map {
        case (k, v: String) => ast.ObjectField(k, ast.StringValue(v))
        case (k, Some(v: String)) => ast.ObjectField(k, ast.StringValue(v))
      }
      ast.ObjectValue(fields.toList)
    },
    coerceUserInput = {
      case s: Map[String, Any] => Right(s)
      case _ => Left(StringCoercionViolation)
    },
    coerceInput = {
      case ast.ObjectValue(fields, _) => {
        val tuples = fields.map {
          case ast.ObjectField(k, v: ast.StringValue, _) => (k, v.value)
          case ast.ObjectField(k, v: ast.IntValue, _) => (k, v.value)
        }
        Right(tuples.toMap)
      }
      case _ => Left(StringCoercionViolation)
    })

The problem is that I'd be repeating all the value cases for String, Int, yada yada and I was wondering if I could relay on sangria to do that, since it is already implemented, but I couldn't find it.

Any ideas, suggestions? Thank you!

  • Luis
@OlegIlyenko
Copy link
Member

I can feel your pain. In fact, I have similar situation with our API, I guess I add a comment on your issue as well. There is also similar issue in graphql-js repo: graphql/graphql-js#207.

I think it should be possible to implement MapType as you described, it is a bit more involving though. Indeed sangria already has functions process AST nodes for input objects, but they would not be very useful in our case. You need very low-level access to the AST, because you don't want, for instance, StringValue to be coerced to something else (like date, in case you have date scala type). All built-in functions do the raw value coercion and variable resolution, which is most probably not what you want.

Also coerceUserInput will not work the way you defined in the example. It will produce an error if you use MapType for object values, since all input unmarshallers at the moment only allow "scalar" values to be used with custom ScalarTypes (things like Int, String, BigDecimal, etc., but not complex objects or lists). You will need to customize on of the InputUnmarshallers, which you use, all allow JValue or JsValue or any other Node type (depending on the implementation) to be a scalar value (you need to customize getScalarValue and isScalarNode methods).

In general, I would prefer to stick to the spec in this respect and not introduce any custom sangria-specific mechanisms or concepts. On the other hand, I do would like to push input interfaces and input union types as well, so I would suggest to push it together into the spec :)

@luisobo
Copy link
Author

luisobo commented Nov 5, 2015

Thanks for the feedback. I'll see what I can do.

@luisobo luisobo closed this as completed Nov 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants