loopbackio / loopbackio/loopback-next

Regression: secondary HasManyThrough inclusion fails with circular dependency

Open
#7,599 23 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug IoC/Context needs grooming Repository
Dominant language
TypeScript
Stars
5.1k
Forks
1.1k
Avg merge
2d 21h
Merged PRs (30d)
27

Description

After the latest release (and possibly the one before - hard to test), a circular dependency error triggers where none did before.

Steps to reproduce

  1. Create 3 models, e.g. User, Participant, Conversation
  2. Set up bi-directional HasManyThrough relations (User through Participant to Conversation, Conversation through Participant to User)
  3. Use one repository to find instances of the other and include the first, e.g.
    userRepo.conversations(userId).find({include:['users']});
  4. Execute create: userRepo.conversations(userId).create({});
  5. Execute find again: userRepo.conversations(userId).find({include:['users']});

Current Behavior

The first execution of find succeeds. The second fails with (using Todo <-> Assignment <-> Person from linked branch):

Error: Circular dependency detected: repositories.TodoRepository --> @TodoRepository.constructor[2] --> repositories.PersonRepository --> @PersonRepository.constructor[2] --> repositories.TodoRepository
      at ResolutionSession.pushBinding (/home/X/project/loopback-next/packages/context/src/resolution-session.ts:228:13)
      at Function.runWithBinding (/home/X/project/loopback-next/packages/context/src/resolution-session.ts:107:13)
      at Binding.getValue (/home/X/project/loopback-next/packages/context/src/binding.ts:535:40)
      at TodoListApplication.getValueOrPromise (/home/X/project/loopback-next/packages/context/src/context.ts:912:32)
      at TodoListApplication.get (/home/X/project/loopback-next/packages/context/src/context.ts:712:17)
      at getter (/home/X/project/loopback-next/packages/context/src/inject.ts:444:16)
      at fetchHasManyThroughModels (/home/X/project/loopback-next/packages/repository/src/relations/has-many/has-many-through.inclusion-resolver.ts:88:30)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at /home/X/project/loopback-next/packages/repository/src/relations/relation.helpers.ts:112:21
      at async Promise.all (index 0)
      at Object.includeRelatedModels (/home/X/project/loopback-next/packages/repository/src/relations/relation.helpers.ts:120:3)

Expected Behavior

Second execution of find on the HasManyThroughRepositoryFactory should succeed as well.

Link to reproduction sandbox

The Todo example at https://github.com/0x0aNL/loopback-next/tree/hmt-circdep-regression is set up with bi-directional HasManyThrough relations: Todo <-> Assignment <-> Person. A test case in src/__tests__/acceptance/todo.acceptance.ts executes:

await todoRepo.people(1).find({include: ['todos']});
await todoRepo.people(1).create({});
await todoRepo.people(1).find({include: ['todos']});

Additional information

> node -e 'console.log(process.platform, process.arch, process.versions.node)'
linux x64 16.1.0
> npm ls --prod --depth 0 | grep loopback
@loopback/example-todo@3.11.1 /home/X/project/loopback-next/examples/todo
├── @loopback/boot@3.4.1
├── @loopback/core@2.16.1
├── @loopback/repository@3.7.0
├── @loopback/rest-explorer@3.3.1
├── @loopback/rest@9.3.1
├── @loopback/service-proxy@3.2.1
├── loopback-connector-rest@4.0.1

It looks as though this regression is introduced between @loopback/repository@3.5.0 / @loopback/core@2.15.0 and @loopback/repository@3.6.0 / @loopback/core@2.16.0.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with packages/repository/src/relations/has-many/has-many-through.inclusion-resolver.ts and the Todo reproduction in src/tests/acceptance/todo.acceptance.ts. Compare the repository/core changes between versions 3.5.0/2.15.0 and 3.6.0/2.16.0, then run the three-step find/create/find sequence. Done means the second included find succeeds without a circular dependency error.

Written by the indexing model from the issue text.

Assessment

Tech stack
nodejs, typescript
Domain
backend-api-design, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.