strawberry-graphql / strawberry-sqlalchemy

A SQLAlchemy Integration for strawberry-graphql
MIT License
91 stars 26 forks source link

Not all items in a relationship are loaded #2

Closed yasoob closed 2 years ago

yasoob commented 2 years ago

Hi :wave:

I really like what you have here. However, when I was trying to use it I came across some issues. Primarily, not all items in a relationship are being returned.

Minimal Example

Here is a minimal example:

1. Tables

class Location(Base):
    __tablename__ = "locations"  # type: ignore
    id: Column[int] = Column(Integer, primary_key=True, index=True)
    name: Column[str] = Column(String, nullable=False, unique=True)

    tasks = relationship("Task", lazy="joined", back_populates="location", uselist=False)

class Task(Base):
    __tablename__ = "tasks"  # type: ignore
    id: Column[int] = Column(Integer, primary_key=True, index=True)
    name: Column[str] = Column(String, nullable=False)
    location_id: Column[Optional[int]] = Column(Integer, ForeignKey(Location.id), nullable=True)

    location = relationship(Location, lazy="joined", back_populates="tasks", uselist=False)

2. Mapper

strawberry_sqlalchemy_mapper = StrawberrySQLAlchemyMapper()

# ...

@strawberry_sqlalchemy_mapper.type(app.models.Location)
class LocationType:
    pass

@strawberry_sqlalchemy_mapper.type(app.models.Task)
class TaskType:
    pass

async def get_context() -> dict:
    return {
        "sqlalchemy_loader": StrawberrySQLAlchemyLoader(bind=SessionLocal()),
    }

# ...

additional_types = list(strawberry_sqlalchemy_mapper.mapped_types.values())
schema = strawberry.Schema(Query, Mutation, extensions=[SQLAlchemySession], types=additional_types)

graphql_app = GraphQLRouter(schema, context_getter=get_context,)

3. Queries

strawberry_sqlalchemy_mapper.finalize()

# ...

@strawberry.type
class Query:
    @strawberry.field
    async def tasks(self, info: Info) -> list[TaskType]:
        db = info.context["db"]
        sql = db.query(app.models.Task).order_by(app.models.Task.name)
        db_tasks = sql.all()
        return db_tasks

    @strawberry.field
    async def locations(self, info: Info) -> list[LocationType]:
        db = info.context["db"]
        sql = db.query(app.models.Location).order_by(app.models.Location.name)
        db_locations = sql.all()
        return db_locations

My database contains this data:

image

Issue

Now when I query using GraphQl, this is the output:

Query:

query Tasks {
  tasks {
    id
    name
    location {
      id
      name
      tasks {
        id
        name
      }
    }
  }
}

Result:

{
  "data": {
    "tasks": [
      {
        "id": 1,
        "name": "new Task",
        "location": {
          "id": 1,
          "name": "New Location",
          "tasks": {
            "id": 4,
            "name": "new Task 1-4"
          }
        }
      },
      {
        "id": 2,
        "name": "new Task 1-2",
        "location": {
          "id": 1,
          "name": "New Location",
          "tasks": {
            "id": 4,
            "name": "new Task 1-4"
          }
        }
      },
      {
        "id": 3,
        "name": "new Task 1-3",
        "location": {
          "id": 1,
          "name": "New Location",
          "tasks": {
            "id": 4,
            "name": "new Task 1-4"
          }
        }
      },
      {
        "id": 4,
        "name": "new Task 1-4",
        "location": {
          "id": 1,
          "name": "New Location",
          "tasks": {
            "id": 4,
            "name": "new Task 1-4"
          }
        }
      },

     // ...

    ]
  }
}

I would have expected all 4 tasks would show up in the nested tasks object. I am very new to GraphQL and not a 100% sure if this is an issue with my query or it is an issue in your wrapper.

mattalbr commented 2 years ago

I'm 90% sure the bug here is tasks = relationship("Task", lazy="joined", back_populates="location", uselist=False)

When you set uselist to False, you're saying that there is only one value for "tasks". By doing it in both directions, you've configured this as a one-to-one relationship, when it really should be one to many (or many to one, I can never remember sqlalchemy's convention there).

CDTiernan commented 2 years ago

yasoob, would you be willing to give some more details on the Mapper section?

I am just starting out with this package and am also using SQLAlchemy. Currently, I can get queries without traversing relationships to work, but when I attempt to do so I get a threading error: "There is no current event loop in thread 'Thread-1'". I haven't been able to find the extension you are using.

Any help is appreciated, thanks!

yasoob commented 2 years ago

Hey @CDTiernan I haven't worked on this almost since I created the issue. I think @raguiar2 would be a better person to help on this :)

TimDumol commented 2 years ago

Hey @CDTiernan, are you using this on Flask w/o async? The package currently assumes an async context for the dataloader, which is probably why you're receiving that error

CDTiernan commented 2 years ago

Yup this was my issue-- although I am using Django. Switching from GraphQLView to AsyncGraphQLView fixed this issue.

Thanks for the help!