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.

Sign up for GitHub

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

Jump to bottom

Define AfterFind behavior for Eager connections #476

Closed
aeneasr opened this issue Nov 27, 2019 · 1 comment · Fixed by #802
Closed

Define AfterFind behavior for Eager connections #476

aeneasr opened this issue Nov 27, 2019 · 1 comment · Fixed by #802
Labels
f: associations the associations feature in pop s: triage Some tests need to be run to confirm the issue
Milestone
v6.1.1

Comments

@aeneasr
Copy link
Member

aeneasr commented Nov 27, 2019

Description

At the moment, AfterFind is being executed before fetching any other relations (using Eager()). This is not documented very well and maybe even unexpected.

Steps to Reproduce the Problem

Have a model with at least one has_many relation defined and a value receiver AfterFind().

Expected Behavior

Three options:

  1. Document this behavior as a limitation
  2. Add another method such as AfterFindEager
  3. Run AfterFind() after fetching relations if an eager connection is made.
@aeneasr
Copy link
Member Author

aeneasr commented Nov 27, 2019

It appears that AfterCreate and AfterUpdate are executed after all relations have been inserted when using Eager(). I would expect consistent behavior across callbacks.

@aeneasr aeneasr mentioned this issue Nov 27, 2019
Merged
aeneasr added a commit to ory/kratos that referenced this issue Dec 1, 2019
@aeneasr
Replace DBAL layer with gobuffalo/pop (#130)
215e13b 
This is a major refactoring of the internal DBAL. After a successful proof of concept and evaluation of gobuffalo/pop, we believe this to be the best DBAL for Go at the moment. It abstracts a lot of boilerplate code away.

As with all sophisticated DBALs, pop too has its quirks. There are several issues that have been discovered during testing and adoption: gobuffalo/pop#136 gobuffalo/pop#476 gobuffalo/pop#473 gobuffalo/pop#469 gobuffalo/pop#466

However, the upside of moving much of the hard database/sql plumbing into another library cleans up the code base significantly and reduces complexity.

As part of this change, the "ephermal" DBAL ("in memory") will be removed and sqlite will be used instead. This further reduces complexity of the code base and code-duplication.

To support sqlite, CGO is required, which means that we need to run tests with `go test -tags sqlite` on a machine that has g++ installed. This also means that we need a Docker Image with `alpine` as opposed to pure `scratch`. While this is certainly a downside, the upside of less maintenance and "free" support for SQLite, PostgreSQL, MySQL, and CockroachDB simply outweighs any downsides that come with CGO.
aeneasr added a commit to ory/kratos that referenced this issue Dec 1, 2019
@aeneasr
Replace DBAL layer with gobuffalo/pop (#130)
21d08b8 
This is a major refactoring of the internal DBAL. After a successful proof of concept and evaluation of gobuffalo/pop, we believe this to be the best DBAL for Go at the moment. It abstracts a lot of boilerplate code away.

As with all sophisticated DBALs, pop too has its quirks. There are several issues that have been discovered during testing and adoption: gobuffalo/pop#136 gobuffalo/pop#476 gobuffalo/pop#473 gobuffalo/pop#469 gobuffalo/pop#466

However, the upside of moving much of the hard database/sql plumbing into another library cleans up the code base significantly and reduces complexity.

As part of this change, the "ephermal" DBAL ("in memory") will be removed and sqlite will be used instead. This further reduces complexity of the code base and code-duplication.

To support sqlite, CGO is required, which means that we need to run tests with `go test -tags sqlite` on a machine that has g++ installed. This also means that we need a Docker Image with `alpine` as opposed to pure `scratch`. While this is certainly a downside, the upside of less maintenance and "free" support for SQLite, PostgreSQL, MySQL, and CockroachDB simply outweighs any downsides that come with CGO.
@Jwonsever Jwonsever mentioned this issue May 31, 2020
Open
@sio4 sio4 added s: triage Some tests need to be run to confirm the issue f: associations the associations feature in pop labels Sep 20, 2022
@sio4 sio4 added this to the v6.1.0 milestone Sep 20, 2022
aeneasr added a commit that referenced this issue Jan 2, 2023
@aeneasr
Add AfterEagerFind()
3596719 
See #557
Closes #476

Signed-off-by: aeneasr <3372410+aeneasr@users.noreply.github.com>
@aeneasr aeneasr mentioned this issue Jan 2, 2023
Merged
@sio4 sio4 closed this as completed in #802 Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
f: associations the associations feature in pop s: triage Some tests need to be run to confirm the issue
Projects
None yet
Milestone
v6.1.1
Development

Successfully merging a pull request may close this issue.

Add AfterEagerFind() gobuffalo/pop
2 participants
@sio4 @aeneasr