Thanks for the detailed review of the javadoc!
About the name for createContextObject methods:
The specs refers to such proxy with contextual information as
"contextual object" or "contextual object proxy" in various places in
the spec and javadoc. While "createContextualizedProxy" is an accurate
description of the object that the method returns, it is not consistent
with how that object is referred to elsewhere. We could definitely
change the method name and update the spec to use the term
"contextualized proxy" if you think that it is the right thing to do. Or
we could rename the methods to "createContextualObject" to be more
consistent with the spec. What do you think?
About the rationale for putting USE_PARENT_TRANSACTION in ContextService:
I thought about this when we added execution properties to ManagedTask
earlier. I decided to leave USE_PARENT_TRANSACTION in ContextService
because this execution property does not make sense when a task is
submitted to run asynchronously on a different thread than the thread
that submitted it, which is what ManagedTask are. So I did not move it
to ManagedTask.
Should the schedule methods in ManagedScheduledExecutorService throws
IllegalArgumentException instead of an NPE if the parameter is null?
They were following the behavior of other schedule(...) methods in the
parent class ScheduledExecutorService in java.util.concurrent, where NPE
is thrown if the Callable or Runnable passed is null.
6. The description of the method:
<T> T createContextObject(T instance, Class<T> intf)
states:
"The contextual object is useful when developing or using Java SE
threading mechanisms spraying events to other component instances or
communicating with component instances on different Java processes."
I don't think it is clear what "spraying" means in this context.
How about replacing "spraying" with "propagating"?
Will make the other changes as suggested. Thanks again!
Regards
Anthony
On 12/22/12 4:31 AM, Frederick W Rowe wrote:
> One correction to my suggested correction for AbortedException, instead
> of:
> "Exception indicating that the result of a task cannot be retrieved
> because the task failed to start for some reason other than being
> cancelled."
> I'd propose:
> "Exception indicating that the result of a task cannot be retrieved
> because the task failed to run for some reason other than being
> cancelled."
>
> Regards,
>
> Fred Rowe
>
> WebSphere Architect
> Senior Software Engineer
> IBM Software Group
>
>
>
> Frederick W Rowe/Raleigh/IBM_at_IBMUS
> 12/21/2012 10:25 PM
> Please respond to
> jsr236-experts
>
>
> To
> jsr236-experts_at_concurrency-ee-spec.java.net
> cc
>
> Subject
> [jsr236-experts] Re: Public Review Draft candidate uploaded
>
>
>
>
>
>
> Since the javadocs are typically the first place a java developer looks
> for guidance on using a new class or interface, getting the details of
> them correct are important particularly for new technology. With that in
> mind, here's some corrections/suggestions for the current javadocs, some
> of which are very minor. One general question first, for EE javadocs, is
> it common practice to use the @since annotation? If so, we should
> consider including them.
> Now, some specific comments...
>
> AbortedException
> The description text of this exception is not clear:
> "Exception indicating that the result of a value-producing task cannot be
> retrieved because the task run was aborted."
> I'd propose:
> "Exception indicating that the result of a task cannot be retrieved
> because the task failed to start for some reason other than being
> cancelled."
>
> ContextService:
> One general comment/question, is "createContextObject" a good method name?
>
> The method doesn't create a context Object, it creates a contextualized
> Proxy. Would createContextualizedProxy(...) be more accurate?
> 1. The current description says:
> "The ContextService provides methods for creating contextual dynamic proxy
>
> objects"
> with contextual dynamic proxy in italics. I think that term contextual
> needs further explanation or definition. I'd propose:
> "The ContextService provides methods for creating dynamic proxy objects
> (as defined by java.lang.reflect.Proxy) with the the addition of context
> typically associated with applications executing in a JEE environment.
> Examples of such context are classloading, namespace, security, etc."
>
> 2. References to other java classes and interfaces (like
> java.lang.reflect.Proxy, java.io.Serializable and java.lang.Object) in the
>
> description are not appearing as hyperlinks in the javadoc, but should be.
>
> 3. Change bullet
> "The proxy instance will retain the context of the application container's
>
> thread (creator's context). "
> to:
> "The proxy instance will retain the context of the creator's thread."
>
> 4. Correct typo in bullet:
> "The object to have a proxy instance created for should not be a component
>
> managed by the Java EE Produce Provider, such as web component or an EJB."
> to:
> "The object to have a proxy instance created for should not be a component
>
> managed by the Java EE Product Provider, such as web component or an EJB."
>
> 5. Change description of field USE_PARENT_TRANSACTION from:
> static String USE_PARENT_TRANSACTION An execution object property that
> disables the normal transaction suspension and UserTransaction access from
>
> the proxied methods.
> to:
> static String USE_PARENT_TRANSACTION An execution property that disables
> the normal transaction suspension and UserTransaction access from the
> proxied methods.
>
> What was our rationale for putting this property on ContextService instead
>
> of ManagedTask like the other execution properties?
> 6. The description of the method:
> <T> T createContextObject(T instance, Class<T> intf)
> states:
> "The contextual object is useful when developing or using Java SE
> threading mechanisms spraying events to other component instances or
> communicating with component instances on different Java processes."
> I don't think it is clear what "spraying" means in this context.
>
> 7. Consider these changes to the example of the method <T> T
> createContextObject(T instance, Class<T> intf).
> Change:
> "For example, to call a normal Runnable with the correct context using a
> Java™ ExecutorService: "
> to:
> "For example, to execute a Runnable which is contextualized with the
> creator's context using a Java™ SE ExecutorService: "
> Change:
> "System.out.println("MyRunnable.run with J2EE Context available.");"
> to:
> "System.out.println("MyRunnable.run with JEE Context available.");"
>
> 8. Correct throws statement from:
> "IllegalArgumentException - - if the instance does not implement the
> interface or there is not an accessible default constructor."
> to:
> "IllegalArgumentException - - if the instance does not implement the
> specified interface or there is not an accessible default constructor."
>
> LastExecution:
> There are a number of method descriptions which read similar to:
> "The time in which the last task was scheduled to run"
> I believe we mean to say:
> "The last time that the task was scheduled to run"
> The problem is that last is used to modify "task" when it should be used
> to modify "time" or "date/time" in some of the descriptions.
>
> ManageableThread:
> The isShutdown method javadoc reads:
>
> boolean isShutdown()
> This method is used by the application component provider to check whether
>
> a thread created by the newThread method of ManagedThreadFactory has been
> shut down. If the value is true, the application component provider should
>
> finish any work on this thread as soon as possible.
> Returns:
> true if the ManagedThreadFactory instance that created this thread has
> been shut down.
>
> The method description and the Returns description do not agree. The
> former says the return val is determined by the state of the thread
> instance, the latter says the return val is determined by the state of the
>
> MaangedThreadFactory instance.
>
> ManagedExecutors:
>
> 1. The description of the method managedTask states:
> "Returns a Runnable object that also implements ManagedTask interface so
> it can receive notification of lifecycle events with the provided the
> ManagedTaskListener..."
> should be corrected to read:
> "Returns a Runnable object that also implements ManagedTask interface so
> it can receive notification of lifecycle events with the provided
> ManagedTaskListener..."
>
> 2. Should the managedTask methods throw IllegalArgumentException instead
> of an NPE if the task is null?
>
> 3. The taskListener parameter of the managedTask methods is marked
> optional in some method descriptions, I believe it is optional in all
> cases? The javadoc states "If task already implements ManagedTask, its
> associated ManagedTaskListener will be ignored." Is that true if
> taskListener is null? I think the intent is " If task implements
> ManagedTask, and taskListener is not null, the ManagedTaskListener
> interface of the task will not be called."
>
> 4. The executionProperties parameter of the managedTask methods states:
> "executionProperties - (optional) execution properties to provide
> additional hints to ManagedExecutorService or
> ManagedScheduledExecutorService when the task is submitted. If task
> already implements ManagedTask with non-empty execution properties, the
> Runnable returned will contain execution properties in both the task and
> in the executionProperties argument, with the latter taking precedence if
> the same property key is specified in both."
> it should be clarified:
> "executionProperties - (optional) execution properties to provide
> additional hints to ManagedExecutorService or
> ManagedScheduledExecutorService when the task is submitted. If task
> implements ManagedTask with non-empty execution properties, the Runnable
> returned will contain the union of the execution properties specified in
> the task and the executionProperties argument, with the latter taking
> precedence if the same property key is specified in both."
>
> ManagedExecutorService
>
> 1. Consider replacing the first paragraphs of the interface description:
> "A manageable version of a ExecutorService.
> A ManagedExecutorService provides methods for submitting tasks for
> execution in a managed environment."
> with:
> "A ManagedExecutorService extends the Java SE ExecutorService to provide
> methods for submitting tasks for execution in a Java EE environment."
>
> ManagedScheduledExecutorService
>
> 1. See comment above regarding first paragraphs of description,
> additionally correct the following typo in the description:
> "For example, if a task is repeat, the lifecycle of the task would be:"
> should be:
> "For example, if a task is repeating, the lifecycle of the task would be:"
>
> 2. Should the schedule methods throw IllegalArgumentException instead of
> an NPE if the parameter is null?
>
> ManagedTask
>
> 1. Some of the constants are described as "hints", but only one
> ("LONGRUNNING_HINT") has hint in the name, should we be more consistent?
>
> 2. Correct description of DISTRIBUTABLE hint:
> "Execution property to be returned in getExecutionProperties() or
> ContextService.createContextObject() to provide hint about where the task
> can be run in a different process that the one where the task is
> submitted. Valid values are "true" or "false". "
> to say:
> "Execution property to be returned in getExecutionProperties() or
> ContextService.createContextObject() to provide a hint as to whether the
> task can be run in a different process that the one where the task is
> submitted. Valid values are "true" or "false". "
>
> 3. Correct description of CONTEXTUAL_CALLBACK hint:
> "Execution property to be returned in getExecutionProperties() or
> ContextService.createContextObject() to provide hint about whether the
> ManagedTaskListener provided to receive notification of lifecycle events
> on this task needs to be called under the same context as the task."
> to say:
> "Execution property to be returned in getExecutionProperties() or
> ContextService.createContextObject() to provide a hint about whether the
> ManagedTaskListener associated with this task needs to be called under the
>
> same context as the task. Valid values are "true" or "false"."
>
> 4. Correct description of IDENTITY_NAME hint:
> "Implementations should not depend upon any thread execution context and
> should typically return only readily-available instance data. to identify
> the task."
> to say:
> "Implementations should not depend upon any thread execution context and
> should typically return only readily-available instance data to identify
> the task."
>
> ManagedTaskListener:
> 1. Correct the following typo:
> "The following state transition figure and tables describe the possible
> task lifecycle events that can occur when a ManagedTaskListener is
> associated with a task. Each method is invoked when the state of the
> future moves from one state to another."
> to state:
> "The following state transition figure and tables describe the possible
> task lifecycle events that can occur when a ManagedTaskListener is
> associated with a task. Each method is invoked when the state of the
> Future moves from one state to another."
>
> ManagedThreadFactory:
> 1. See comments for ManagedScheduledExecutorService and
> ManagedExecutorService regarding first paragraphs.
>
> SkippedException:
> 1. I believe the description incorrectly states:
> "A task can be skipped if the
> Trigger.skipRun(javax.enterprise.concurrent.LastExecution, java.util.Date)
>
> method returns false..."
> I believe it should state:
> "A task may be skipped if the
> Trigger.skipRun(javax.enterprise.concurrent.LastExecution, java.util.Date)
>
> method returns true..."
>
>
>
> Regards,
>
> Fred Rowe
>
> WebSphere Architect
> Senior Software Engineer
> IBM Software Group
>
>
>
>
> Anthony Lai <anthony.lai_at_oracle.com>
> 12/20/2012 06:36 PM
> Please respond to
> jsr236-experts
>
>
> To
> jsr236-experts_at_concurrency-ee-spec.java.net
> cc
>
> Subject
> [jsr236-experts] Public Review Draft candidate uploaded
>
>
>
>
>
>
> Dear experts,
>
> I have incorporated the feedback we have collected since the Early Draft
> and have posted a candidate for Public Review Draft to the project
> website.
>
> The document can be found at:
> http://java.net/projects/concurrency-ee-spec/downloads/download/EE%20Concurrency%20Utilities-Dec-20-2012.pdf
>
>
>
> A version with changebars showing changes from the previous revision can
> be found at:
> http://java.net/projects/concurrency-ee-spec/downloads/download/EE%20Concurrency%20Utilities%20Dec-20-2012-delta.pdf
>
>
>
> Updated javadoc archive can be found at:
> http://java.net/projects/concurrency-ee-spec/downloads/download/javax.enterprise.concurrent-api-1.0-SNAPSHOT-javadoc.jar
>
>
>
> Regarding Javadoc changes, I have made the following changes in addition
> to what we have discussed recently in the experts mailing list:
> - Renamed getProperties() to getExecutionProperties() in ContextService
> - Added a new requirement for proxy object in ContextService: The object
> to have a proxy instance created for should not be a component managed by
> the Java EE Produce Provider, such as web component or an EJB.
>
> Browseable javadoc at http://concurrency-ee-spec.java.net/javadoc/ are
> also updated with the same changes.
>
> Please send any feedback to the
> jsr236-experts_at_concurrency-ee-spec.java.net mailing list. Readers not on
> the expert group can send feedback to users_at_concurrency-ee-spec.java.net
> mailing list. We are hoping to submit Public Review Draft to the JCP on
> Dec 27 to start the Public Review process in time to meet the Java EE 7
> release schedule.
>
> Regards
> Anthony
>
>
>