Feature Request: force select columns

[Migushthe2nd]

Is your feature request related to a problem? Please describe. I use @nestjs/graphql together with typeorm. This allows me to use a single entity type and base the graphql and database off of it. In my case this means that there are some properties that only have the @Field() decorator (they then use @FieldResolver()), or only @Column. For example,

...
    @Column("bytea", {nullable: true})
    avatarFile?: string;
    @Column("bytea", {nullable: true})
    bannerFile?: string;

    @Field()
    bannerUrl: string;
    @Field()
    avatarUrl: string;
...

The issue is that PerchQueryBuilder will never fetch avatarFile or bannerFile, since they never are in the GraphQL query. In my case I need to check if these are null, in order to correctly determine avatarUrl and bannerUrl.

Describe the solution you'd like New decorators that can be added to a property. Something like: @SelectAlways() and @SelectLinkTo(property: string) (Other possible names for the second decorator could be ForceSelect, SelectIf, SelectAlsoIf, SelectWhenSelected)

The use of @SelectAlways() should be clear. Example usage of @SelectLinkTo(property: string):

...
    @Column("bytea", {nullable: true})
    @SelectLinkTo("avatarUrl")
    avatarFile?: string;

    @Field()
    avatarUrl: string;
...

This should select the avatarFile column if avatarUrl is in the query, which will in turn allow me to check the value of avatarFile when resolving the avatarUrl field.

Describe alternatives you've considered

• My first idea was respect the select config property of the @Column decorator (@Column("bytea", {nullable: true, select: true})), since the TypeORM docs mention that it "Indicates if column is always selected by QueryBuilder and find operations. Default value is 'true'.". The last part, however, makes it difficult. This would require the developer to manually set all other columns to {select: false}.

[wesleyyoung]

For now, if you want to force it to always be selected use the PerchQueryBuilder.generateQueryBuilder(repository, graphQLInfo) method and add addSelect clauses. If you wish to implement business logic before returning the results of the query, you can store the result in a variable and perform logic on it before returning in your resolver. I like the SelectAlways decorator, but the others do not belong in this package, as overall database interaction logic does not belong inside an entity class.

[wesleyyoung]

I will also say that since TypeORM actually has this feature, it's likely out of scope for this project. PerchQueryBuilder is about combining GraphQL with TypeORM, not replacing or overlapping either. I'm willing to hear why it really should be a part of this library, but if the TypeORM devs require you to manually set the others to false I'm willing to bet they have a good reason for doing so.

It turns out TypeORM select: true | false doesn't work the way I think you are describing here. It looks like since they are just marking the fields returned by a find or similar call. With PerchQueryBuilder we are actually adding the SELECT statements in, so it won't overlap.

My issue here however, is this seems to be confusing entity declaration with what is really the concern of the resolver class. Specifying the fields to forcibly return, when to return them, and what to fallback to inside the entity declaration seems like a bad design decision. Particularly if you need a new resolver on the table that has different requirements or access. How would this be handled in a JOIN statement?

[Migushthe2nd]

It turns out TypeORM select: true | false doesn't work the way I think you are describing here. It looks like since they are just marking the fields returned by a find or similar call.

That is indeed also my understanding.

With PerchQueryBuilder we are actually adding the SELECT statements in, so it won't overlap.

Yes I also seem to have this correct. I meant that the typeorm select is indeed not suitable, and it was basically not an alternative option to begin with (except if you completely modify the behaviour of @Column, which is not the goal of this package).

My issue here however, is this seems to be confusing entity declaration with what is really the concern of the resolver class. Specifying the fields to forcibly return, when to return them, and what to fallback to inside the entity declaration seems like a bad design decision.

I agree. The design is a bit confusing, but I am not entirely sure what a better alternative would be. Currently, my files are as follows (with respective .module.tss):

// User.entity.ts
@ObjectType()
@Entity()
export default class User extends BaseEntity {

    @PrimaryGeneratedColumn()
    id: number;    

    @Field(() => UserProfile)
    @OneToOne(() => UserProfile, profile => profile.user, {cascade: true})
    profile: UserProfile;

}

// User.resolver.ts
@Injectable()
@Resolver()
export default class UserResolver {

    constructor(@InjectRepository(User) private userRepository: Repository<User>) {}

    @Query(() => [User], {description: `Generic Collection Query For Users`,})
    async users(@Context() ctx, @Info() info: GraphQLResolveInfo): Promise<User[]> {
        return await PerchQueryBuilder.find<User>(this.userRepository, info);
    }

}

// UserProfile.entity.ts
@ObjectType()
@Entity()
export default class UserProfile {

    @Column("bytea", {nullable: true})
    avatarFile?: string;
    @Column("bytea", {nullable: true})
    bannerFile?: string;

    @Field({description: "WebP image", nullable: true})
    avatar?: string;
    @Field({description: "WebP image", nullable: true})
    banner?: string;

}

// UserProfile.resolver.ts
@Resolver(UserProfile)
export default class UserProfileResolver {

    @ResolveField()
    avatar(): string {
        return "url";
    }

    @ResolveField()
    banner(): string {
        return "url";
    }

}

The thing is, UserProfile is not a BaseEntity. I don't want there to be a userProfile query. If you're up for it, could you give a suggestion? Like, would you recommend to split the UserProfile.entity.ts into two files (entities?): one for TypeORM and one for GraphQL? Or perhaps assign banner in the UserResolver by looping over all user objects before returning?

I know is quite the response and a bit off-topic, sorry for that.

[wesleyyoung]

would you recommend to split the UserProfile.entity.ts into two files (entities?): one for TypeORM and one for GraphQL

You don't need two different entities (you might mean classes in which case yes), and yes this is much better practice. Ideally 3, one which defines the schema, one which is annotated @Entity which holds the column definitions and another which is the @ObjectType.

For a code example you can do this:

@Query(() => [User], {description: `Generic Collection Query For Users`,})
async users(@Context() ctx, @Info() info: GraphQLResolveInfo): Promise<User[]> {
    const qb = PerchQueryBuilder.generateQueryBuilder<User>(this.userRepository, info);
    qb.addSelect(`${qb.alias}.avatar`);
    const users = await qb.getMany();
    return users.map(user => {
        if (!user.avatar) {
           user.avatar = 'foo';
       }

        return user;
    })
}

[Migushthe2nd]

Alright, I see now that splitting in two classes is probably the best idea. What I am still wondering however, is how would I return an Entity class as a Model class (entity: repository entity, model: graphql schema)? They can't be "casted" automatically, since there are properties on the Model that are not on the Entity. Manually mapping all fields from Entity to Model seems tedious and very inefficient for large arrays.

I figured an option would be to use "virtual fields"; basically create the extra properties in the Entity class, but then I'm back where I started.

Edit: I tried something else that seem like it could simplify it a lot, however this does introduce new properties in the Entity class: "Virtual Columns" and @AfterLoad() to determine their values

@Entity()
export class UserProfileEntity {

    @Column("bytea", {nullable: true})
    avatarFile?: string;

    @Column("bytea", {nullable: true})
    bannerFile?: string;

    avatarUrl: string;
    bannerUrl: string;

    @AfterLoad()
    afterLoad() {
        if (!!this.avatarFile) {
            this.avatarUrl = `https://........`
        }
        if (!!this.bannerFile) {
            this.bannerUrl = `https://........`
        }
    }

}

[wesleyyoung]

// user.model.ts
export interface UserModel {
  id: number; 
  profile: UserProfileModel;
}

// User.entity.ts
@Entity()
export default class UserEntity implements UserModel {

    @PrimaryGeneratedColumn()
    id: number;    

    @OneToOne(() => UserProfileEntity, profile => profile.user, {cascade: true})
    profile: UserProfileEntity;
}

// user.object-type.ts
@ObjectType()
export default class UserObjectType implements UserModel {

    @Field(() => Int)
    id: number;    

    @Field(() => UserProfileObjectType)
    profile: UserProfileObjectType;
}

Not inefficient at all, very good practice in fact. Utilizing a shared interface ensures any classes you have that represent a user contain at very minimum the required fields. And there is nothing "inefficient" performance wise either, interfaces don't exist at runtime and the memory footprint for class definitions is measured in bytes, at best. Infact this if anything is more performant because you can create classes that represent Users without having to bring in all the dependencies of all of your libraries with it.

Imagine if you moved your project into a mono-repo and wanted to use your same entity definitions for your Angular or React client? You couldn't because Angular and React don't know what to do with the @ObjectType or @Entity decorators. Having a base interface and separate classes for data storage and data view layer allows you far greater control and helps you maintain the project better in the long term.

[Migushthe2nd]

Not inefficient at all, very good practice in fact.

I was concerned of the performance impact of using map() to assign URLs in the way you mentioned two messages back:

@Query(() => [User], {description: `Generic Collection Query For Users`,})
async users(@Context() ctx, @Info() info: GraphQLResolveInfo): Promise<User[]> {
    const qb = PerchQueryBuilder.generateQueryBuilder<User>(this.userRepository, info);
    qb.addSelect(`${qb.alias}.avatarFile`);
    const users = await qb.getMany();
    return users.map(user => {
       if (!!user.avatarFile) {
           user.avatarUrl = 'https://......';
       }

        return user;
    })
}

Now reconsidering, performance impact of an extra map might be very minimal. Btw, wouldn't I have to do this in every resolver that has UserObjectType nested?

I can see how using an interface will improve modularity and reusability. My question is more about how one can "convert" your UserEntity into a UserObjectType in the case the UserObjectType has fields that are not in the UserEntity. Specifically since I do not store avatarUrl in the DB, but I do have the property in the UserObjectType. I thought of a loop, but I'd have create many new UserObjectType() and manually assign values, right? From my edited previous comment, the following seems very simple instead:

@Entity()
export class UserProfileEntity {

    @Column("bytea", {nullable: true})
    avatarFile?: string;

    avatarUrl: string;

    @AfterLoad()
    afterLoad() {
        if (!!this.avatarFile) {
            this.avatarUrl = `https://........`
        }
    }

}

But this in turn makes it for example possible to call

UserProfileEntity.create({
    avatarUrl: "asdfas"
})

While the property is not in the DB.

Regarding interfaces, I'd have to either include avatarUrl or avatarFile (since a User should have an avatar). I myself would choose avatarUrl, but if the UserEntity and UserObjectType both implement the interface this would mean that the UserObjectType would have to include an empty avatarUrl field, making the use of @AfterLoad() very tempting.

[wesleyyoung]

Now reconsidering, performance impact of an extra map might be very minimal. Btw, wouldn't I have to do this in every resolver that has UserObjectType nested?

Not if you abstract the logic into some sort of DataProvider service that can be consumed by any resolver.

I can see how using an interface will improve modularity and reusability. My question is more about how one can "convert" your UserEntity into a UserObjectType in the case the UserObjectType has fields that are not in the UserEntity.

In my example the UserObjectType implements the UserModel, so you can add as many new properties to the ObjectType as you would like!

You'll need to update the resolver to:

@Query(() => [UserObjectType], {description: `Generic Collection Query For Users`,})
async users(@Context() ctx, @Info() info: GraphQLResolveInfo): Promise<UserObjectType[]> {
    const qb = PerchQueryBuilder.generateQueryBuilder<User>(this.userRepository, info);
    qb.addSelect(`${qb.alias}.avatarFile`);
    const users = await qb.getMany() as UserObjectType[];
    return users.map(user => {
       if (!!user.avatarFile) {
           user.avatarUrl = 'https://......';
       }

        return user;
    })
}

[Migushthe2nd]

Alright, I managed to explore typeorm and typegraphql better and I am happy with a solution I am using now. Please mark the conversation above as off-topic. Regarding the feature request: I will consider using addSelect, but will this work for circular relations? If not, the feature request still stands