From 3a51b67d6d465099fb7a18bb7eb2e553a0416b75 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Wed, 18 Dec 2024 23:20:00 +0100 Subject: [PATCH] Avoid NullPointerException --- .../cern/CernHrLifecycleHandler.java | 29 +++++++++---------- .../CernAccountLifecycleDisableUserTests.java | 4 +-- .../cern/CernAccountLifecycleTests.java | 24 +++++++-------- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernHrLifecycleHandler.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernHrLifecycleHandler.java index 9aa4d7a63..60bf967c9 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernHrLifecycleHandler.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernHrLifecycleHandler.java @@ -76,7 +76,7 @@ public class CernHrLifecycleHandler implements Runnable, SchedulingConfigurer { public static final Logger LOG = LoggerFactory.getLogger(CernHrLifecycleHandler.class); public enum CernStatus { - IGNORED, ERROR, NOT_FOUND, NOT_MEMBER, OK + IGNORED, ERROR, EXPIRED, VO_MEMBER } private final CernProperties cernProperties; @@ -100,7 +100,6 @@ public void handleAccount(String cernPersonId, String experiment, IamAccount a) deleteDeprecatedLabels(a); if (CernHrLifecycleUtils.isAccountIgnored(a)) { - LOG.debug("Ignoring account '{}'", a.getUsername()); setCernStatusLabel(a, CernStatus.IGNORED, IGNORE_MESSAGE); return; } @@ -114,9 +113,8 @@ public void handleAccount(String cernPersonId, String experiment, IamAccount a) return; } if (voPerson.isEmpty()) { - LOG.debug("No record found for person id {}", cernPersonId); - setCernStatusLabel(a, CernStatus.NOT_FOUND, format(NO_PERSON_FOUND_MESSAGE, cernPersonId)); - expireIfActiveAndValid(a); + expireIfActiveAndMember(a); + setCernStatusLabel(a, CernStatus.EXPIRED, format(NO_PERSON_FOUND_MESSAGE, cernPersonId)); return; } @@ -126,15 +124,14 @@ public void handleAccount(String cernPersonId, String experiment, IamAccount a) .getMostRecentMembership(voPerson.get().getParticipations(), experiment); if (ep.isEmpty()) { - LOG.warn("No membership to '{}' found for person id {}", experiment, cernPersonId); - setCernStatusLabel(a, CernStatus.NOT_MEMBER, format(NO_PARTICIPATION_MESSAGE, experiment)); - expireIfActiveAndValid(a); + expireIfActiveAndMember(a); + setCernStatusLabel(a, CernStatus.EXPIRED, format(NO_PARTICIPATION_MESSAGE, experiment)); return; } syncInstitute(a, ep.get().getInstitute()); syncAccountEndTime(a, ep.get().getEndDate()); - setCernStatusLabel(a, CernStatus.OK, format(SYNCHRONIZED_MESSAGE)); + setCernStatusLabel(a, CernStatus.VO_MEMBER, format(SYNCHRONIZED_MESSAGE)); if (CernHrLifecycleUtils.isActiveMembership(a.getEndTime()) && !a.isActive() && accountWasSuspendedByIamLifecycleJob(a)) { @@ -142,6 +139,12 @@ && accountWasSuspendedByIamLifecycleJob(a)) { } } + private void expireIfActiveAndMember(IamAccount a) { + if (CernHrLifecycleUtils.isActiveMembership(a.getEndTime()) && a.isActive()) { + expireAccount(a); + } + } + @Override public void run() { @@ -252,11 +255,7 @@ private void syncAccountEndTime(IamAccount a, Date endDate) { } } - private void expireIfActiveAndValid(IamAccount a) { - Date currentDate = new Date(); - if (a.isActive() && a.getEndTime().after(currentDate)) { - LOG.warn("User {} has a valid membership but cannot be found on HR db", a.getUsername()); - accountService.setAccountEndTime(a, currentDate); - } + private void expireAccount(IamAccount a) { + accountService.setAccountEndTime(a, new Date()); } } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleDisableUserTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleDisableUserTests.java index ae902c14c..60fcc0acb 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleDisableUserTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleDisableUserTests.java @@ -162,7 +162,7 @@ public void testCernPersonIdNotFoundMeansUserEndTimeIsResetToCurrentDate() { testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); assertThat(statusLabel.isPresent(), is(true)); - assertThat(statusLabel.get().getValue(), is(CernHrLifecycleHandler.CernStatus.NOT_FOUND.name())); + assertThat(statusLabel.get().getValue(), is(CernHrLifecycleHandler.CernStatus.EXPIRED.name())); assertThat(timestampLabel.isPresent(), is(false)); @@ -208,7 +208,7 @@ public void testNoParticipationIsFoundMeansUserEndTimeIsResetToCurrentDate() { assertThat(statusLabel.isPresent(), is(true)); assertThat(statusLabel.get().getValue(), - is(CernHrLifecycleHandler.CernStatus.NOT_MEMBER.name())); + is(CernHrLifecycleHandler.CernStatus.EXPIRED.name())); assertThat(timestampLabel.isPresent(), is(false)); diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java index 95fecc732..1e2f28d59 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java @@ -174,7 +174,7 @@ public void testUserSuspensionWorksAfterCernHrEndTimeUpdate() { Optional cernStatusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); Optional cernMessageLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); @@ -193,7 +193,7 @@ public void testUserSuspensionWorksAfterCernHrEndTimeUpdate() { cernStatusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); cernMessageLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); assertThat(cernMessageLabel.isPresent(), is(true)); @@ -225,7 +225,7 @@ public void testUserRemovalWorksAfterCernHrEndTimeUpdate() { Optional cernStatusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); Optional cernMessageLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); @@ -266,7 +266,7 @@ public void testLifecycleWorksForValidAccounts() { Optional cernStatusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); Optional cernMessageLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); @@ -316,7 +316,7 @@ public void testLifecycleWorksForAccountsWithOneValidParticipationAndOneExpired( Optional cernStatusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); Optional cernMessageLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); @@ -354,7 +354,7 @@ public void testLifecycleWorksForAccountsWithOneUnlimitedParticipationAndOneExpi Optional cernStatusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); Optional cernMessageLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); @@ -391,7 +391,7 @@ public void testLifecycleWhenVOPersonEndDateIsNull() { Optional cernStatusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); Optional cernMessageLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE); @@ -432,7 +432,7 @@ public void testRestoreLifecycleWorks() { Optional iamStatusLabel = testAccount.getLabelByName(LIFECYCLE_STATUS_LABEL); assertThat(cernStatusLabel.isPresent(), is(true)); - assertThat(cernStatusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(cernStatusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); assertThat(cernMessageLabel.isPresent(), is(true)); assertThat(cernMessageLabel.get().getValue(), is(format(SYNCHRONIZED_MESSAGE))); @@ -488,7 +488,7 @@ public void testApiReturnsNullVoPersonIsHandled() { assertThat(statusLabel.isPresent(), is(true)); assertThat(statusLabel.get().getValue(), - is(CernHrLifecycleHandler.CernStatus.NOT_FOUND.name())); + is(CernHrLifecycleHandler.CernStatus.EXPIRED.name())); assertThat(timestampLabel.isPresent(), is(false)); @@ -517,7 +517,7 @@ public void testApiReturnsVoPersonWithNoParticipationsIsHandled() { assertThat(statusLabel.isPresent(), is(true)); assertThat(statusLabel.get().getValue(), - is(CernHrLifecycleHandler.CernStatus.NOT_MEMBER.name())); + is(CernHrLifecycleHandler.CernStatus.EXPIRED.name())); assertThat(timestampLabel.isPresent(), is(false)); @@ -545,7 +545,7 @@ public void testLifecycleNotRestoreAccountsSuspendedByAdmins() { Optional statusLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_STATUS); assertThat(statusLabel.isPresent(), is(true)); - assertThat(statusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(statusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); Optional timestampLabel = testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_TIMESTAMP); @@ -607,7 +607,7 @@ public void testPaginationWorks() { account.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_TIMESTAMP); assertThat(statusLabel.isPresent(), is(true)); - assertThat(statusLabel.get().getValue(), is(CernStatus.OK.name())); + assertThat(statusLabel.get().getValue(), is(CernStatus.VO_MEMBER.name())); assertThat(timestampLabel.isPresent(), is(false)); }