Proposal to modularize the guardrail repo

blast-hardcheese commented 3 years ago

Problem

Currently, guardrail has no explicit library dependencies brought into consuming projects, permitting users to supply their own implementations that satisfy source compatibility. This is a core design goal of guardrail, and one that I aim to maintain.

One challenge, however, is that any change to any source generators for any language necessitates a version bump for the whole project, leading to unnecessary upgrade PRs and work for users, for explicitly no benefit.

Explicitly, a user who only uses Scala source generators should not have to bump guardrail if only the Java generators have changed, and vice versa.

Proposed solution

Core guardrail repo

The guardrail project will be further modularized, instead of emitting

dev.guardrail::guardrail:x.y.z

we will instead release

dev.guardrail::guardrail-core:a.b.c
dev.guardrail::guardrail-${language}-${library}:x.y.z

where each individual module has its own version.sbt, distinct from guardrail-core.

As it stands today, this will end up with the following:

dev.guardrail::guardrail-core:a.b.c
dev.guardrail::guardrail-scala-akka-http:d.e.f
dev.guardrail::guardrail-scala-http4s:g.h.i
dev.guardrail::guardrail-scala-dropwizard:j.k.l
dev.guardrail::guardrail-java-dropwizard:m.n.o
dev.guardrail::guardrail-java-spring-boot:p.q.r

Build tools (sbt)

For sbt-guardrail, this will have two ramifications.

First, the modules configuration will move into sbt-guardrail.

Instead of:

// project/guardrail.sbt
addSbtPlugin("dev.guardrail" % "sbt-guardrail" % "a.b.c")

// build.sbt
Compile / guardrailTasks += ScalaClient(file("server.yaml"), pkg="example.client", framework="http4s")

we would instead have:

// project/guardrail.sbt
addSbtPlugin("dev.guardrail" % "sbt-guardrail" % "a.b.c")
libraryDependencies += "dev.guardrail" %% "guardrail-scala-http4s" % "d.e.f"

// build.sbt
Compile / guardrailTasks += ScalaClient(file("server.yaml"), pkg="example.client").modules(_.http4s)

where a proof-of-concept of how the modules might be implemented being

object GuardrailModules
def modules[L <: LA, F[_]](holders: (GuardrailModules.type => Holder[L, F])*): Types.Args = ...

with sbt-guardrail depending on all known guardrail-published modules using the Provided scope.

GuardrailModules being a special type, similar to the magnet pattern, wherein the following implicit conversions would be supplied:

implicit def holderFromServerTerms[L <: LA, F[_]](value: ServerTerms[L, F]): Holder[L, F] = new ServerHolder[L, F](value)
implicit def holderFromClientTerms[L <: LA, F[_]](value: ClientTerms[L, F]): Holder[L, F] = new ClientHolder[L, F](value)
implicit def holderFromProtocolTerms[L <: LA, F[_]](value: ProtoTerms[L, F]): Holder[L, F] = new ProtosHolder[L, F](value)

implicit def holderBuilderFromServerTerms[L <: LA, F[_]](value: ServerTerms[L, F]): Function1[GuardrailModules.type, Holder[L, F]] = _ => new ServerHolder[L, F](value)
implicit def holderBuilderFromClientTerms[L <: LA, F[_]](value: ClientTerms[L, F]): Function1[GuardrailModules.type, Holder[L, F]] = _ => new ClientHolder[L, F](value)
implicit def holderBuilderFromProtocolTerms[L <: LA, F[_]](value: ProtoTerms[L, F]): Function1[GuardrailModules.type, Holder[L, F]] = _ => new ProtosHolder[L, F](value)

which has the additional benefit of permitting ad-hoc extensions of guardrail functionality:

val myCustomHttp4sClient = GuardrailModules.http4s.client.copy(
  newGetExtraImports={ tracingEnabled =>
    import scala.meta._
    dev.guardrail.Target.pure(List(q"import com.example.myInstances._"))
  }
)

Compile / guardrailTasks += ScalaClient(file("server.yaml"), pkg="example.client").modules(myCustomHttp4sClient, _.circe),

... or for extensions to core functionality to be released as a library itself and depended on:

// project/guardrail.sbt
addSbtPlugin("dev.guardrail" % "sbt-guardrail" % "a.b.c")
libraryDependencies += "com.example" %% "guardrail-scala-http4s-extensions" % "d.e.f"

// build.sbt
Compile / guardrailTasks += ScalaClient(file("server.yaml"), pkg="example.client").modules(com.example.myCustomHttp4sClient)

Build tools (maven)

Similar to sbt, maven would utilize runtime class loading from fully-qualified class names (FQCN) in order to describe modules, with modular dependencies specified such that they would be available to generate-sources:

<plugin>
  <groupId>dev.guardrail</groupId>
  <artifactId>guardrail-maven-plugin</artifactId>
  <executions>
    <execution>
      <id>generate-myservice-server</id>
      <goals>
        <goal>generate-sources</goal>
      </goals>
      <configuration>
        <language>scala</language>
        <specPath>${project.basedir}/src/main/swagger/my-service.yaml</specPath>
        <packageName>com.example.server</packageName>
        <modules>
          <module>dev.guardrail.generators.Java.DropwizardServerGenerator.ServerTermInterp</module>
          <module>dev.guardrail.generators.Java.JacksonGenerator</module>
        </modules>
      </configuration>
    </execution>
  </executions>
  <dependencies> <!-- This would be new, permitting custom com.example:my-dropwizard-extensions:g.h.i to be used in the <modules> section above -->
    <dependency>
      <groupId>dev.guardrail</groupId>
      <artifactId>guardrail-java-dropwizard</artifactId>
      <version>a.b.c</version>
    </dependency>
    <dependency>
      <groupId>dev.guardrail</groupId>
      <artifactId>guardrail-java-jackson</artifactId>
      <version>d.e.f</version>
    </dependency>
  </dependencies>
</plugin>

This would reduce the need to release the guardrail-maven-plugin to only ABI-incompatible releases, again similar to the sbt plugin, with guardrail module updates being opened against the section under the plugin directly.

Build tools (gradle)

Presumably the changes to the gradle plugin will be somewhere between the sbt and maven changes, but I really don't have much confidence in my ability to design around gradle at this time.

TL;DR

I believe it would be better for users if...

... the guardrail repo were modular,
... dependencies on modules were specified individually,
... build tools were modified to permit specifying implementations directly,
... versions bumped were only what is actually used

I believe it would be better for maintainers if...

... the guardrail build plugins needed to be released less frequently
... providing bespoke extensions to guardrail were possible through the official build plugins
... the core supported modules stayed in a monorepo to prevent ABI drift and maintenance headache

kelnos commented 3 years ago

I don't love the <module> syntax there, making users type out fully-qualified class names. I think we could do some things here to simplify:

Have a convention for the modules we ship, so that it's always dev.guardrail.generators.{language}.{generator-name}.Interp (or something). Then a user would only have to specify {generator-name} and we'd fill in the rest. If we see a FQCN, then we use it as is.
We could also package a small description file in the generator jars at well-known locations. We'd want to be careful here so that people building fat JARs don't run into conflicts. Or make the format simple enough that sbt-assembly/maven-shade-plugin can use their "concat" strategy to deal with conflicts. Then the core can just look for those files on the classpath and translate the long class names into shorter module names.

blast-hardcheese commented 3 years ago

Option 1 feels kind of like how Gradle handles plugin registration, I'm not super thrilled about the lack of flexibility.

I think 2 makes sense, and kind of parallels how openapi-parser registers classpath transformers. I'm not super thrilled with adding the mapping files into the core modules though.

What about having the mapping done in the plugin itself, exposing short names but giving the user the opportunity to specify FQCNs only if they truly need something custom?

kelnos commented 3 years ago

Option 1 feels kind of like how Gradle handles plugin registration, I'm not super thrilled about the lack of flexibility.

How is that not flexible? It's a shortcut for the generators we distribute, but we'd still accept FQCNs in that spot for third-party things that don't conform to the 'standard' naming.

What about having the mapping done in the plugin itself, exposing short names but giving the user the opportunity to specify FQCNs only if they truly need something custom?

I'd rather not; this just means we have the same list in three places that will need to be updated.

nickhudkins commented 3 years ago

Yes 🙌. That is all ☺️. Let me know if there is grunt work you would like done. I'm happy to participate!

blast-hardcheese commented 3 years ago

Good progress! #1280 actually works

blast-hardcheese commented 3 years ago

Repackaging notes, for people implementing against guardrail internals: https://gist.github.com/blast-hardcheese/1c4dcfe2151510dfc87ec8e384d87cd1

blast-hardcheese commented 9 months ago

So, with 1.0.0-SNAPSHOTs being published and everything working reasonably well, I think it's fair to close this out. 🎉

guardrail-dev / guardrail