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

@Flag annotation doesn't work with Seq/List #431

Closed
amartinsn opened this issue Aug 25, 2017 · 3 comments
Closed

@Flag annotation doesn't work with Seq/List #431

amartinsn opened this issue Aug 25, 2017 · 3 comments
Labels

Comments

@amartinsn
Copy link

Getting an error when defining a flag that returns a Seq and try to inject that in a Controller implementation using the @Flag annotation.

Expected behavior

Given this flag defined within a TwitterModule:

flag("things", Seq[String]("foo", "bar"), "A list of things")

When I inject that flag in a Controller:

class MyController @Inject()(
  @Flag("things") things: Seq[String]) extends Controller {
   ...
}

Then things val should be Seq("foo", "bar")

Actual behavior

Currently I'm getting the following error when bootstrapping the server:

com.google.inject.ConfigurationException: Guice configuration errors:

1) No implementation for scala.collection.Seq<java.lang.String> annotated with @com.twitter.inject.annotations.Flag(value=enabled) was bound.
  while locating scala.collection.Seq<java.lang.String> annotated with @com.twitter.inject.annotations.Flag(value=enabled)
    for parameter 0 at com.example.MyController.<init>(MyController.scala:11)
  while locating com.example.MyController

Steps to reproduce the behavior

The steps are pretty much defined in the Expected behavior section. Please let me know if you need more details.

@amartinsn
Copy link
Author

It might have something to do with the Flag type information being hardcoded as String.

https://github.com/twitter/finatra/blob/develop/inject/inject-app/src/main/scala/com/twitter/inject/app/internal/FlagsModule.scala#L18

@cacoco cacoco added the backlog label Sep 11, 2017
@cacoco
Copy link
Contributor

cacoco commented Dec 14, 2017

Just an update here. Flags don't currently carry enough type information to allow for correctly binding a parameterized type (like Seq or List). We could do better for binding non-parameterized types instead of always a String but without some re-working of Flags this is pretty challenging. And even then it's non-trivial to parse the parameterized types correctly via reflection to provide the correct type key for Guice binding.

@cacoco
Copy link
Contributor

cacoco commented Jun 23, 2020

Good news! In 92a4706, @vkostyukov added TypeConversions which support converting Seq and List of Flaggable types meaning that you can now define a Flag flag("things", Seq[String]("foo", "bar"), "A list of things") and inject it via:

class MyController @Inject()(
  @Flag("things") things: Seq[String]) extends Controller {
   ...
}

as we now have registered Guice TypeConverters to translate the bound String type into a Seq/List of the Flaggable type. This should work for any Flag defined as a Seq/List of a Flaggable type.

@cacoco cacoco closed this as completed Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants