walmartlabs / lacinia

GraphQL implementation in pure Clojure
http://lacinia.readthedocs.io/en/latest/
Other
1.82k stars 160 forks source link

Fixes a runtime error that occurs when deep-merging fragments #462

Closed namenu closed 3 months ago

namenu commented 3 months ago

Fixes a bug where inconsistent results occur based on the fragment's declared order.

I fixed a similar issue in #453, but in that case the fragment was nested, whereas this case is not.

This fails:

query MyQuery {
  node(id: "1000") {
    ... on Post {
      id
      ...PostFragment
    }
  }
}

fragment PostFragment on Post {
  author {
    alwaysFail
  }
  ...PostFragment2   # <--
}

fragment PostFragment2 on Post {
  author {
    name
  }
}

While this is not:

query MyQuery {
  node(id: "1000") {
    ... on Post {
      id
      ...PostFragment
    }
  }
}

fragment PostFragment on Post {
  ...PostFragment2   # <--
  author {
    alwaysFail
  }
}

fragment PostFragment2 on Post {
  author {
    name
  }
}

My guess is that deep-merge doesn't handle the situation where nil comes in instead of map.

hlship commented 3 months ago

I'll review this once you re-open it.

namenu commented 3 months ago

@hlship Thanks. We have encountered a regression bug after applying this changes. I am not confident much so that I can handle this problem since it requires profound understanding of the internal design. I'm still finding the way how to reproduce the bug. If you have any idea about this please let me know.

namenu commented 3 months ago

Reopened #463