🐛 fix refresh/reset/error handling

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/AdminDataCache.java

@@ -14,7 +14,7 @@ public enum AdminDataCache {
 
     COMMONHAUS_DATA(b -> b.expireAfterAccess(1, TimeUnit.HOURS)),
 
-    KNOWN_USER(b -> b.expireAfterWrite(2, TimeUnit.DAYS)),
+    KNOWN_USER(b -> b.expireAfterWrite(1, TimeUnit.DAYS)),
 
     ALIASES(b -> b.expireAfterWrite(6, TimeUnit.HOURS)),
 

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/CommonhausUser.java

@@ -70,6 +70,12 @@ public boolean updateFromPending() {
             return this == UNKNOWN
                     || this == PENDING;
         }
+
+        public boolean couldBeActiveMember() {
+            return this != MemberStatus.REVOKED
+                    && this != MemberStatus.SUSPENDED
+                    && this != MemberStatus.DECLINED;
+        }
     }
 
     @SuppressWarnings("unused")
@@ -247,18 +253,48 @@ public void addHistory(String message) {
         history.add("%s %s".formatted(when, message));
     }
 
-    boolean updateMemberStatus(AppContextService ctx, Set<String> roles) {
-        MemberStatus oldStatus = data.status;
-        if (data.status == MemberStatus.UNKNOWN && !roles.isEmpty()) {
+    public boolean isMember() {
+        return data.status.couldBeActiveMember()
+                && (isMember != null && isMember);
+    }
+
+    MemberStatus updateStatus(AppContextService ctx, Set<String> roles, MemberStatus oldStatus) {
+        MemberStatus newStatus = oldStatus;
+        if (isMember() && !roles.contains(MEMBER_ROLE)) {
+            // inconsistency: user is a member but does not have the member role
+            // this could be a missed automation step
+            ctx.logAndSendEmail("status",
+                    "%s is a member but does not have the member role (missing group?)".formatted(login()),
+                    null, null);
+            roles.add(MEMBER_ROLE);
+        }
+        if (newStatus == MemberStatus.UNKNOWN && !roles.isEmpty()) {
             List<MemberStatus> status = roles.stream()
                     .map(r -> ctx.getStatusForRole(r))
                     .sorted()
                     .toList();
-            data.status = status.get(0);
+            newStatus = status.get(0);
         }
+        return newStatus;
+    }
+
+    // read-only: test for change in status value
+    public boolean statusUpdateRequired(AppContextService ctx, Set<String> roles) {
+        MemberStatus newStatus = updateStatus(ctx, roles, data.status);
+        return data.status != newStatus;
+    }
+
+    // update user status
+    boolean updateMemberStatus(AppContextService ctx, Set<String> roles) {
+        MemberStatus oldStatus = data.status;
+        data.status = updateStatus(ctx, roles, data.status);
         return oldStatus != data.status;
     }
 
+    public void updateApplicationStatus(MemberSession session) {
+        session.applicationStatus(this.application != null);
+    }
+
     public ApiResponse toResponse() {
         return new ApiResponse(ApiResponse.Type.HAUS, data)
                 .responseStatus(postConflict() ? Response.Status.CONFLICT : Response.Status.OK);
@@ -327,8 +363,4 @@ public CommonhausUser build() {
             return new CommonhausUser(this);
         }
     }
-
-    public void updateApplicationStatus(MemberSession session) {
-        session.applicationStatus(this.application != null);
-    }
 }

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberApplicationResource.java

@@ -44,7 +44,7 @@ public class MemberApplicationResource {
     @Produces("application/json")
     public Response getApplication() {
         try {
-            CommonhausUser user = datastore.getCommonhausUser(session, false, false);
+            CommonhausUser user = datastore.getCommonhausUser(session);
             if (user == null) {
                 return Response.status(Response.Status.NOT_FOUND).build();
             }
@@ -77,7 +77,7 @@ public Response setApplication(ApplicationPost applicationPost) {
                 Log.errorf("setApplication|%s: No application data", session.login());
                 return Response.status(Response.Status.BAD_REQUEST).build();
             }
-            CommonhausUser user = datastore.getCommonhausUser(session, false, false);
+            CommonhausUser user = datastore.getCommonhausUser(session);
             if (user == null) {
                 return Response.status(Response.Status.NOT_FOUND).build();
             }

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberAttestationResource.java

@@ -49,6 +49,7 @@ public Response updateAttestation(AttestationPost post) {
         try {
             final Set<String> roles = session.roles();
             CommonhausUser user = datastore.getCommonhausUser(session, false, true);
+            // update status first so correct values are used for attestation
             user.updateMemberStatus(ctx, roles);
 
             Attestation newAttestation = createAttestation(user.status(), post);
@@ -83,6 +84,7 @@ public Response updateAttestations(List<AttestationPost> postList) {
         try {
             final Set<String> roles = session.roles();
             CommonhausUser user = datastore.getCommonhausUser(session, false, true);
+            // update status first so correct values are used for attestation
             user.updateMemberStatus(ctx, roles);
 
             Map<String, Attestation> newAttestations = new HashMap<>();

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberResource.java

@@ -14,7 +14,6 @@
 import jakarta.ws.rs.core.NewCookie;
 import jakarta.ws.rs.core.Response;
 
-import org.commonhaus.automation.admin.AdminDataCache;
 import org.commonhaus.automation.admin.github.AppContextService;
 import org.commonhaus.automation.admin.github.CommonhausDatastore;
 import org.commonhaus.automation.admin.github.CommonhausDatastore.UpdateEvent;
@@ -72,12 +71,7 @@ public Response finishLogin() {
     @KnownUser
     @Path("/me")
     @Produces("application/json")
-    public Response getUserInfo(@DefaultValue("false") @QueryParam("refresh") boolean refresh) {
-        if (refresh) {
-            AdminDataCache.KNOWN_USER.invalidate(session.login());
-            session.userIsKnown(ctx);
-        }
-
+    public Response getUserInfo() {
         if (session.hasConnection()) {
             return Response.ok(new ApiResponse(ApiResponse.Type.INFO, session.getUserData())).build();
         } else {
@@ -90,15 +84,12 @@ public Response getUserInfo(@DefaultValue("false") @QueryParam("refresh") boolea
     @KnownUser
     @Path("/commonhaus")
     @Produces("application/json")
-    public Response getCommonhausUser(@DefaultValue("false") @QueryParam("refresh") boolean refresh) {
-        if (refresh) {
-            AdminDataCache.COMMONHAUS_DATA.invalidate(CommonhausDatastore.getKey(session));
-        }
-
+    public Response getCommonhausUser() {
         try {
-            CommonhausUser user = datastore.getCommonhausUser(session, refresh, true);
+            CommonhausUser user = datastore.getCommonhausUser(session, false, true);
             if (user == null) {
-                return Response.status(Response.Status.NOT_FOUND).build();
+                // This should not happen after a fetch with create=true
+                return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
             }
             user.updateApplicationStatus(session);
             return user.toResponse()
@@ -113,11 +104,20 @@ public Response getCommonhausUser(@DefaultValue("false") @QueryParam("refresh")
     @KnownUser
     @Path("/commonhaus/status")
     @Produces("application/json")
-    public Response updateUserStatus() {
+    public Response updateUserStatus(@DefaultValue("false") @QueryParam("refresh") boolean refresh) {
+        if (refresh) {
+            // reset all the things.
+            session.forgetUser(ctx);
+
+            // re-fetch the user
+            session.userIsKnown(ctx);
+            Log.debugf("[%s] REFRESH /member/commonhaus/status %s", session.login(), session.roles());
+        }
+
         try {
-            CommonhausUser user = datastore.getCommonhausUser(session, false, false);
+            CommonhausUser user = datastore.getCommonhausUser(session, refresh, false);
             final Set<String> roles = session.roles();
-            if (user.updateMemberStatus(ctx, roles)) {
+            if (user.statusUpdateRequired(ctx, roles)) {
                 // Refresh the user's status
                 user = datastore.setCommonhausUser(new UpdateEvent(
                         user,

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/api/MemberSession.java

@@ -43,6 +43,10 @@ private MemberSession(UserInfo userInfo) {
         this.nodeId = userInfo.getString("node_id");
     }
 
+    public void forgetUser(AppContextService ctx) {
+        ctx.forgetKnown(this);
+    }
+
     public boolean userIsKnown(AppContextService ctx) {
         UserQueryContext userQc = ctx.newUserQueryContext(this);
         return ctx.userIsKnown(userQc, login(), roles());

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/AdminGitHubEvents.java

@@ -7,7 +7,6 @@
 import jakarta.inject.Inject;
 import jakarta.json.JsonObject;
 
-import org.commonhaus.automation.admin.AdminDataCache;
 import org.commonhaus.automation.admin.api.ApplicationData;
 import org.commonhaus.automation.admin.api.MemberApplicationProcess;
 import org.commonhaus.automation.admin.config.UserManagementConfig.AttestationConfig;
@@ -72,7 +71,9 @@ public void updateMembership(GitHub github, DynamicGraphQLClient graphQLClient,
         Log.debugf("[%s] updateMembership: %s",
                 installationId, eventPayload.getOrganization().getLogin());
 
-        AdminDataCache.KNOWN_USER.invalidate(eventPayload.getMember().getLogin());
+        // membership changed. Forget the user + roles
+        ctx.forgetKnown(eventPayload.getMember());
+
         GHRepository repo = eventPayload.getRepository();
         GHOrganization org = eventPayload.getOrganization();
 

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/AppContextService.java

@@ -262,6 +262,14 @@ public AttestationConfig attestationConfig() {
         return userConfig.attestations();
     }
 
+    public void forgetKnown(GHUser user) {
+        AdminDataCache.KNOWN_USER.invalidate(user.getLogin());
+    }
+
+    public void forgetKnown(MemberSession session) {
+        AdminDataCache.KNOWN_USER.invalidate(session.login());
+    }
+
     public boolean userIsKnown(QueryContext initQc, String login, Set<String> roles) {
         if (userConfig.isDisabled()) {
             return false;

cf-admin-bot/src/main/java/org/commonhaus/automation/admin/github/CommonhausDatastore.java

@@ -155,16 +155,21 @@ public Uni<CommonhausUser> fetchCommonhausUser(QueryEvent event) {
             GHRepository repo = qc.getRepository();
             result = readCommonhausUser(qc, repo, event, key);
         }
-
-        if (qc.clearNotFound() && event.create()) {
-            // create a new user
+        if (result == null && qc.clearNotFound() && event.create()) {
+            // create a new user if not found and no errors
             result = CommonhausUser.create(event.login(), event.id());
         }
+
+        // any other kind of error (including parse errors) will be logged and returned
         if (qc.hasErrors()) {
             Throwable e = qc.bundleExceptions();
             qc.clearErrors();
             Log.errorf(e, "[%s|%s] Unable to fetch user data for %s", qc.getLogId(), event.login(), e);
             return Uni.createFrom().failure(e);
+        } else if (result == null && event.create()) {
+            Exception e = new IllegalStateException("No result for user after fetch with create");
+            ctx.logAndSendEmail(qc.getLogId(), "Failed to update Commonhaus user", e, null);
+            return Uni.createFrom().failure(e);
         }
 
         Log.debugf("[%s|%s] Fetched Commonhaus user data: %s", qc.getLogId(), event.login(), result);