CurrentUser: Return Optional from methods that retrieve a property

Returning Optional makes it more explicit that callers must expect that
the return value is absent.

Change-Id: If739844482ac8966dbcdc2285869cfac7ab7e059
Signed-off-by: Edwin Kempin <ekempin@google.com>

java/com/google/gerrit/server/CurrentUser.java

@@ -125,11 +125,10 @@ public abstract class CurrentUser {
    * Lookup a previously stored property.
    *
    * @param key unique property key.
-   * @return previously stored value, or {@code null}.
+   * @return previously stored value, or {@code Optional#empty()}.
    */
-  @Nullable
+  public <T> Optional<T> get(PropertyKey<T> key) {
-  public <T> T get(PropertyKey<T> key) {
+    return Optional.empty();
-    return null;
   }
 
   /**
@@ -144,7 +143,7 @@ public abstract class CurrentUser {
     put(lastLoginExternalIdPropertyKey, externalIdKey);
   }
 
-  public ExternalId.Key getLastLoginExternalIdKey() {
+  public Optional<ExternalId.Key> getLastLoginExternalIdKey() {
     return get(lastLoginExternalIdPropertyKey);
   }
 }

java/com/google/gerrit/server/IdentifiedUser.java

@@ -450,14 +450,13 @@ public class IdentifiedUser extends CurrentUser {
   }
 
   @Override
-  @Nullable
+  public synchronized <T> Optional<T> get(PropertyKey<T> key) {
-  public synchronized <T> T get(PropertyKey<T> key) {
     if (properties != null) {
       @SuppressWarnings("unchecked")
       T value = (T) properties.get(key);
-      return value;
+      return Optional.ofNullable(value);
     }
-    return null;
+    return Optional.empty();
   }
 
   /**

java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java

@@ -37,6 +37,7 @@ import java.io.IOException;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 
 @Singleton
@@ -159,18 +160,21 @@ public class DefaultPermissionBackend extends PermissionBackend {
     }
 
     private Boolean computeAdmin() {
-      Boolean r = user.get(IS_ADMIN);
+      Optional<Boolean> r = user.get(IS_ADMIN);
-      if (r == null) {
+      if (r.isPresent()) {
-        if (user.isImpersonating()) {
+        return r.get();
-          r = false;
-        } else if (user instanceof PeerDaemonUser) {
-          r = true;
-        } else {
-          r = allow(capabilities().administrateServer);
-        }
-        user.put(IS_ADMIN, r);
       }
-      return r;
+
+      boolean isAdmin;
+      if (user.isImpersonating()) {
+        isAdmin = false;
+      } else if (user instanceof PeerDaemonUser) {
+        isAdmin = true;
+      } else {
+        isAdmin = allow(capabilities().administrateServer);
+      }
+      user.put(IS_ADMIN, isAdmin);
+      return isAdmin;
     }
 
     private boolean canEmailReviewers() {

java/com/google/gerrit/server/restapi/account/DeleteExternalIds.java

@@ -41,6 +41,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 
 @Singleton
@@ -81,7 +82,7 @@ public class DeleteExternalIds implements RestModifyView<AccountResource, List<S
             .collect(toMap(i -> i.key(), i -> i));
 
     List<ExternalId> toDelete = new ArrayList<>();
-    ExternalId.Key last = resource.getUser().getLastLoginExternalIdKey();
+    Optional<ExternalId.Key> last = resource.getUser().getLastLoginExternalIdKey();
     for (String externalIdStr : extIds) {
       ExternalId id = externalIdMap.get(ExternalId.Key.parse(externalIdStr));
 
@@ -91,7 +92,7 @@ public class DeleteExternalIds implements RestModifyView<AccountResource, List<S
       }
 
       if ((!id.isScheme(SCHEME_USERNAME))
-          && ((last == null) || (!last.get().equals(id.key().get())))) {
+          && (!last.isPresent() || (!last.get().equals(id.key())))) {
         toDelete.add(id);
       } else {
         throw new ResourceConflictException(

java/com/google/gerrit/server/restapi/account/GetExternalIds.java

@@ -37,6 +37,7 @@ import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 @Singleton
 public class GetExternalIds implements RestReadView<AccountResource> {
@@ -78,8 +79,8 @@ public class GetExternalIds implements RestReadView<AccountResource> {
       // establish this web session, and if only if an identity was
       // actually used to establish this web session.
       if (!id.isScheme(SCHEME_USERNAME)) {
-        ExternalId.Key last = resource.getUser().getLastLoginExternalIdKey();
+        Optional<ExternalId.Key> last = resource.getUser().getLastLoginExternalIdKey();
-        info.canDelete = toBoolean(last == null || !last.get().equals(info.identity));
+        info.canDelete = toBoolean(!last.isPresent() || !last.get().get().equals(info.identity));
       }
       result.add(info);
     }