JavaDoc for Event's subjectId methods

[Denis Garus]

Hello, Igniters!

Some events contain the subjectId method, for example, TaskEvent#subjectId.
The JavaDoc for this method is:
"Gets security subject ID initiated this task event, if available.
This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
task event.
Subject ID will be set either to node ID or client ID initiated task
execution."

I think It's wrong. The main point is a subject id doesn't have any sense
if IgniteSecurity is disabled.
However, if IgniteSecurity is enabled, the method must return the subject
id from the current security context.
Thus, the description (and behavior) of the method should be the following:
Gets security subject ID initiated this task event if IgniteSecurity is
enabled, otherwise returns null.

The same is actual for CacheEvent, CacheQueryExecutedEvent and
CacheQueryReadEvent.

If there are no objections, I am going to create a relevant issue in Jira.

[dmagda]

Denis, please feel free to go and edit the JavaDocs in place without a
ticket. The changes suggested by you are reasonable.

-
Denis


On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <[hidden email]> wrote:

> Hello, Igniters!
>
> Some events contain the subjectId method, for example, TaskEvent#subjectId.
> The JavaDoc for this method is:
> "Gets security subject ID initiated this task event, if available.
> This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
> task event.
> Subject ID will be set either to node ID or client ID initiated task
> execution."
>
> I think It's wrong. The main point is a subject id doesn't have any sense
> if IgniteSecurity is disabled.
> However, if IgniteSecurity is enabled, the method must return the subject
> id from the current security context.
> Thus, the description (and behavior) of the method should be the following:
> Gets security subject ID initiated this task event if IgniteSecurity is
> enabled, otherwise returns null.
>
> The same is actual for CacheEvent, CacheQueryExecutedEvent and
> CacheQueryReadEvent.
>
> If there are no objections, I am going to create a relevant issue in Jira.
>

[Ivan Pavlukhin]

Hi,

Do we allow commits to master without a ticket? I can imagine only
reverts as an exception.

Otherwise a ticket is a primary process item. Work description,
review, CI checks (we have a job checking javadocs).

ср, 25 сент. 2019 г. в 01:15, Denis Magda <[hidden email]>:

>
> Denis, please feel free to go and edit the JavaDocs in place without a
> ticket. The changes suggested by you are reasonable.
>
> -
> Denis
>
>
> On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <[hidden email]> wrote:
>
> > Hello, Igniters!
> >
> > Some events contain the subjectId method, for example, TaskEvent#subjectId.
> > The JavaDoc for this method is:
> > "Gets security subject ID initiated this task event, if available.
> > This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
> > task event.
> > Subject ID will be set either to node ID or client ID initiated task
> > execution."
> >
> > I think It's wrong. The main point is a subject id doesn't have any sense
> > if IgniteSecurity is disabled.
> > However, if IgniteSecurity is enabled, the method must return the subject
> > id from the current security context.
> > Thus, the description (and behavior) of the method should be the following:
> > Gets security subject ID initiated this task event if IgniteSecurity is
> > enabled, otherwise returns null.
> >
> > The same is actual for CacheEvent, CacheQueryExecutedEvent and
> > CacheQueryReadEvent.
> >
> > If there are no objections, I am going to create a relevant issue in Jira.
> >

--
Best regards,
Ivan Pavlukhin

[Denis Garus]

Hello!

I suggested to Maksim Stepachev include these changes in the scope of your
thicket [1]
and it looks like he agreed [2].

Maksim Stepachev, could you please reflect JavaDoc and behavior changes of
events in your ticket?

1. https://issues.apache.org/jira/browse/IGNITE-11992
2.
http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html

пн, 30 сент. 2019 г. в 11:07, Ivan Pavlukhin <[hidden email]>:

> Hi,
>
> Do we allow commits to master without a ticket? I can imagine only
> reverts as an exception.
>
> Otherwise a ticket is a primary process item. Work description,
> review, CI checks (we have a job checking javadocs).
>
> ср, 25 сент. 2019 г. в 01:15, Denis Magda <[hidden email]>:
> >
> > Denis, please feel free to go and edit the JavaDocs in place without a
> > ticket. The changes suggested by you are reasonable.
> >
> > -
> > Denis
> >
> >
> > On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <[hidden email]> wrote:
> >
> > > Hello, Igniters!
> > >
> > > Some events contain the subjectId method, for example,
> TaskEvent#subjectId.
> > > The JavaDoc for this method is:
> > > "Gets security subject ID initiated this task event, if available.
> > > This property is not available for
> GridEventType#EVT_TASK_SESSION_ATTR_SET
> > > task event.
> > > Subject ID will be set either to node ID or client ID initiated task
> > > execution."
> > >
> > > I think It's wrong. The main point is a subject id doesn't have any
> sense
> > > if IgniteSecurity is disabled.
> > > However, if IgniteSecurity is enabled, the method must return the
> subject
> > > id from the current security context.
> > > Thus, the description (and behavior) of the method should be the
> following:
> > > Gets security subject ID initiated this task event if IgniteSecurity is
> > > enabled, otherwise returns null.
> > >
> > > The same is actual for CacheEvent, CacheQueryExecutedEvent and
> > > CacheQueryReadEvent.
> > >
> > > If there are no objections, I am going to create a relevant issue in
> Jira.
> > >
>
>
>
> --
> Best regards,
> Ivan Pavlukhin
>

[Maksim Stepachev]

Denis,

I added it in my ticket and pull-request. Should I change the only first
sentence or full comment?

пн, 30 сент. 2019 г. в 11:27, Denis Garus <[hidden email]>:

> Hello!
>
> I suggested to Maksim Stepachev include these changes in the scope of your
> thicket [1]
> and it looks like he agreed [2].
>
> Maksim Stepachev, could you please reflect JavaDoc and behavior changes of
> events in your ticket?
>
> 1. https://issues.apache.org/jira/browse/IGNITE-11992
> 2.
> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html
>
> пн, 30 сент. 2019 г. в 11:07, Ivan Pavlukhin <[hidden email]>:
>
>> Hi,
>>
>> Do we allow commits to master without a ticket? I can imagine only
>> reverts as an exception.
>>
>> Otherwise a ticket is a primary process item. Work description,
>> review, CI checks (we have a job checking javadocs).
>>
>> ср, 25 сент. 2019 г. в 01:15, Denis Magda <[hidden email]>:
>> >
>> > Denis, please feel free to go and edit the JavaDocs in place without a
>> > ticket. The changes suggested by you are reasonable.
>> >
>> > -
>> > Denis
>> >
>> >
>> > On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <[hidden email]>
>> wrote:
>> >
>> > > Hello, Igniters!
>> > >
>> > > Some events contain the subjectId method, for example,
>> TaskEvent#subjectId.
>> > > The JavaDoc for this method is:
>> > > "Gets security subject ID initiated this task event, if available.
>> > > This property is not available for
>> GridEventType#EVT_TASK_SESSION_ATTR_SET
>> > > task event.
>> > > Subject ID will be set either to node ID or client ID initiated task
>> > > execution."
>> > >
>> > > I think It's wrong. The main point is a subject id doesn't have any
>> sense
>> > > if IgniteSecurity is disabled.
>> > > However, if IgniteSecurity is enabled, the method must return the
>> subject
>> > > id from the current security context.
>> > > Thus, the description (and behavior) of the method should be the
>> following:
>> > > Gets security subject ID initiated this task event if IgniteSecurity
>> is
>> > > enabled, otherwise returns null.
>> > >
>> > > The same is actual for CacheEvent, CacheQueryExecutedEvent and
>> > > CacheQueryReadEvent.
>> > >
>> > > If there are no objections, I am going to create a relevant issue in
>> Jira.
>> > >
>>
>>
>>
>> --
>> Best regards,
>> Ivan Pavlukhin
>>
>

[Denis Garus]

Thank you, Maksim!

I think that the description should look like:
"Gets security subject ID initiated this task event if IgniteSecurity is
enabled, otherwise returns null.
This property is not available for GridEventType#EVT_TASK_SESSION_ATTR_SET
task event."

Please pay close attention that described behavior is confirmed by tests.

пн, 30 сент. 2019 г. в 13:11, Maksim Stepachev <[hidden email]>:

> Denis,
>
> I added it in my ticket and pull-request. Should I change the only first
> sentence or full comment?
>
> пн, 30 сент. 2019 г. в 11:27, Denis Garus <[hidden email]>:
>
>> Hello!
>>
>> I suggested to Maksim Stepachev include these changes in the scope of
>> your thicket [1]
>> and it looks like he agreed [2].
>>
>> Maksim Stepachev, could you please reflect JavaDoc and behavior changes
>> of events in your ticket?
>>
>> 1. https://issues.apache.org/jira/browse/IGNITE-11992
>> 2.
>> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html
>>
>> пн, 30 сент. 2019 г. в 11:07, Ivan Pavlukhin <[hidden email]>:
>>
>>> Hi,
>>>
>>> Do we allow commits to master without a ticket? I can imagine only
>>> reverts as an exception.
>>>
>>> Otherwise a ticket is a primary process item. Work description,
>>> review, CI checks (we have a job checking javadocs).
>>>
>>> ср, 25 сент. 2019 г. в 01:15, Denis Magda <[hidden email]>:
>>> >
>>> > Denis, please feel free to go and edit the JavaDocs in place without a
>>> > ticket. The changes suggested by you are reasonable.
>>> >
>>> > -
>>> > Denis
>>> >
>>> >
>>> > On Tue, Sep 24, 2019 at 3:55 AM Denis Garus <[hidden email]>
>>> wrote:
>>> >
>>> > > Hello, Igniters!
>>> > >
>>> > > Some events contain the subjectId method, for example,
>>> TaskEvent#subjectId.
>>> > > The JavaDoc for this method is:
>>> > > "Gets security subject ID initiated this task event, if available.
>>> > > This property is not available for
>>> GridEventType#EVT_TASK_SESSION_ATTR_SET
>>> > > task event.
>>> > > Subject ID will be set either to node ID or client ID initiated task
>>> > > execution."
>>> > >
>>> > > I think It's wrong. The main point is a subject id doesn't have any
>>> sense
>>> > > if IgniteSecurity is disabled.
>>> > > However, if IgniteSecurity is enabled, the method must return the
>>> subject
>>> > > id from the current security context.
>>> > > Thus, the description (and behavior) of the method should be the
>>> following:
>>> > > Gets security subject ID initiated this task event if IgniteSecurity
>>> is
>>> > > enabled, otherwise returns null.
>>> > >
>>> > > The same is actual for CacheEvent, CacheQueryExecutedEvent and
>>> > > CacheQueryReadEvent.
>>> > >
>>> > > If there are no objections, I am going to create a relevant issue in
>>> Jira.
>>> > >
>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Ivan Pavlukhin
>>>
>>