cdi-468 Split Spec doc to introduce CDI Lite by antoinesd · Pull Request #470 · jakartaee/cdi

antoinesd · 2021-03-18T11:24:12Z

This PR solves #cdi-468

Signed-off-by: antoinesd antoine@sabot-durand.net

antoinesd · 2021-03-18T11:25:05Z

I still have some modifications to add before a first review

spec/src/main/asciidoc/preface.asciidoc

manovotn · 2021-03-23T12:49:56Z

I still have some modifications to add before a first review

I am aware of this but I still got to glance at it and thought I might leave some notes for you if you don't mind :)

FTR any chapter numbers I mention below are from your snapshot.

We need to add something akin to Packaging and deployment to Lite part which will describe the way discovery works for Lite
- I think the only proposal we acknowledged as working ATM is the one I wrote to the mailing list, but even if you leave it blank for now, we should have the chapter in place not to forget it
Chapter 2.1.5. Default bean discovery mode is linked to the point above; I agree we want annotated but I don't think we found a way to achieve that in a non-breaking way without stating it in beans.xml
Any and all mentions of beans.xml used for enablement have to be moved into Full (probably a separate chapter that will mention that you can use beans.xml for local enablement?)
- I saw this in chapter about alternatives (2.3.1.1 Declaring selected alternatives) and interceptors (2.6.4 Interceptor enablement and ordering)
Parts of 3.1. Inheritance and specialization need to be moved to Lite; namely the inheritance still holds true for Lite, it is only the specialization that belongs to Full
2.4.7.4. Conversation context lifecycle defines conversation scope/context which should belong to Full
- Same goes for @SessionScoped - according to this doc, we wanted to have it optional (basically not part of Lite by default)
- If the above is true, then perhaps the whole section 2.4.6. Passivation and passivating scopes probably doesn't make much sense in Lite spec part?
[Optional] Adapt code snippets in Lite part to only use scopes we allow there; a lot of examples have session/conversation scopes

manovotn

Added some comments - I will send a PR against your branch with the suggestions I am making here (majority of them).
We also need a placeholder chapter for bean archives (packaging and deployment basically) in Lite which is TBD but we mustn't forget about it.

There are few more point that my PR won't address and that I wanted to bring attention to:

Specialization section should IMO be under Full according to what we agreed on
CDI Full has tons on very small chapters, most of which deal with limitations and deployment exceptions for decorators (and other single feature related topics). I was thinking if it wouldn't be more readable if we were to just create, for example, a decorator chapter and list all the limitations there?
Modularity in Lite currently speaks about explicit bean archives and enabling alternatives per archive. This is IMO wrong and should be mostly moved into Full. WDYT?
Lite needs a section about Packing and deployment. It will re-use part of the text from Full (such as how discovery works for implicit) but even with just

spec/src/main/asciidoc/core/scopescontexts_full.asciidoc

spec/src/main/asciidoc/preface.asciidoc

spec/src/main/asciidoc/core/injectionandresolution.asciidoc

spec/src/main/asciidoc/core/injectionandresolution_full.asciidoc

spec/src/main/asciidoc/preface.asciidoc

manovotn · 2021-04-27T10:46:13Z

@antoinesd I sent the above suggested changes as a PR to your repo - antoinesd#6

spec/src/main/asciidoc/preface.asciidoc

manovotn · 2021-04-27T14:26:28Z

We will also need to state that we don't support @Interceptors in CDI Lite (as previously discussed in various places).
Otherwise we would need to support it as we only inherit what interceptors spec says.

spec/src/main/asciidoc/core/scopescontexts.asciidoc

Signed-off-by: antoinesd <antoine@sabot-durand.net>

additional split Signed-off-by: antoinesd <antoine@sabot-durand.net>

…Modularity paragraph.

manovotn · 2021-04-29T10:43:21Z

Addressed other comments present here. Also rebased onto master which contained some doc changes causing clashes.

Moved specialization under Full while leaving inheritance rules in Lite.
Changed Modularity to not mention per-archive enablement of alternatives and also remove notion of explicit bean archives - both were moved to Full section.

If you find anything unclear or discover any typos, misplaces chapter etc, let me know and I'll fix that.
Meanwhile, I am trying to change the Full section in some way that would make it more readable (e.g. something like condense all decorator related bits into one chapter).

…ions into one chapter. Add multiple notions of Full - specific rules cak to original chapters and note that this behaviour is not in Lite.

manovotn · 2021-04-29T13:30:26Z

I've added a commit that tries to make the Full part a but more readable. Feedback is welcome including a different way to handle this altogether, this is just my first idea on how to improve it.

What I've done:

Put all decorator-related limitation into a paragraph under decorators
Take several other rules from Full and put then back in Lite but state explicitly that given behavior is related to Full

This drastically reduces the amount of paragraphs and sections needed under Full so that this section can instead fully focus on specifying what features it offers on top of Lite.

…upport inteception declaring with bindings.

manovotn · 2021-05-03T08:49:08Z

Added a commit splitting interceptor chapter into Lite and Full to separate things such as per-archive activation. I have also added a statement into Lite section that eliminates support of @Interceptors in Lite as we only wanted to support resolution based on interceptor bindings.

Ladicek · 2021-05-04T13:16:28Z

I did a comprehensive review of the entire spec and made quite a bit of changes, mostly related to the Lite/Full split. I pushed a commit with all those changes.

I have also liberally (yet carefully) added TODO comments all over the place, so that unfinished parts are easy to find.

Ladicek · 2021-05-04T13:17:31Z

For the record, I also maintain a set of HTML diffs of the spec here: https://ladicek.github.io/cdi-spec/ I'm going to add a diff for my changes later today -- generating the diff takes some time.

manovotn · 2021-05-04T13:21:44Z

spec/src/main/asciidoc/core/definition.asciidoc

 Note that to ensure compatibility with other Jakarta Dependency Injection implementations, all pseudo-scope annotations except `@Dependent` *are not* bean defining annotations.
 However, a stereotype annotation including a pseudo-scope annotation *is* a bean defining annotation.

+// TODO we might be able to make `@Singleton` a bean defining annotation in Lite? depending on how we decide on the beans.xml thing


I don't think you can do that. Because in Full it cannot be bean defining annotation and that would create inconsistency between Lite and Full.

Fair point.

manovotn · 2021-05-04T13:24:07Z

spec/src/main/asciidoc/core/definition.asciidoc


 A bean may declare zero, one or multiple stereotypes.

+// TODO how about we finally allowed declaring `@Priority` on stereotypes?


What is the use case? You actually want to avoid having (for instance) multiple interceptors that all have the same priority because then you don't know which one goes first. And in case of alternatives, it is probably even an error to have two with identical priority.

https://issues.redhat.com/browse/CDI-695 :-)

manovotn · 2021-05-04T13:30:24Z

spec/src/main/asciidoc/core/inheritance.asciidoc

 The second bean is able to serve the same role in the system as the first.
 In a particular deployment, only one of the two beans may fulfill that role.

+Specialization is only present in {cdi_full} and is specified therein.


Maybe add a link to the chapter?

Can do. I think back-links from Full to Lite are very important -- the other way, not so much.

manovotn · 2021-05-04T13:39:39Z

spec/src/main/asciidoc/core/injectionandresolution.asciidoc

 * `jakarta.enterprise.inject.Any`
 * `jakarta.enterprise.inject.Default`
 * `jakarta.enterprise.inject.Specializes`
+// TODO move to Full?


I noticed these as well. IMO if the user gets to consume the exact same CDI API jar, then there is little reason to extract this from spec as those literals are simply present, even though their respective annotations are ignored by Lite.

Or you could just separate these two and add them right below, stating that these are related to CDI Full?

manovotn · 2021-05-04T13:43:45Z

spec/src/main/asciidoc/core/injectionandresolution_full.asciidoc


-* the bean is not a decorator
+* the bean is not a decorator,
+* the bean is either not an alternative, or the module is a bean archive and the bean is a selected alternative of the bean archive.


+1 for overriden becuase with the last sentence you added, the rules become very cryptic.

I think a similarly cryptic sentence is present in the original, but I see what you mean :-)

Yeah,... I never said the original was better :D

Emily-Jiang · 2021-05-04T13:44:59Z

spec/src/main/asciidoc/architecture.asciidoc

@@ -7,10 +7,10 @@ This specification defines a powerful set of complementary services that help to
 * A sophisticated, typesafe _dependency injection_ mechanism, including the ability to select dependencies at either development or deployment time, without verbose configuration


I suggest to define cdi full and cdi liter concept first in this chapter and then refer to it.

Emily-Jiang · 2021-05-04T13:48:55Z

spec/src/main/asciidoc/core/beanmanager_lite.asciidoc

+
+== Programmatic access to container
+
+CDI 3.0 used to provide a single `BeanManager` object, which allows access to many useful operations.


This is not only true for CDI 3.0 though. I would say: The CDI version prior to 4.0

If you could suggest a way how to phrase this without the historical reference, that would be best. I admit I didn't give it much thought, I'll think about it again.

Prior to CDI 4.0, there used to be a single ....?

Emily-Jiang · 2021-05-04T13:49:33Z

spec/src/main/asciidoc/core/beanmanager_lite.asciidoc

+`BeanManagerLite` provides access to a subset of `BeanManager` features which can be implemented in more restricted environments;
+It is available in {cdi_lite} environments.
+
+`BeanManager` extends `BeanManagerLite` and provides the remaining features.


the remaining features -> more capabilities

Emily-Jiang · 2021-05-04T13:50:33Z

spec/src/main/asciidoc/core/beanmanager_lite.asciidoc

+It is available in {cdi_lite} environments.
+
+`BeanManager` extends `BeanManagerLite` and provides the remaining features.
+It is available in {cdi_full} environments.


{cdi_full} environments environments -> a {cdi_full} environment

The plural form here is correct because there are two CDI Full environments - SE and EE

I'll try to rephrase this paragraph today, there are too many comments so I clearly have to do a better job :-)

Emily-Jiang · 2021-05-04T13:54:16Z

spec/src/main/asciidoc/core/beanmanager_lite.asciidoc

+
+=== The `BeanManagerLite` object
+
+The interface `jakarta.enterprise.inject.spi.BeanManagerLite` provides operations for obtaining contextual references for beans, along with many other operations of use to applications.


Can BeanManagerLite be looked up via jndi lookup at runtime?

I believe we should avoid references to JNDI, as well as other Jakarta EE concepts, in Lite.

(In fact, I briefly thought we should move that JNDI thing to the EE section. I don't know why it's present in "Core CDI", it's clearly EE only and for example can't be used in SE.)

EDIT: I just realized you can have JNDI in SE. I'd still like to avoid references to JNDI in Lite.

I think I raised this question at some earlier comment on the BM Lite PR as well. Technically speaking you can get the BML via JNDI simply because you can get ordinary BM (which extends BML).

That being said, IMO we don't want any specific notion of Lite supporting JNDI in any way.

Ladicek · 2021-05-04T15:05:12Z

FTR, the HTML diff for the latest commit in this PR is now available at https://ladicek.github.io/cdi-spec/.

manovotn · 2021-05-05T10:04:32Z

As discussed, we want to avoid having Lite as part of CDI API names. So I've changed the current proposal of BeanManagerLite to BeanContainer (see #471) and I have also changed all references in this PR to keep it consistent.

Ladicek · 2021-05-05T11:36:27Z

Looking at the diff I published yesterday, I have noticed some typos -- some of then I fixed yesterday, some of them I fixed right now. I also tried to rephrase that one paragraph under "Programmatic access to container".

Latest diff is available at https://ladicek.github.io/cdi-spec/diff4.html

manovotn · 2021-05-05T12:38:59Z

spec/src/main/asciidoc/core/beanmanager_lite.asciidoc

 ----

 The operations of `BeanContainer` may be called at any time during the execution of the application.
+// TODO Full has restrictions on when BeanManager methods can be called, do we want to reflect them here in some way?


How about we instead change the The BeanManager object and mention that those rules apply to both, BM and BeanContainer?

That should be implied by

In {cdi_full} environment, BeanContainer is subject to the same rules as BeanManager.

What I'm more thinking about here is: if we're saying that in Lite, BeanContainer can be invoked at any time, that's a superset of what's allowed in Full. Which might be problematic at least from the "Lite is a strict subset of Full" perspective.

Or at least, it might look like a superset. That will depend on how we define the container lifecycle in Lite, which isn't there yet. (In Full, it's defined in the SPI section.)

BeanContainer can be invoked at any time

That's not what you wrote there though. You stated ... at any time during the execution of the application which means after the CDI container is bootstrapped, doesn't it? In other words, in Lite you cannot even try to use BeanContainer earlier.
And so long as you cannot use BeanContainer where you couldn't use BM (from within extensions), you should be good.

Or am I missing something?

Again, "at any time during the execution of the application" depends on how we define lifecycle, which we didn't do yet. If we define it in a way that all initialization phases are considered before application execution, then sure.

I added the TODO here mainly as a reminder, to revisit this section after we define lifecycle. I could have worded it more explicitly, for sure -- sorry about that.

Ladicek · 2021-05-20T14:50:28Z

I have summarized all the TODO comments that are currently present in this PR here: #482 I believe that this PR should be merged as is and all the issues summarized in #482 should be addressed separately.

antoinesd · 2021-05-20T15:18:15Z

I have summarized all the TODO comments that are currently present in this PR here: #482 I believe that this PR should be merged as is and all the issues summarized in #482 should be addressed separately.

It's ok for me. Waiting for @manovotn agreement

manovotn

Sure, fine by me!

antoinesd changed the title ~~DO NOT Merge cdi-468 Split Spec doc to introduce CDI Lite~~ **DO NOT MERGE** cdi-468 Split Spec doc to introduce CDI Lite Mar 18, 2021

Ladicek reviewed Mar 18, 2021

View reviewed changes

spec/src/main/asciidoc/preface.asciidoc Outdated Show resolved Hide resolved

antoinesd marked this pull request as draft April 12, 2021 14:04

manovotn requested changes Apr 27, 2021

View reviewed changes

otaviojava reviewed Apr 27, 2021

View reviewed changes

spec/src/main/asciidoc/preface.asciidoc Outdated Show resolved Hide resolved

otaviojava reviewed Apr 27, 2021

View reviewed changes

spec/src/main/asciidoc/preface.asciidoc Outdated Show resolved Hide resolved

Azquelt reviewed Apr 27, 2021

View reviewed changes

spec/src/main/asciidoc/core/scopescontexts.asciidoc Show resolved Hide resolved

manovotn mentioned this pull request Apr 28, 2021

Contributing suggested changes. antoinesd/cdi#6

Closed

antoinesd and others added 4 commits April 29, 2021 11:03

cdi-468 Split Spec doc to introduce CDI Lite

58177d9

Signed-off-by: antoinesd <antoine@sabot-durand.net>

cdi-468 Split Spec doc to introduce CDI Lite

0aec646

additional split Signed-off-by: antoinesd <antoine@sabot-durand.net>

Contributing suggested changes.

893141b

Incorporate additional changes - move specialization to Full; change …

a45fbe3

…Modularity paragraph.

manovotn force-pushed the cdi-468 branch from 3089bb8 to a45fbe3 Compare April 29, 2021 09:13

Attempt to make the Full part more readable - merge decorator limitat…

af8fc82

…ions into one chapter. Add multiple notions of Full - specific rules cak to original chapters and note that this behaviour is not in Lite.

Split interceptors chapter into Lite and Full, state that Lite only s…

2717c29

…upport inteception declaring with bindings.

comprehensive review and various Lite/Full split changes

ac06dcd

remove duplicate interceptors_full inclusion

e29216e

manovotn reviewed May 4, 2021

View reviewed changes

Emily-Jiang reviewed May 4, 2021

View reviewed changes

Ladicek and others added 2 commits May 4, 2021 17:49

actually include the beanmanager_lite.asciidoc file

b1638a2

Rename occurrences of BeanManagerLite to BeanContainer.

d8d2885

fix some typos and clumsy wording

864e1b4

manovotn reviewed May 5, 2021

View reviewed changes

Ladicek mentioned this pull request May 20, 2021

Lite/Full split: eliminate all TODOs from the specification #482

Closed

12 tasks

antoinesd marked this pull request as ready for review May 20, 2021 15:15

antoinesd requested a review from manovotn May 20, 2021 15:15

manovotn approved these changes May 20, 2021

View reviewed changes

manovotn changed the title **DO NOT MERGE** cdi-468 Split Spec doc to introduce CDI Lite cdi-468 Split Spec doc to introduce CDI Lite May 20, 2021

manovotn merged commit fec78b1 into jakartaee:master May 20, 2021

Ladicek added the Lite Related to CDI Lite label Oct 18, 2021


		A bean may declare zero, one or multiple stereotypes.

		// TODO how about we finally allowed declaring `@Priority` on stereotypes?

		@@ -7,10 +7,10 @@ This specification defines a powerful set of complementary services that help to
		* A sophisticated, typesafe _dependency injection_ mechanism, including the ability to select dependencies at either development or deployment time, without verbose configuration


		== Programmatic access to container

		CDI 3.0 used to provide a single `BeanManager` object, which allows access to many useful operations.


		=== The `BeanManagerLite` object

		The interface `jakarta.enterprise.inject.spi.BeanManagerLite` provides operations for obtaining contextual references for beans, along with many other operations of use to applications.

Conversation

antoinesd commented Mar 18, 2021

Uh oh!

antoinesd commented Mar 18, 2021

Uh oh!

Uh oh!

manovotn commented Mar 23, 2021

Uh oh!

manovotn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

manovotn commented Apr 27, 2021

Uh oh!

Uh oh!

Uh oh!

manovotn commented Apr 27, 2021

Uh oh!

Uh oh!

manovotn commented Apr 29, 2021

Uh oh!

manovotn commented Apr 29, 2021

Uh oh!

manovotn commented May 3, 2021

Uh oh!

Ladicek commented May 4, 2021

Uh oh!

Ladicek commented May 4, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manovotn May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

manovotn May 4, 2021 •

edited

Loading

Ladicek May 4, 2021 •

edited

Loading

Ladicek commented May 4, 2021 •

edited

Loading