Enable support to store last password update time in nanoseconds #50
Conversation
| @@ -89,6 +88,13 @@ public void handleEvent(Event event) throws IdentityEventException { | |||
| } | |||
|
|
|||
| long timestamp = System.currentTimeMillis(); | |||
| String tenantDomain = (String) event.getEventProperties() | |||
There was a problem hiding this comment.
Have we handled the tenantDomain empty/null case?
| .get(IdentityEventConstants.EventProperty.TENANT_DOMAIN); | ||
|
|
||
| // Store the last password update time in nanoseconds if the configuration is enabled. | ||
| if (isStoreTimeAsNanoseconds(tenantDomain)) { |
There was a problem hiding this comment.
multiplying by 1000000 wont give the exact nanosecond time. It will only add zeros to the end of milisecond value. We need a way to get the correct nano second values. Can we check if there is a better option?
| @@ -210,6 +220,22 @@ public Properties getDefaultPropertyValues(String tenantDomain) throws IdentityG | |||
| properties.setProperty(PasswordPolicyConstants.CONNECTOR_CONFIG_PRIOR_REMINDER_TIME_IN_DAYS, | |||
| priorReminderTimeInDays); | |||
|
|
|||
| // Setting the store time as nanoseconds default value | |||
There was a problem hiding this comment.
Add a full stop at the end of the comment
There was a problem hiding this comment.
Check other places as well
| // Setting the store time as nanoseconds default value | ||
| String storeTimeAsNanoseconds = PasswordPolicyUtils.getIdentityEventProperty(tenantDomain, | ||
| PasswordPolicyConstants.CONNECTOR_CONFIG_STORE_TIME_AS_NANOSECONDS); | ||
| if (storeTimeAsNanoseconds == null) { // To avoid null pointer exceptions if user had not added module config |
There was a problem hiding this comment.
it is better to use StringUtils.isNotBlank instead of a null check here
| if (!isNanoSeconds) { | ||
| passwordChangedTime = passwordChangedTime * 1_000_000; | ||
| } | ||
| long currentTIme = System.currentTimeMillis() * 1000000; |
There was a problem hiding this comment.
We may need to get the time using the Calendar like we have did previously because when the servers are running in different time zones, there can be inconsistencies. Please verify and update accordingly
| * @param tenantDomain | ||
| * @return true if the configuration is enabled | ||
| */ | ||
| private boolean isStoreTimeAsNanoseconds(String tenantDomain) { |
There was a problem hiding this comment.
Why not move these to a Util class? Seems code duplicated
|
Closing since PR #51 covers the changes. |
The last password update time was always being stored as milliseconds. With this PR, the update time can be stored in nanoseconds. In order to cater this requirement, a new event property is introduced for
passwordExpiryevent handler. By default, the time will be stored in milliseconds. By enabling this property as in [1], the time can be stored as nanoseconds.[1]
Related issue: