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

Conflict between ServiceClass and AssemblerClass #167

Closed
bzp2010 opened this issue Aug 16, 2023 · 3 comments
Closed

Conflict between ServiceClass and AssemblerClass #167

bzp2010 opened this issue Aug 16, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@bzp2010
Copy link

bzp2010 commented Aug 16, 2023

Describe the bug

When using AutoResolver and both ServiceClass and AssemblerClass are set, only AssemblerClass will take effect and ServiceClass will not work.

Have you read the Contributing Guidelines?

Yes

To Reproduce
Steps to reproduce the behavior:

  1. Step 1
    Define the following resources.
@Module({
  imports: [
    NestjsQueryGraphQLModule.forFeature({
      imports: [NestjsQueryTypeOrmModule.forFeature([Test])],
      services: [TestService],
      assemblers: [TestAssembler],
      resolvers: [
        {
          DTOClass: TestDTO,
          ServiceClass: TestService,
          AssemblerClass: TestAssembler,
        },
      ],
    }),
  ],
})
export class APISourceModule {}
@QueryService(Test)
export class TestService extends TypeOrmQueryService<Test> {
  constructor(
    @InjectRepository(Test) repo: Repository<Test>,
  ) {
    super(repo, { useSoftDelete: true });
  }

  createOne(
    record: DeepPartial<APISourceInputDTO>,
  ): Promise<ProviderAPISource> {
    console.log("Service createOne!");
    return super.createOne(apiSource);
  }
}
@Assembler(TestDTO, Test)
export class TestAssembler extends ClassTransformerAssembler<TestDTO, Test> {
  convertToCreateEntity(dto: DeepPartial<TestDTO>): DeepPartial<TestEntity> {
    console.log("Assembler convertToCreateEntity!");
    return dto;
  }
}
  1. Step 2

Calling the GQL Mutation API createOneTest and observing the terminal, I only see Assembler convertToCreateEntity!.

  1. Step 3

https://github.com/TriPSs/nestjs-query/blob/master/packages/query-graphql/src/providers/resolver.provider.ts#L167-L185

function createResolver<
  DTO,
  EntityServiceOrAssembler extends DeepPartial<EntityServiceOrAssembler>,
  C,
  U,
  R,
  PS extends PagingStrategies
>(resolverOpts: AutoResolverOpts<DTO, EntityServiceOrAssembler, C, U, R, PS>): Provider {
  if (isFederatedResolverOpts(resolverOpts)) {
    return createFederatedResolver(resolverOpts)
  }
  if (isAssemblerCRUDAutoResolverOpts(resolverOpts)) {
    return createAssemblerAutoResolver(resolverOpts)
  }
  if (isServiceCRUDAutoResolverOpts(resolverOpts)) {
    return createServiceAutoResolver(resolverOpts)
  }
  return createEntityAutoResolver(resolverOpts)
}

According to its logic, Assembler and CustomService are mutually exclusive, and the Assembler takes precedence over the Service to take effect.

Expected behavior

When using Custom Service and Assembler at the same time, make them automatically form a chain call, i.e. the Service automatically uses the Assembler for data conversion.

According to the documentation, the Assembler acts as a bridge between the DTO and the Service, and the Custom Service is apparently not included here, which is a bit strange, to be honest.

I noticed that this project's predecessor was also implemented this way, was this by any particular design?

Users often use Custom Services for purposes such as soft deletion, but Assembler still has a role to play, such as injecting foreign keys into the entity, such as createdBy.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • Node Version [e.g. 14.14.0] 18.16.1
  • Nestjs-query Version [e.g. v0.21.0] 4.0.0

Additional context
Add any other context about the problem here.

@bzp2010 bzp2010 added the bug Something isn't working label Aug 16, 2023
@TriPSs
Copy link
Owner

TriPSs commented Aug 16, 2023

I have to say that I personally do not use assembles, looking at the code I have the feeling that the docs are either missing some information on how to make this work (As I have the feeling the AssemblerQueryService should be used somewhere).

I would need to take a deeper look into how these parts work before I could help you with the issue.

@smolinari
Copy link
Contributor

smolinari commented Aug 16, 2023

Yeah. In my POC with assemblers, I got the automation along with the assembler working by extending the AssemblerQueryService as @TriPSs suggests.

@Assembler(UserDTO, UserEntity)
export class UserAssembler extends ClassTransformerAssembler<UserDTO, UserEntity> {}

@Injectable()
@QueryService(UserDTO)
export class UserService extends AssemblerQueryService<UserDTO, UserEntity> {
  constructor (
    readonly assembler: UserAssembler,
    @InjectQueryService(UserEntity) private readonly userService: QueryService<UserEntity>
  ) {
    super(assembler, userService)
  }

And of course, I entered my UserAssember in the module's assemblers property .

Seems the AssemblerQueryService does the work, when you give it the assembler and the pertinent service. Don't ask me how I found that. Haha... I can't remember.

Edit: Yeah, I probably just took a look at this and said to myself, why not just extend it?
https://github.com/TriPSs/nestjs-query/blob/master/packages/core/src/services/assembler-query.service.ts

Scott

@TriPSs
Copy link
Owner

TriPSs commented Dec 5, 2023

Closing, if still an issue please reopen.

@TriPSs TriPSs closed this as completed Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants