Task #1291
closed![Author: Alena Peterová](/plugin_assets/redmine_people/images/person.png?1637230883)
![Assignee: Ondřej Kopr](/plugin_assets/redmine_people/images/person.png?1637230883)
Recalculate contract slices after synchronization of slices, before HR processes
100%
Description
Version 9.2.0-SNAPSHOT
After synchronization of contract slices ends, it should first recalculate the slices into contracts and after that start HR processes (otherwise HR processes run with old data). Please add option to run SelectCurrentContractSliceTaskExecutor after end of slice synchronization.
Real-world example:
One employee had contract to 30.9. The contract was prolonged to the next month. This information appeared in HR at last minute - on 30.9. So a new slice was created during the night 30.9.-1.10. The HR processes run after synchronization of slices, but before slices recalculation. So the user was disabled on 1.10., where he should be working. He was enabled again on 2.10., but that is too late.
Files
Updated by Vít Švanda almost 6 years ago
- Assignee changed from Vít Švanda to Ondřej Kopr
- Target version set to Onyx (9.3.0)
Updated by Ondřej Kopr almost 6 years ago
- Status changed from New to In Progress
- Target version changed from Onyx (9.3.0) to Opal (9.4.0-rc.1)
This will be implemented as new option for contract slice synchronization, but implementing with test is little bit complicated so I moved it on next version.
Updated by Ondřej Kopr almost 6 years ago
- % Done changed from 0 to 20
After consult I decided that I'm not implement new checkbox for contract slice synchronization, because contract slice synchronization use same settings as contract synchronization (to much reimplementing and it will be non backward compatible).
So when is checked run HR process after correctly ended synchronization. The task select current contract will be started. @Alca are you OK with the solution?
Updated by Alena Peterová almost 6 years ago
Just to be sure:
- slices recalculation will be started only if it is synchronization of contract slices, not for synchronization of contracts?
- if I have some project specific implementation (my own ContractSliceManager), it will be used for this recalculation started from the sync?
If this solution is simple, could you maybe put it into 9.2.3? :-) It would save us some time when scheduling tasks on our project.
Updated by Ondřej Kopr almost 6 years ago
Alena Peterová wrote:
- slices recalculation will be started only if it is synchronization of contract slices, not for synchronization of contracts?
Of course, task SelectCurrentContractExecutor is started only in contract slice synchronization.
Alena Peterová wrote:
- if I have some project specific implementation (my own ContractSliceManager), it will be used for this recalculation started from the sync?
Your implementation of ContractSliceManager will be works as it is. Implementation is done only in ContractSliceSynchronizationExecutor (contract slice synchronization).
Thank you for answer. I will start with implementation.
Updated by Ondřej Kopr almost 6 years ago
- % Done changed from 20 to 80
I implement executing select current contract slice after synchronization of contract slices correctly ended.
commit: https://github.com/bcvsolutions/CzechIdMng/commit/2458f9e871bd9349e4247f8a76caf2906a86b062 (branch: okopr/1291-select-current-contract)
Branch will be merged after release version 9.3.0
Tests is included.
Documentation will be also updated after will be released new version of documentation.
Updated by Ondřej Kopr almost 6 years ago
Branch was merged into develop and checked tests with current develop. Still missing documentation.
Updated by Ondřej Kopr almost 6 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Ondřej Kopr to Vít Švanda
- % Done changed from 80 to 90
Documentation was added: https://wiki.czechidm.com/devel/documentation/synchronization/dev/contract-slice-sync#select_current_contract_slice
Please Vitek could you make a review? Thank you :)
Updated by Alena Peterová over 5 years ago
!! Problem alert!!
I found out in the version 9.3.1 that the recalculation of slices is in fact done during the synchronization of slices after every slice. That could lead to removing manually assigned roles, deleting and recreating accounts. Consider this situation:- Slice 1 - slice is valid from 1.1.2018, the contract is valid from 1.1.2018 to infinity
- HR changes on 3.12.2018:
- slice 1 - valid till of the contract is changed to 30.11.2018
- slice 2 is created, the slice is valid from 1.12.2018, the contract is valid from 1.12.2018 to infinity
- Synchronization of slices will first update the slice 1 and so update the parent contract as invalid = valid till 30.11.2018. All roles and accounts are removed.
- The same synchronization will create the slice 2 and so update the parent contract as valid. Only automatic roles are assigned again and their accounts are recreated.
Such changes are nothing strange in HRs - e.g. if the type of contract is changed, the HR may create a new slice in this way.
Synchronization of slices must skip recalculation of slices and do it only at the end (if the checkbox is checked).
Sorry about the misleading original description of this ticket. We thought the reason was missing recalculation, but the reason was probably deleting the original slice from HR which wasn't processed by synchronization.
Updated by Alena Peterová over 5 years ago
Adding to the previous note:
I tried "workaround" - to disable contract-slice-save-recalculate-processor during synchronization. But then SelectCurrentContractSliceTaskExecutor run endlessly, probably due to invalid state of slices.
If fix of this problem (= recalculation of slices during synchronization) is complicated (or not available for 9.3.1), would you please try to think of some safe workaround for us?
Updated by Vít Švanda over 5 years ago
- Due date set to 12/28/2018
- Status changed from Needs feedback to In Progress
- Assignee changed from Vít Švanda to Ondřej Kopr
- % Done changed from 90 to 50
Conclusion (goal of this ticket changed):
We need:
- Skip slice recalculation during sync (not implemented now). IdmContractSliceService.SKIP_RECALCULATE_CONTRACT_SLICE should be help, but this skip was not designed directly for this purpose, so beware on conseqences :-) (SelectCurrentContractSliceTaskExecutor as Alca descripted).
- Start recalculation of all slices (implemented in this task). Recalculation of silces is controlled by checbox for recalculation of HR processes (please extends the label for this information ... I know, same label will be see in contract sync and add big warning to changelog ... ).
We don't need this feature in version 9.4.0.rc.1, so please remove this from develop (sorry).
We need to release this feature as 9.3.2 and after that as 9.4.0.
Updated by Vít Švanda over 5 years ago
- Priority changed from Normal to High
- Target version changed from Opal (9.4.0-rc.1) to Opal (9.4.0)
Updated by Radek Tomiška over 5 years ago
- Target version changed from Opal (9.4.0) to Onyx (9.3.2)
Updated by Ondřej Kopr over 5 years ago
I removed the commit from current develop and cherry pick into 9.3.2
commit in 9.3.2: https://github.com/bcvsolutions/CzechIdMng/commit/fb1e26884571e4c056fe674157262b868b531aab
revert changes: https://github.com/bcvsolutions/CzechIdMng/commit/f33ae96e8027b00b9eaeeb15e31214d4d432816c
Updated by Ondřej Kopr over 5 years ago
- Target version deleted (
Onyx (9.3.2))
The ticket will be solved in another hotfix (eq 9.3.3 or 9.4.0 final version).
Please move all changes in the ticket into some new branch.
Updated by Alena Peterová over 5 years ago
Ondřej Kopr wrote:
The ticket will be solved in another hotfix (eq 9.3.3 or 9.4.0 final version).
Please not 9.4, we need this as hotifx for 9.3.x and we can't update to 9.4 yet.
Updated by Ondřej Kopr over 5 years ago
- Target version set to Onyx (9.3.3)
Alena Peterová wrote:
Ondřej Kopr wrote:
The ticket will be solved in another hotfix (eq 9.3.3 or 9.4.0 final version).
Please not 9.4, we need this as hotifx for 9.3.x and we can't update to 9.4 yet.
Not problem, I create next hotfix 9.3.3. Thank you.
Updated by Ondřej Kopr over 5 years ago
- Target version changed from Onyx (9.3.3) to Onyx (9.3.2)
Solution was removed from 9.3.2: https://github.com/bcvsolutions/CzechIdMng/commit/7ff79878b946ea35715c2d43f3363a0e71694e1d
Commit was moved into branch: https://github.com/bcvsolutions/CzechIdMng/commits/hotfix-9.3.3
Updated by Ondřej Kopr over 5 years ago
- Target version changed from Onyx (9.3.2) to Onyx (9.3.3)
Updated by Ondřej Kopr over 5 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Ondřej Kopr to Alena Peterová
Today we tried simulate same problem on current develop:
Test scenario:- Synchronization 1:
- creates one new slice[ID:1] with validFrom 1.1. 2019 and validTill: infity
- after synchronization was added automatic roles by structure.
- We manually added two roles to newly created contract
- Synchronization 2:
- updates slice[ID:1] validTill: 3.1. 2019
- creates new slice slice[ID:2 ] with validFrom 1.1. 2019 and validTill: infinity
- after synchronization user still has all roles. In audit exits only infomation about validity change.
The problem descibes in ticket description:
One employee had contract to 30.9. The contract was prolonged to the next month. This information appeared in HR at last minute - on 30.9. So a new slice was created during the night 30.9.-1.10. The HR processes run after synchronization of slices, but before slices recalculation. So the user was disabled on 1.10., where he should be working. He was enabled again on 2.10., but that is too late.
Can't be simulated, because now doesn't exists skip for slice recalculation in synchronization. So for every slices is done recalculation during synchronization immediate (in future we probably need some skip :)). Task SelectCurrentContractSliceTaskExecutor exits only for recalculate existing slices, that expire during time, not from synchronization.
Question is, if we really need some changes for hotfix 9.3.3? Please could you test your scenarios?
Updated by Alena Peterová over 5 years ago
- File provisioning_archive.png provisioning_archive.png added
- File entity_event_IdmIdentityRole.png entity_event_IdmIdentityRole.png added
- File audit_IdmIdentityRole_Modify.png audit_IdmIdentityRole_Modify.png added
- Status changed from Needs feedback to In Progress
- Assignee changed from Alena Peterová to Ondřej Kopr
OK, you're right that the roles aren't removed, only their validity is temporarily changed. But it's enough to delete and recreate the accounts :-(
I tried to debug it in my project and I think this happens:- IdmIdentityRole is updated - validTill is set to the past (see audit_IdmIdentityRole_Modify.png)
- it causes IdmIdentityRole NOTIFY event, which runs identity-role-save-provisioning-processor (see entity_event_IdmIdentityRole.png)
- the processor starts account management just when all roles are invalid:
2019-01-08 16:41:24.224 INFO 20773 --- [task-executor-2] e.b.i.c.a.e.AbstractEntityEventProcessor : Processor [identity-role-save-provisioning-processor]([acc]) start for [CoreEvent [type : eu.bcvsolutions.idm.core.model.service.impl.DefaultEntityEventManager$$Lambda$111/42994890@144e55ff, content: eu.bcvsolutions.idm.core.api.dto.IdmIdentityRoleDto [id= a9df673d-8f87-4abd- 8efe-4510a259e916], properties: {idm:parent-event-id=null, idm:super-owner-id=46ed6b7e-efc1-4e88-abcb-4dbfdeb2156a, idm:parent-event-type=UPDATE, idm:root-event-id=null, idm:event-id=3fd8a d24-afcb-4ddf-abfd-db9b67ebd311, idm:priority=NORMAL, skipCheckAuthorities=true, idm:execute-date=null}]] with order [1000]. ...... 2019-01-08 16:41:24.734 INFO 20773 --- [task-executor-2] e.b.i.c.m.s.i.DefaultEntityEventManager : Publishing event [IdentityAccountEvent [type: DELETE, content: eu.bcvsolutions.idm.acc.dto.AccIdentityAccountDto [id= c46f5e87-0b09-43e7-9070-7a7300ef911c], properties: {deleteTargetAccount=true, forceDeleteOfIdentityAccount=false}]]
- ACM deletes the AccIdentityAccount, which deletes AccAccount, which causes provisioning Delete (see provisioning_archive.png - 3 accounts are deleted and later recreated, other 3 fall to account protection because the systems have it enabled).
Updated by Ondřej Kopr over 5 years ago
- File 009.png 009.png added
- File 008.png 008.png added
- File 007.png 007.png added
- File 006.png 006.png added
- File 005.png 005.png added
- File 004.png 004.png added
- File 003.png 003.png added
- File 002.png 002.png added
- File 001.png 001.png added
- Status changed from In Progress to Needs feedback
- Assignee changed from Ondřej Kopr to Alena Peterová
Thank you for your test scenario, I started test it again and there is screenshots from my tests:
- STEP 1: I synchronized contract slice [id:1], after synchronization user recive automatic roles by treeStructure and I added one manually role:
- STEP 2: In end system I change validity for contract slice [id: 1] to past and then I create second one slice that is valid [id: 2]
- STEP 3: I start with new synchronization, during synchronization is actualized contract slice [id:1] and the slice and contract is now invalid (also roles), but account was deleted with also provisioning delete:
After synchronized the second slice [id:2] still exists roles with new validity and account with new provisioning operation was created:
So the main problem isn't with removing roles, but with remove account during this scenario. Do you agree?
Updated by Alena Peterová over 5 years ago
- Assignee changed from Alena Peterová to Ondřej Kopr
Exactly, thank you for detailed screenshots.
Updated by Ondřej Kopr over 5 years ago
- Status changed from Needs feedback to In Progress
Alena Peterová wrote:
Exactly, thank you for detailed screenshots.
I also thank you for confirmation, Now I try the skip recalculation for contract slices during synchronization.
Updated by Ondřej Kopr over 5 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Ondřej Kopr to Vít Švanda
- % Done changed from 50 to 90
Done, I implement new skip for contract slice recalculation (it is called set_dirty_state). Dirty state is calculated by new LRT ClearDirtyStateForContractSliceTaskExecutor. Task check all dirty states in entity states and then start recalculation for each slice.
Please Vitek could you make a review?
commit: https://github.com/bcvsolutions/CzechIdMng/commit/4e6872f94905a4efc43905381e4861801262b0bd
documentation: https://wiki.czechidm.com/devel/documentation/identities/dev/contractual-relationship-slice#slices_recalculation
Thank you for check. If you want I have prepared test data with test end system.
Updated by Alena Peterová over 5 years ago
I tested this in C... project and it works fine, thanks.
Only strange thing is that the ClearDirtyStateForContractSliceTaskExecutor has different count of processed items, please see the screenshot.
The total amount is correct - it corresponds to the number of synchronized slices. But the count of processed items is sometimes lower.
EDIT: There must be something wrong with paging, because next run of ClearDirtyStateForContractSliceTaskExecutor processed the remaining 100 dirty flags and didn't process my 3 new dirty flags.
Updated by Vít Švanda over 5 years ago
- Status changed from Needs feedback to Resolved
- Assignee changed from Vít Švanda to Ondřej Kopr
- % Done changed from 90 to 100
I did review and tested sync of slices.
It works well, so thanks for nice new recalculation with dirty flags. I think this is good way how generally would be recalculation realized.
- I found problem when current using slice was changed (contract validity) and validity From of slice was changed too. On end of my analysis, the problem is not in the bulk recalculation, but in recalculation of slice (one by one).
I fixed this in the commit:
https://github.com/bcvsolutions/CzechIdMng/commit/79e4692cc10c99676ac6b1aa4df8a98ed8b8498f
Updated by Ondřej Kopr over 5 years ago
Alena Peterová wrote:
Only strange thing is that the ClearDirtyStateForContractSliceTaskExecutor has different count of processed items, please see the screenshot.
Thanks for check with more slices. I fix the pageable by removing it :D the main problem is with iterator and removing item from the result set.
Commit: https://github.com/bcvsolutions/CzechIdMng/commit/e53f4c6226e932320aeaa79c558bb4f0a8e4cd5a