Skip to content

Commit

Permalink
Avoid NullPointerException
Browse files Browse the repository at this point in the history
  • Loading branch information
@enricovianello
enricovianello committed Dec 19, 2024
1 parent 35240c2 commit 3a51b67
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 29 deletions.
29 changes: 14 additions & 15 deletions ...ogin-service/src/main/java/it/infn/mw/iam/core/lifecycle/cern/CernHrLifecycleHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
}

Expand All @@ -126,22 +124,27 @@ 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)) {
restoreAccount(a);
}
}

private void expireIfActiveAndMember(IamAccount a) {
if (CernHrLifecycleUtils.isActiveMembership(a.getEndTime()) && a.isActive()) {
expireAccount(a);
}
}

@Override
public void run() {

Expand Down Expand Up @@ -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());
}
}
4 changes: 2 additions & 2 deletions ...rc/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleDisableUserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down
24 changes: 12 additions & 12 deletions ...n-service/src/test/java/it/infn/mw/iam/test/lifecycle/cern/CernAccountLifecycleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public void testUserSuspensionWorksAfterCernHrEndTimeUpdate() {
Optional<IamLabel> 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<IamLabel> cernMessageLabel =
testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE);
Expand All @@ -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));
Expand Down Expand Up @@ -225,7 +225,7 @@ public void testUserRemovalWorksAfterCernHrEndTimeUpdate() {
Optional<IamLabel> 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<IamLabel> cernMessageLabel =
testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE);
Expand Down Expand Up @@ -266,7 +266,7 @@ public void testLifecycleWorksForValidAccounts() {
Optional<IamLabel> 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<IamLabel> cernMessageLabel =
testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE);
Expand Down Expand Up @@ -316,7 +316,7 @@ public void testLifecycleWorksForAccountsWithOneValidParticipationAndOneExpired(
Optional<IamLabel> 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<IamLabel> cernMessageLabel =
testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE);
Expand Down Expand Up @@ -354,7 +354,7 @@ public void testLifecycleWorksForAccountsWithOneUnlimitedParticipationAndOneExpi
Optional<IamLabel> 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<IamLabel> cernMessageLabel =
testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE);
Expand Down Expand Up @@ -391,7 +391,7 @@ public void testLifecycleWhenVOPersonEndDateIsNull() {
Optional<IamLabel> 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<IamLabel> cernMessageLabel =
testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_MESSAGE);
Expand Down Expand Up @@ -432,7 +432,7 @@ public void testRestoreLifecycleWorks() {
Optional<IamLabel> 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)));
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -545,7 +545,7 @@ public void testLifecycleNotRestoreAccountsSuspendedByAdmins() {
Optional<IamLabel> 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<IamLabel> timestampLabel =
testAccount.getLabelByPrefixAndName(LABEL_CERN_PREFIX, LABEL_TIMESTAMP);
Expand Down Expand Up @@ -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));
}
Expand Down

0 comments on commit 3a51b67

Please sign in to comment.