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

Implement wireResource #173

Open
schrepfler opened this issue Jul 23, 2021 · 12 comments
Open

Implement wireResource #173

schrepfler opened this issue Jul 23, 2021 · 12 comments

Comments

@schrepfler
Copy link

Implement some sort of deferred wiring which can express some sort of dependency delayed in time. This would allow more precise loading of components when they're ready to be loaded thus not kicking all dependency injection at bootstrap which often kicks off a lot of work for nothing.
Unsure what's the best approach in terms of expressing the dependency, chaining effect types? How to deal with initialization failures... maybe lean on concrete effect types error handling mechanism.

@adamw
Copy link
Member

adamw commented Jul 24, 2021

I imagined sth along the lines:

wireResource[F, T]: Resource[F, T]

I think it would make sense to make it recursively search for values, just as wireRec. Example usage would then be:

class DatabaseAccess()
class SecurityFilter()
class UserFinder(databaseAccess: DatabaseAccess, securityFilter: SecurityFilter)
class UserStatusReader(userFinder: UserFinder)

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  // some code to initialize & cleanup a pool of database connections
  lazy val theDatabaseAccess: Resource[F, DatabaseAccess] = ???
  lazy val theUserStatusReader: Resource[F, UserStatusReader] = wireResource[UserStatusReader]
}

would generate sth along the lines of:

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  // some code to initialize & cleanup a pool of database connections
  lazy val theDatabaseAccess: Resource[F, DatabaseAccess] = ???
  lazy val theUserStatusReader: Resource[F, UserStatusReader] = theDatabaseAccess.map { da =>
    new UserFinder(da, new SecurityFilter()))
  }
}

Multiple resources would be flatmapped. Plus we would need to figure out the right order of flatmapping, so that all necessary values are in scope. Wdyt?

@mbore
Copy link
Contributor

mbore commented Jul 24, 2021

Looks promising for me, but I would rather name the function that @adamw proposed wireResourceRec to keep it "compatible" with wire/ wireRec.

When it comes to the Resource I think that it makes sens to use existing cats/zio implementations (It would be great to make that macro resource-implementation-agnostic and prepare separate integration modules for cats & zio which would only choose the right resource implementation, but not sure if it's possible) .

@adamw
Copy link
Member

adamw commented Jul 26, 2021

@mbore yeah I was thinking about it, wireResourceRec is definitely more consistent, but it's also quite ugly ;) And since wiring resources makes most sense recursively (otherwise you can just flatMap + wire), I think the shorter name is justified.

I agree that we should start with cats-effect. ZIO has their own dependency management system (through environment), so I think we'll have a tougher timer convincing them that our way is better ;) We can later try to generalise through a Resource-like typeclass.

As for concrete implementation, it will have to diverge a bit from the current design:

  1. first, we need to create the dependency graph, where each node is an object that needs to be constructed (either through a constructor, a factory etc.) or that is available in scope (the dependencies that are explicitly created by the user)
  2. in the graph, we need to find all Resources and determine the appropriate order of creating them (though this might not be strictly necessary, it probably makes most sense to create the resources according as induced by the transitive dependencies in the graph; plus, some resources might be created depending on previously created values)
  3. having all resources in place, we can proceed with normal wiring

We could also use this graph to provide better error reporting - when a value is not found, we can print the path to that dependency, from the wiring root.

@mbore
Copy link
Contributor

mbore commented Aug 17, 2021

I discussed this feature with @adamw and we would like to start with a slightly simplified version of this feature. We would like to make wireResource less "magical", therefore at the beginning we would like to enforce explicit declaration of all required resources. It would require passing a list of available resources to wireResource, so it could be used like:

class DatabaseAccess()
class EsAccess()
class SecurityFilter()
class UserFinder(databaseAccess: DatabaseAccess, esAccess: EsAccess, securityFilter: SecurityFilter)
class UserStatusReader(userFinder: UserFinder)

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  lazy val theDatabaseAccess: Resource[F, DatabaseAccess] = ???
  lazy val theEsAccess: Resource[F, EsAccess] = ???

  lazy val theUserStatusReader: Resource[F, UserStatusReader] = wireResource[UserStatusReader](theDatabaseAccess, theEsAccess)
}

and we should generate smth like

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  lazy val theDatabaseAccess: Resource[F, DatabaseAccess] = ???
  lazy val theEsAccess: Resource[F, EsAccess] = ???

  lazy val theUserStatusReader: Resource[F, UserStatusReader] = theDatabaseAccess.flatMap(da => theEsAccess.map { ea =>
    new UserStatusReader(new UserFinder(da, ea, new SecurityFilter()))
  }
}

By "less magical" solution, I mean that we want to eliminate the following problems:

  • missing imports or mixed-in traits - it may be difficult to debug recursive scopes search, therefor we would like to get rid of it
  • autogenerating more than we want to - e.g. let's consider simple config class which we has an empty constructor and bunch of var fields (we need to define it this way because of the framework we use). We may at some point forget to create an filled instance of this config, which would cause autogeneration of an empty config instance, which would cause runtime error

In my opinion we should chain (flatMap) resources in the same order in which they were passed to wireResource.

In the next step we would like to take also as parameters functions which return Resorce e.g.

class DatabaseConfig private ()

class DatabaseAccess()
object DatabaseAccess {
  def apply[F[_]](cfg: DatabaseConfig): Resource[F, DatabaseAccess] = ???
}

class SecurityFilter()
class UserFinder(databaseAccess: DatabaseAccess, securityFilter: SecurityFilter)
class UserStatusReader(userFinder: UserFinder)

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  lazy val databaseConfig: DatabaseConfig = ???
  
  lazy val theUserStatusReader: Resource[F, UserStatusReader] = wireResource[UserStatusReader](databaseConfig, DatabaseAccess.apply)
}

and we should generate

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  lazy val databaseConfig: DatabaseConfig = ???
  
  lazy val theUserStatusReader: Resource[F, UserStatusReader] = DatabaseAccess.apply(databaseConfig).map(da =>   new UserStatusReader(new UserFinder(da, new SecurityFilter())))

But in this case it would not be so easy to set up the order of generated resources

@adamw
Copy link
Member

adamw commented Aug 17, 2021

@mbore thanks for the summary :) I think the ordering should be left to the library - that is, this should be the library's problem. It's trivial when resources don't have dependencies, but once they do have them, it's not necessary to put the burden of properly ordering the definitions on the user.

I'd go even a step further - making it necessary to list all dependencies, that shouldn't be auto-generated, as explicit parameters of wireResource (or wireApp, or however we end up calling this). Though I know you're not a fan (yet? ;) ) of this approach. But I think that explicitly defined dependencies are rare, and that most will be auto-generated anyway. On the plus side, it also removes all of the scoping rules as to where we look for instances, which makes the whole mechanism easier to understand.

@mbore
Copy link
Contributor

mbore commented Aug 17, 2021

Regarding "explicite list of all dependencies" I was thinking about it some time and I think that it's a good idea to start with this approach (because it's simpler :)) and later on we may consider some kind of recursive scopes. I think that implementing it this way would be welcomed from the users point of view, because it would be easier to keep the backward compatibility.

Edited my previous comment to stress that we will require all "impure" dependencies.

@schrepfler
Copy link
Author

Which part of the design addresses the staged sequencing of bootstrap? In my mind this feature should allow a less chaotic bootstrap (example waiting for cluster forum to bootstrap or similar, long-running procedures).

@adamw
Copy link
Member

adamw commented Aug 18, 2021

@schrepfler it probably doesn't, as we've focused on other aspects :) And I think I don't fully understand your requirements. What should separate the bootstrap stages? Would you like to specify some specific ordering in which Resources are combined? Maybe that's partially adressed by what @mbore proposed, that the resources are combines in the order in which they are given to wireResource (unless there are dependencies)

@schrepfler
Copy link
Author

schrepfler commented Aug 18, 2021

So, this came as an idea when we were discussing the bootstrap of Lagom (and thus Guice injection and Akka Cluster and Kafka consumers and producers). In that case your entire system is maxing out at 100% CPU and can't understand what's going all. We're hypothesising that if we'd bootstrap everything in a controlled and staged approach we'd probably have a much smoother start and the system would get stable sooner.

@adamw
Copy link
Member

adamw commented Aug 19, 2021

@schrepfler Ah I see :) so how would you like the control to look - is it the ordering of when the resources are created? Or the general order in which dependency instances are created? Or maybe you're looking to parametrising parallelism of resource creation as well?

I think some example fake-service classes and the target code which you'd like to generate would help in design.

@mbore
Copy link
Contributor

mbore commented Aug 27, 2021

I started working on this feature and my first attempt (which you can find in #175 ) is a really simple one. Basically I compose provided resources and then run simple wire operation, which may be express as transformation from

class DatabaseAccess()
class EsAccess()
class UserFinder(databaseAccess: DatabaseAccess, esAccess: EsAccess)

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  lazy val theDatabaseAccess: Resource[F, DatabaseAccess] = ???
  lazy val theEsAccess: Resource[F, EsAccess] = ???

  lazy val theUserStatusReader: Resource[F, UserFinder] = wireResource[UserFinder](theDatabaseAccess, theEsAccess)
}

into

trait UserModule[F[_]] {
  import com.softwaremill.macwire._

  lazy val theDatabaseAccess: Resource[F, DatabaseAccess] = ???
  lazy val theEsAccess: Resource[F, EsAccess] = ???

  lazy val theUserStatusReader: Resource[F, UserFinder] = for {
    da <- theDatabaseAccess
    ea <- theEsAccess.map
  } yield wire[UserFinder]
}

Now I'm going to replace wire with wireRec-like behaviour.

@schrepfler Your idea seems to be really interesting, but we definitely need more details to design it :)

@adamw
Copy link
Member

adamw commented Sep 2, 2021

@mbore Looks like a great first step - esp if it works :)

I'd like to go a step further with wireResource, abandoning the search that wire performs entirely. So I'm not sure if we'll be able to reuse it in the end. Essentially, any dependencies that shouldn't be created using a constructor, should be passed explicitly as parameter (or a Resource, IO, factory method).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants