Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Chetan Mehrotra]

Opened OAK-4640 to track this
Chetan Mehrotra


On Wed, Aug 3, 2016 at 9:36 PM, Michael Dürig  wrote:
>
>
> On 3.8.16 5:58 , Chetan Mehrotra wrote:
>>
>> On Wed, Aug 3, 2016 at 8:57 PM, Michael Dürig  wrote:
>>>
>>> I would suggest to add an new, internal mechanism to CommitInfo for your
>>> purpose.
>>
>>
>> So introduce a new CommitAttributes instance which would be returned
>> by CommitInfo ... ?
>
>
> Probably the best of all ugly solutions yes ;-) (Meaning I don't have a
> better idea neither...)
>
> Michael
>
>>
>> Chetan Mehrotra
>>
>

Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Michael Dürig]

On 3.8.16 5:58 , Chetan Mehrotra wrote:

On Wed, Aug 3, 2016 at 8:57 PM, Michael Dürig  wrote:

I would suggest to add an new, internal mechanism to CommitInfo for your
purpose.


So introduce a new CommitAttributes instance which would be returned
by CommitInfo ... ?


Probably the best of all ugly solutions yes ;-) (Meaning I don't have a 
better idea neither...)


Michael



Chetan Mehrotra

Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Chetan Mehrotra]

On Wed, Aug 3, 2016 at 8:57 PM, Michael Dürig  wrote:
> I would suggest to add an new, internal mechanism to CommitInfo for your
> purpose.

So introduce a new CommitAttributes instance which would be returned
by CommitInfo ... ?

Chetan Mehrotra

Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Michael Dürig]

So maybe we need some additional means to survey this context then. If 
you look at https://issues.apache.org/jira/browse/OAK-1438 where the 
commit info map was introduced and at the usages of the same, it is 
evident that this map belongs to the client (i.e. caller). Modifying it 
midway through is not expected. One of the more subtle "usages" is the 
info map being part of the CommitInfo's equals() and hashCode() 
implementations. It is very difficult to know what parts of the code 
rely on this and what impact a mutable map would have here.


I would suggest to add an new, internal mechanism to CommitInfo for your 
purpose.


Michael


On 3.8.16 5:01 , Chetan Mehrotra wrote:

That would depend on the CommitHook impl which client code would not
be aware of. And commit hook would also know only as commit traversal
is done. So it needs to be some mutable state
Chetan Mehrotra


On Wed, Aug 3, 2016 at 8:27 PM, Michael Dürig  wrote:


Couldn't we keep the map immutable and instead add some "WhateverCollector"
instances as values? E.g. add a AffectedNodeTypeCollector right from the
beginning?

Michael



On 3.8.16 4:06 , Chetan Mehrotra wrote:


So would it be ok to make the map within CommitInfo mutable ?
Chetan Mehrotra


On Wed, Aug 3, 2016 at 7:29 PM, Michael Dürig  wrote:





#A -Probably we can introduce a new type CommitAttributes which can be
attached to CommitInfo and which can be modified by the CommitHooks.
The CommitAttributes can then later be accessed by Observer




This is already present via the CommitInfo.info map. It is even used in a
similar way. See CommitInfo.getPath() and its usages. AFAIU the only part
where your cases would differ is that the information is assembled by
some
commit hooks instead of being provided at the point the commit was
initiated.


Michael

Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Chetan Mehrotra]

That would depend on the CommitHook impl which client code would not
be aware of. And commit hook would also know only as commit traversal
is done. So it needs to be some mutable state
Chetan Mehrotra


On Wed, Aug 3, 2016 at 8:27 PM, Michael Dürig  wrote:
>
> Couldn't we keep the map immutable and instead add some "WhateverCollector"
> instances as values? E.g. add a AffectedNodeTypeCollector right from the
> beginning?
>
> Michael
>
>
>
> On 3.8.16 4:06 , Chetan Mehrotra wrote:
>>
>> So would it be ok to make the map within CommitInfo mutable ?
>> Chetan Mehrotra
>>
>>
>> On Wed, Aug 3, 2016 at 7:29 PM, Michael Dürig  wrote:
>>>
>>>

 #A -Probably we can introduce a new type CommitAttributes which can be
 attached to CommitInfo and which can be modified by the CommitHooks.
 The CommitAttributes can then later be accessed by Observer
>>>
>>>
>>>
>>> This is already present via the CommitInfo.info map. It is even used in a
>>> similar way. See CommitInfo.getPath() and its usages. AFAIU the only part
>>> where your cases would differ is that the information is assembled by
>>> some
>>> commit hooks instead of being provided at the point the commit was
>>> initiated.
>>>
>>>
>>> Michael

Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Michael Dürig]

Couldn't we keep the map immutable and instead add some 
"WhateverCollector" instances as values? E.g. add a 
AffectedNodeTypeCollector right from the beginning?


Michael


On 3.8.16 4:06 , Chetan Mehrotra wrote:

So would it be ok to make the map within CommitInfo mutable ?
Chetan Mehrotra


On Wed, Aug 3, 2016 at 7:29 PM, Michael Dürig  wrote:




#A -Probably we can introduce a new type CommitAttributes which can be
attached to CommitInfo and which can be modified by the CommitHooks.
The CommitAttributes can then later be accessed by Observer



This is already present via the CommitInfo.info map. It is even used in a
similar way. See CommitInfo.getPath() and its usages. AFAIU the only part
where your cases would differ is that the information is assembled by some
commit hooks instead of being provided at the point the commit was
initiated.


Michael

Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Chetan Mehrotra]

So would it be ok to make the map within CommitInfo mutable ?
Chetan Mehrotra


On Wed, Aug 3, 2016 at 7:29 PM, Michael Dürig  wrote:
>
>>
>> #A -Probably we can introduce a new type CommitAttributes which can be
>> attached to CommitInfo and which can be modified by the CommitHooks.
>> The CommitAttributes can then later be accessed by Observer
>
>
> This is already present via the CommitInfo.info map. It is even used in a
> similar way. See CommitInfo.getPath() and its usages. AFAIU the only part
> where your cases would differ is that the information is assembled by some
> commit hooks instead of being provided at the point the commit was
> initiated.
>
>
> Michael

Re: Way to capture metadata related to commit as part of CommitInfo from within CommitHook

[Michael Dürig]

#A -Probably we can introduce a new type CommitAttributes which can be
attached to CommitInfo and which can be modified by the CommitHooks.
The CommitAttributes can then later be accessed by Observer


This is already present via the CommitInfo.info map. It is even used in 
a similar way. See CommitInfo.getPath() and its usages. AFAIU the only 
part where your cases would differ is that the information is assembled 
by some commit hooks instead of being provided at the point the commit 
was initiated.



Michael