OnetoMany relationships not returning results, blocked in graphql about nestjs-query HOT 32 CLOSED

The main reason that is changed was simply that the way we do it now performs way better then the old one (see for example #1495), the old one builded queries that where very very inefficient if you where going to fetch more then a couple of rows.

Having to add all the columns to your entity was not a issue for me (since I was already doing it as I also want to filter on it) , I also do not see this as a "hack" as in my optionion it is better to have your entity actually reflect with that is in the database, so when I created the initial fork for this I did not have any issues with it. Also, adding this field does have no impact on the database since Typeorm already creates the field, adding it only makes sure that it's also being fetched/set when doing queries.

You can downgrade to the old versions again if you want but this fork is not going back to the old implementation simply because of the massive performance drawback the old implementation has.

from nestjs-query.

@TriPSs I didn't have @joincolumn on both sides. But the issue I had was already fixed when I updated the package, my bad.

from nestjs-query.

@jpv-os feel free to use my demo project which is set up and demonstrates the issue: https://github.com/coupster74/nestjs-query-issue

from nestjs-query.

Should be fixed by #175, thanks @jpv-os for the PR!

from nestjs-query.

just gets weirder, but thought I would share as I think this might help isolate where the issue is:

I've updated my test resolver to be as follows:

@Query(() => Location)
  async test() {
    // const results = await this.locationRepository.find({
    //   where: { id: 'us-minnesota-sauk rapids' },
    //   relations: {
    //     actions: true,
    //   },
    // });
    // console.log(results);
    const results2 = await this.locationService.getById(
      'us-minnesota-sauk rapids',
    );
    results2.actions = await this.locationService.queryRelations(
      Action,
      'actions',
      results2,
      {},
    );
    console.log(results2);
    return results2;
  }

note that console.log(results2) shows exactly what I would expect. The location, with an array of the actions:

Location {
  isActive: true,
  isArchived: false,
  internalComment: null,
  created: 2023-03-21T15:50:04.902Z,
  createdBy: null,
  updated: 2023-03-21T15:50:04.902Z,
  updatedBy: null,
  deletedOn: null,
  deletedBy: null,
  id: 'us-minnesota-sauk rapids',
  city: 'Sauk Rapids',
  province: 'Minnesota',
  country: 'US',
  timezone: 'America/Chicago',
  region: 'NA',
  actions: [
    Action {
      isActive: true,
      isArchived: false,
      internalComment: null,
      created: 2023-03-21T15:47:11.000Z,
      createdBy: 'Unknown',
      updated: 2023-03-21T15:50:04.992Z,
      updatedBy: 'Unknown',
      deletedOn: null,
      deletedBy: null,
      id: 'd72330a0-ffc7-ed11-b596-000d3af4f717',
      title: '-Sauk Rapids-30 Apr 2023 23:00:00-36585527-6746-4f27-9179-7799f769192b',
      content: `"Some action content."`,
      highlightImageUrl: null,
      locationDescription: 'Sauk Rapids Government Center; Sauk Rapids',
      date: 2023-04-30T23:00:00.000Z,
    }
  ]
}

and yet, the results returned is the same - the same problem.

{
  "data": {
    "test": {
      "id": "us-minnesota-sauk rapids",
      "actions": {
        "edges": []
      }
    }
  }
}

This leads me to believe the aggregator is not correctly adding related elements together (aggregate-relations.resolver?, or potentially within the graphql-resolve-info? ). Again, this is only for oneToMany relation from the single side.

I've only just started digging into the bowels of nestjs-query cause typically it has just worked. I'm attempting to reproduce with the basic example.

from nestjs-query.

what's the secret to getting the examples to run? I've walked through the readme, but the npm run start -- {example name} "start" command doesn't exist.

from nestjs-query.

Could you add the code of your Location.module.ts?
Also just a tip, when you add code snippets, you can add the language so that github will colorize it nicely, making it easier to read. Ex: ```ts

from nestjs-query.

sure! Not too special.. My testing is actually being done in a reporting module. Other than the test code above, the reporting module is primarily custom SQL queries executed via typeorm query. here is the code:

import { Module } from '@nestjs/common';
import { TypeOrmModule } from '@nestjs/typeorm';
import { Location } from '../../model/location.entity';
import { Article } from '../../model/article.entity';
import { ReportingResolver } from './reporting.resolver';
import { ReportingService } from './reporting.service';
import { NestjsQueryTypeOrmModule } from '@ptc-org/nestjs-query-typeorm';

@Module({
  imports: [
    TypeOrmModule.forFeature([Article]),
    TypeOrmModule.forFeature([Location]),
    NestjsQueryTypeOrmModule.forFeature([Location]),
  ],
  providers: [ReportingService, ReportingResolver],
})
export class ReportingModule {}

and thanks for the tip!

from nestjs-query.

Ok so you're not using NestjsQueryGraphQLModule.forFeature anywhere?

from nestjs-query.

Could you try to enable debug logging of TypeORM to see what queries it does? Maybe there is a mistake in the query or is the data not mapped correctly back to the correct items.

from nestjs-query.

Sorry, the original location.module.ts looks like this and demonstrates the same issue of not pulling the one to many children. you can see if uses NestjsQueryGraphQLModule:

import { NestjsQueryGraphQLModule } from '@ptc-org/nestjs-query-graphql';
import { NestjsQueryTypeOrmModule } from '@ptc-org/nestjs-query-typeorm';
import { Module } from '@nestjs/common';
import { Location } from '../../model/location.entity';
import { LocationResolver } from './location.resolver';
//import { LocationAssembler } from './location.assembler';
//import { LocationResolver } from './location.resolver';

@Module({
  //providers: [LocationResolver],
  imports: [
    NestjsQueryGraphQLModule.forFeature({
      imports: [NestjsQueryTypeOrmModule.forFeature([Location])],
      //assemblers: [LocationAssembler],
      resolvers: [
        {
          DTOClass: Location,
          EntityClass: Location,
          enableAggregate: true,
          enableTotalCount: true,
          create: { disabled: true },
          update: { disabled: true },
          delete: { many: { disabled: true } },
        },
      ],
      // describe the resolvers you want to expose
    }),
  ],
  providers: [LocationResolver],
})
export class LocationModule {}

from nestjs-query.

For the logging question, I think typeorm is ok. I set the limit to 10, and the logging looks as expected. As mentioned, It appears it is an issue with how the results are assembled within graphql results (sorry.. tough to read as there are a lot of fields):

query: SELECT "Location"."isActive" AS "Location_isActive", "Location"."isArchived" AS "Location_isArchived", "Location"."internalComment" AS "Location_internalComment", "Location"."created" AS "Location_created", "Location"."createdBy" AS "Location_createdBy", "Location"."updated" AS "Location_updated", "Location"."updatedBy" AS "Location_updatedBy", "Location"."deletedOn" AS "Location_deletedOn", "Location"."deletedBy" AS "Location_deletedBy", "Location"."id" AS "Location_id", "Location"."city" AS "Location_city", "Location"."province" AS "Location_province", "Location"."country" AS "Location_country", "Location"."timezone" AS "Location_timezone", "Location"."region" AS "Location_region" FROM "location" "Location" WHERE "Location"."deletedOn" IS NULL LIMIT 11
query: SELECT "actions"."isActive" AS "actions_isActive", "actions"."isArchived" AS "actions_isArchived", "actions"."internalComment" AS "actions_internalComment", "actions"."created" AS "actions_created", "actions"."createdBy" AS "actions_createdBy", "actions"."updated" AS "actions_updated", "actions"."updatedBy" AS "actions_updatedBy", "actions"."deletedOn" AS "actions_deletedOn", "actions"."deletedBy" AS "actions_deletedBy", "actions"."id" AS "actions_id", "actions"."remoteUpdated" AS "actions_remoteUpdated", "actions"."title" AS "actions_title", "actions"."content" AS "actions_content", "actions"."sources" AS "actions_sources", "actions"."hashtags" AS "actions_hashtags", "actions"."status" AS "actions_status", "actions"."draftedBy" AS "actions_draftedBy", "actions"."publishedBy" AS "actions_publishedBy", "actions"."contentApprovedBy" AS "actions_contentApprovedBy", "actions"."styleApprovedBy" AS "actions_styleApprovedBy", "actions"."publishedDate" AS "actions_publishedDate", "actions"."lastModifiedDate" AS "actions_lastModifiedDate", "actions"."mainUntilDate" AS "actions_mainUntilDate", "actions"."source" AS "actions_source", "actions"."sourceRefId" AS "actions_sourceRefId", "actions"."tsv" AS "actions_tsv", "actions"."highlightImageUrl" AS "actions_highlightImageUrl", "actions"."locationDescription" AS "actions_locationDescription", "actions"."bringForward" AS "actions_bringForward", "actions"."date" AS "actions_date", "actions"."endDate" AS "actions_endDate", "actions"."raw" AS "actions_raw", "actions"."impact" AS "actions_impact", "actions"."latitude" AS "actions_latitude", "actions"."longitude" AS "actions_longitude", "actions"."virtual" AS "actions_virtual", "actions"."interested" AS "actions_interested", "actions"."going" AS "actions_going", "actions"."locationId" AS "actions_locationId", "actions"."tacticId" AS "actions_tacticId" FROM "action" "actions" WHERE "actions"."deletedOn" IS NULL AND "actions"."locationId" IN ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) LIMIT 1001 -- PARAMETERS: ["france--morlaas","us-minnesota-sauk rapids","uk--galway","poland--poznan","us-dc-washingon","us-california-baldwin park","france--lusignan","us-oregon-salem","us-california-weaverville","france--fontenay-le-comte"]

from nestjs-query.

Okay thanks, then I think the issue happens somewhere here, maybe you could debug/log some steps there? Checkout if the column.propertyName for example are correct.

Otherwise I think it would be helpfull if you are able to:

1. Create a repo with the issue
2. Or create a example inside this repo with the issue

from nestjs-query.

ok. I'll take a look and see what I can do.. just to confirm you think it is in this method getManyToOneOrOneToOneOwnerMeta vs this one getOneToManyOrOneToOneNotOwnerMeta a few lines down? My issue is primarily with the oneToMany from the "one" side.

from nestjs-query.

Yea that one indeed! And then mapRelations part in particular.

from nestjs-query.

getOneToManyOrOneToOneNotOwnerMeta is getting called, and thus mapRelations is called correctly per row. Within the mapRelations function, relations has the relation dataset, however, this return statement return (0, lodash_filter_1.default)(relations, filter) returns an empty array.

From what I can ascertain, the related entity does not have the attribute the filter is targeting. Here is an example:

Action {
  isActive: true,
  isArchived: false,
  internalComment: null,
  created: 2022-04-19T21:53:18.000Z,
  createdBy: null,
  updated: 2022-04-21T18:07:03.713Z,
  updatedBy: null,
  deletedOn: null,
  deletedBy: null,
  id: '535f8e1c-2bc0-ec11-983e-0022486dfebd',
  remoteUpdated: 2022-04-19T21:53:25.000Z,
  title: '-Atlanta-21 Apr 2022 17:00:00-00512705-214b-4f77-99da-17768dab80c5',
  content: `"content here"`,
  sources: [],
  hashtags: [],
  status: 'Published',
  draftedBy: null,
  publishedBy: null,
  contentApprovedBy: null,
  styleApprovedBy: null,
  publishedDate: 2022-04-19T21:53:18.000Z,
  lastModifiedDate: 2022-04-19T21:53:18.000Z,
  mainUntilDate: null,
  source: null,
  sourceRefId: null,
  highlightImageUrl: null,
  locationDescription: 'Starbucks at Howell Mill, 1801 Howell Mill Rd NW; Atlanta',
  bringForward: null,
  date: 2022-04-21T17:00:00.000Z,
  endDate: 2022-04-21T18:00:00.000Z,
  raw: true,
  impact: '2 (medium)',
  latitude: 33.80422,
  longitude: -84.413,
  virtual: false,
  interested: null,
  going: null

and the filter:

{ location: 'us-texas-point comfort' }

And, because location isn't an attribute on Action, no entities are returned from the lodash filter.

I would need a pointer for where to look next.

from nestjs-query.

@TriPSs bump..

from nestjs-query.

That it a weird, I would expect the filter to actually have the relation ids, or is location the relation?

Could you create a repo containing this issue?

from nestjs-query.

I can attempt that, yes.. Just a small tweak to your statement... the entities are missing the relationship key which the filter is attempting to filter (yes, in this case, location's id)

I'm unclear about what is expected to exist in the related entities since from a db schema perspective, my actions would have a locationId and yet the filter is focused on location. If this is "graphql style" I would expect action to have a sub object of location with an id attribute, in which case shouldn't the filter should be { location: { id: '' } }.

I'm not sure if I'm being clear - the point I'm making is I don't know what should be in the action to enable the filter to work...

Action {
  <!-- snip -->
  location: { id: 'us-texas-point comfort' }

or

Action {
  <!-- snip -->
  locationId: 'us-texas-point comfort' 

from nestjs-query.

@TriPSs https://github.com/coupster74/nestjs-query-issue reproduces the issue. Once installed, within an explorer, you can produce the following which is obviously incorrect:

{
  "data": {
    "location": {
      "id": "canada-alberta-calgary",
      "actions": {
        "edges": []
      }
    },
    "actions": {
      "edges": [
        {
          "node": {
            "id": "f2469e7d-42bd-4450-9c9c-47d54718417f",
            "title": "action in calgary",
            "location": {
              "id": "canada-alberta-calgary"
            }
          }
        }
      ]
    }
  }
}

from nestjs-query.

Thanks for the repo! Will have a look at it this week!

from nestjs-query.

@coupster74 I found out what caused your issue, the returned object did not contain the locationId causing the filter to fail when remapping the results back.

This can easily be solved by changing the following in your location.entity.ts

  @ManyToOne(() => Location, (location) => location.actions)
  @Index()
  location?: Location;

  // Add this field, this will make sure Typeorm maps the value to this column and the filter can then use it when mapping,
  @Column()
  locationId: string

from nestjs-query.

from nestjs-query.

from nestjs-query.

@TriPSs thanks - I've got it working in my main app now, and performance is a reasonable reason for the change, and I can get on board with that. However, it is a difference that I recommend be called out in the documentation since we don't need to put these typeorm column decorated entries in for other relationships.

what do you think about the nestjs-query library dynamically adding these column attributes to the entities in this scenario?

from nestjs-query.

@TriPSs I think I found another problem related to these changes as well. When you specify a one-to-one relationship, @relation only works if the @joincolumn is set on the other side:

Doing that is also required by TypeORM

Again, @joincolumn must be set only on one side of relation - the side that must have the foreign key in the database table.

from nestjs-query.

@TriPSs Has adding the field and column been documented somewhere yet? I still didn't seem to find this info other than this thread.

from nestjs-query.

I experienced the same issue in my code base. As @TriPSs suggested, I tried adding an ID column to my existing entity and it fixed my problem.

However, I have to agree with @coupster74 in that I don't find the solution particularly clean. I would have to add a new "virtual" column to every single entity in my very large mono repo and remember to not use the new column in my code, as its only purpose is to "help" the graphql library.

Because of that, I tried to find a different approach: I was able to create a new decorator that wraps the nestjs-query relation decorators. The decorator will declare a property with the required name (in my example: create column plantId for relation plant) and apply the nestjs-query decorators. This way, I still need to update all my entity files, but now I have a unified way of doing things that hides all the dirty details from the developer and I can update the compatibility code in one place should I have to.

The decorators:

// Disable naming conventions for the current file to be able to set up the annotations grouped in a class.
/* eslint-disable @typescript-eslint/naming-convention */
import { ObjectType } from '@nestjs/graphql';
import {
  FilterableRelation,
  FilterableUnPagedRelation,
  PagingStrategies,
  QueryOptions,
  Relation,
  RelationTypeFunc,
  UnPagedRelation,
} from '@ptc-org/nestjs-query-graphql';
import { RelationClassDecorator } from '@ptc-org/nestjs-query-graphql/src/decorators/relation.decorator';
import { Column } from 'typeorm';

/**
 * Represents any constructor (i.e. a class).
 */
// Since this is a general purpose constructor type, we need to allow the "any" type.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
interface Constructor<ClassType = any, ArgsArrayType extends any[] = any[]> {
  new (...args: ArgsArrayType): ClassType;
}

/**
 * Options for defining a graphql relation.
 */
export interface MyCustomGqlRelationOptions {
  /**
   * The relation value can be nullable.
   */
  nullable?: boolean;
  /**
   * Override relation name.
   */
  relationName?: string;
  /**
   * Makes a relation filterable, default is true.
   */
  filterable?: boolean;
}

const createRelationDecorator = <RelationType>(
  relationKind: 'to-many' | 'to-one',
  name: string,
  relationTypeFunc: RelationTypeFunc<RelationType>,
  options?: MyCustomGqlRelationOptions,
): ClassDecorator => {
  const nullable = options?.nullable ?? false;
  const filterable = options?.filterable ?? true;
  return <TFunction extends Function>(target: TFunction): TFunction | void => {
    // apply nestjs-query relation class decorator
    let nestjsQueryClassDecorator: RelationClassDecorator<unknown>;
    if (relationKind === 'to-many' && filterable) {
      nestjsQueryClassDecorator = FilterableUnPagedRelation(name, relationTypeFunc, {
        nullable: options?.nullable ?? false,
        relationName: options?.relationName,
      });
    } else if (relationKind === 'to-many' && !filterable) {
      nestjsQueryClassDecorator = UnPagedRelation(name, relationTypeFunc, {
        nullable: options?.nullable ?? false,
        relationName: options?.relationName,
      });
    } else {
      // define a join column property and apply typeorm property decorator
      const idColumnName = `${name}Id`;
      Object.defineProperty(target, idColumnName, {});
      const typeormColumnDecorator = Column({ type: 'integer', nullable });
      typeormColumnDecorator(target.prototype as Object, idColumnName);
      if (filterable) {
        nestjsQueryClassDecorator = FilterableRelation(name, relationTypeFunc, {
          nullable: options?.nullable ?? false,
          relationName: options?.relationName,
        });
      } else {
        nestjsQueryClassDecorator = Relation(name, relationTypeFunc, {
          nullable: options?.nullable ?? false,
          relationName: options?.relationName,
        });
      }
    }
    const decoratedClass = nestjsQueryClassDecorator(target as unknown as Constructor);
    return decoratedClass as unknown as TFunction | void;
  };
};

/**
 * Namespace for defining graphql decorators.
 */
export class MyCustomGql {
  /**
   * Decorator for defining graphql entity schemas.
   * @param name The entity name.
   * @returns {ClassDecorator} Class decorator.
   */
  static readonly Schema =
    (name: string): ClassDecorator =>
    (target: Function) => {
      ObjectType(name)(target);
      QueryOptions({ pagingStrategy: PagingStrategies.OFFSET })(target as Constructor);
    };

  /**
   * Decorator for one-to-one relationships.
   * @param name The relation name.
   * @param relationTypeFunc The relation type.
   * @param options The relation options.
   * @returns {ClassDecorator} Class decorator.
   */
  static readonly OneToOne = <RelationType>(
    name: string,
    relationTypeFunc: RelationTypeFunc<RelationType>,
    options?: MyCustomGqlRelationOptions,
  ): ClassDecorator => createRelationDecorator('to-one', name, relationTypeFunc, options);

  /**
   * Decorator for one-to-many relationships.
   * @param name The relation name.
   * @param relationTypeFunc The relation type.
   * @param options The relation options.
   * @returns {ClassDecorator} Class decorator.
   */
  static readonly OneToMany = <RelationType>(
    name: string,
    relationTypeFunc: RelationTypeFunc<RelationType>,
    options?: MyCustomGqlRelationOptions,
  ): ClassDecorator => createRelationDecorator('to-many', name, relationTypeFunc, options);

  /**
   * Decorator for many-to-one relationships.
   * @param name The relation name.
   * @param relationTypeFunc The relation type.
   * @param options The relation options.
   * @returns {ClassDecorator} Class decorator.
   */
  static readonly ManyToOne = <RelationType>(
    name: string,
    relationTypeFunc: RelationTypeFunc<RelationType>,
    options?: MyCustomGqlRelationOptions,
  ): ClassDecorator => createRelationDecorator('to-one', name, relationTypeFunc, options);

  /**
   * Decorator for many-to-many relationships.
   * @param name The relation name.
   * @param relationTypeFunc The relation type.
   * @param options The relation options.
   * @returns {ClassDecorator} Class decorator.
   */
  static readonly ManyToMany = <RelationType>(
    name: string,
    relationTypeFunc: RelationTypeFunc<RelationType>,
    options?: MyCustomGqlRelationOptions,
  ): ClassDecorator => createRelationDecorator('to-many', name, relationTypeFunc, options);
}

Example usage:

@Entity()
@MyCustomGql.Schema('Plant')
@MyCustomGql.OneToOne('photo', () => PhotoEntity, { nullable: true })
@MyCustomGql.OneToMany('plantCares', () => PlantCareEntity, { nullable: true })
export class PlantEntity {
  @PrimaryGeneratedColumn()
  @IDField(() => Int)
  id: number;

  @Column()
  @FilterableField()
  name: string;

  @Column({ nullable: true })
  @FilterableField({ nullable: true })
  description?: string;

  @OneToOne(() => PhotoEntity, photo => photo.plant, { nullable: true })
  photo?: Promise<PhotoEntity>;

  @Column()
  @FilterableField()
  location: string;

  @Column()
  @FilterableField()
  customer: string;

  @OneToMany(() => PlantCareEntity, care => care.plant)
  plantCares: Promise<PlantCareEntity[]>;
}

I am open to suggestions or comments about this approach, as it works correctly now but I may be overlooking some problems down the line.

from nestjs-query.

Time to report back after 2 weeks of my "fix" in production code, and it failed miserably!

First of all, my approach didn't account for non-owning OneToOne relationships and generated the additional column on both sides of the relation. I fixed that problem by introducing an owner flag for the OneToOne relationship and generating the additional column only when the entity is the owner. With that in place, the GraphQL API worked like a charm again.

However, when using the TypeORM features such as repositories and entity manager, there came new problems:

• deleting an entity by relation ID did not work anymore, such as: orderRepository.delete({ customer: { id: 42 } }) (the autogenerated customerId column caused this problem, because there already was a customer relation and now it was defined twice, leading to undefined behaviour
• changing a non-nullable relation caused similar problems and resulted in a NULL error, e.g.: order.customer = <another customer entity>; orderRepository.save(order)

After trying to solve the new problems, i had to accept that my solution did not work and thus, the suggested solution by @trips does not work as intended.

Because of that, I tried a similar approach as @coupster74 and started digging into the code. As was pointed out in this thread already, the problem appeared to be the mapRelations function definition in the getOneToManyOrOneToOneNotOwnerMeta of relation-query.builder.

For a quick proof of concept solution, I copied the JS files in my monorepo into a new folder dependencies/nestjs-query-typeorm, updated my package.json to use the local package ("@ptc-org/nestjs-query-typeorm": "file:./dependencies/nestjs-query-typeorm") and started some experimental coding directly in the JS files to test against my projects. This is what I ended up with as a proof of concept:

// additional lodash imports at top of file
const lodash_keys_1 = tslib_1.__importDefault(require("lodash.keys"));
const lodash_pickBy_1 = tslib_1.__importDefault(require("lodash.pickby"));

// ...

mapRelations: (entity, relations, rawRelations) => {
    const rawFilter = columns.reduce((columnsFilter, column) => ({
        ...columnsFilter,
        [this.buildAlias(column.databaseName)]: column.referencedColumn.getEntityValue(entity)
    }), {});
    const rawIndices = (lodash_keys_1.default(lodash_pickBy_1.default(rawRelations, rawFilter))).map(key => +key);
    return rawIndices.map(idx => relations[idx]);
},

// ...

The objects in the relation array did not have any join information, but the rawRelations did! Using the provided buildAlias function, I was able to create the required property paths, e.g. order_customerId when a customer relation was expected to exist on an order entity. So I modified the filter to run on the raw data and then only returned the mapped entities where the raw indices matched. This solution works (for me) without modifying my entity/DTO files.

Next steps are to hopefully receive feedback on this issue (paging @TriPSs ! thanks for maintaining), then creating a PR if acceptable and finally switching back from my hacky local dependency solution.

from nestjs-query.

I created a pull request for my proposed solution: #175

from nestjs-query.

@coupster74 can you confirm that this fixes your problem as well?

from nestjs-query.

I'll give it a go (likely next week). I did the work to implement the key columns in the schema and there were a number of nuances I would want to test out related to query depth, aggregation, etc.

from nestjs-query.