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

Provide literals for built-in annotations #82

Closed
arjantijms opened this issue Jul 4, 2018 · 4 comments
Closed

Provide literals for built-in annotations #82

arjantijms opened this issue Jul 4, 2018 · 4 comments

Comments

@arjantijms
Copy link
Contributor

Since CDI 2.0, CDI provides annotation literals for many of its annotations (see https://issues.jboss.org/browse/CDI-485).

Just as the built-in CDI annotations, EE Security annotation literals are also being implemented by projects. As a public example, see https://github.com/javaee-samples/javaee8-samples/blob/master/security/dynamic-rememberme/src/main/java/org/javaee8/security/dynamic/rememberme/util/RememberMeAnnotationLiteral.java

Literals are useful for looking up bean instances using the bean manager, as well as when applying interceptors dynamically.

@arjantijms arjantijms added this to the 1.1 milestone Jul 4, 2018
arjantijms added a commit that referenced this issue Jul 4, 2018
* #82 - Added simple annotation literal for AutoApplySession

Signed-off-by: arjantijms <arjan.tijms@gmail.com>

* #82 - Literals for built-in annotations

Signed-off-by: arjantijms <arjan.tijms@gmail.com>
@keilw
Copy link
Member

keilw commented Jul 4, 2018

Why a no-op of() method, wouldn't INSTANCE also work here?

An of() without any argument, see Optional is rather unusual in Java SE, or did CDI pick it up somewhere?

@arjantijms
Copy link
Contributor Author

arjantijms commented Jul 4, 2018

Why a no-op of() method, wouldn't INSTANCE also work here?

With INSTANCE you'd expect a constant. The CDI examples all use of() but with a parameter (since all these only have 1 annotation attribute).

There needs to be something to kickoff the builder. Open to suggestions in a followup PR.

@arjantijms
Copy link
Contributor Author

P.s. an alternative I was playing with is:

RememberMe literal = RememberMe.Literal.of(
    cookieMaxAgeSeconds(100),
    cookieSecureOnly(false),
    cookieHttpOnly(false)
 );

Where cookieMaxAgeSeconds etc are statically imported methods that return a NamedParameter instance. The of() method is a varargs method exception these. It's roughly like how the Java SE 8 stream method collect() works.

It reads a bit nicer, but you'd have to import these methods, which may be a bit less self-discovery than chaining builders calls.

Thoughts?

@keilw
Copy link
Member

keilw commented Jul 4, 2018

I'll propose something along the lines of

RememberMe literal = RememberMe.Literal.builder()
                                       .cookieMaxAgeSeconds(100)
                                       .cookieSecureOnly(false)
                                       .cookieHttpOnly(false)
                                       .build();

As the method does not return Literal itself, but the LiteralBuilder. And look at Optional and all other cases I know, of(), valueOf() or instanceOf() always return the exact class they're applied on, and not a builder.

@arjantijms arjantijms reopened this Jul 4, 2018
keilw added a commit that referenced this issue Jul 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants