Remove expiration check from Device#isEnabled()

This commit is contained in:
Jon Chambers
2024-06-07 13:39:11 -04:00
committed by GitHub
parent b376458963
commit 2f55747601
25 changed files with 99 additions and 128 deletions

View File

@@ -23,9 +23,9 @@ import org.whispersystems.textsecuregcm.util.Pair;
/**
* This {@link WebsocketRefreshRequirementProvider} observes intra-request changes in {@link Account#isEnabled()} and
* {@link Device#isEnabled()}.
* {@link Device#hasMessageDeliveryChannel()}.
* <p>
* If a change in {@link Account#isEnabled()} or any associated {@link Device#isEnabled()} is observed, then any active
* If a change in {@link Account#isEnabled()} or any associated {@link Device#hasMessageDeliveryChannel()} is observed, then any active
* WebSocket connections for the account must be closed in order for clients to get a refreshed
* {@link io.dropwizard.auth.Auth} object with a current device list.
*

View File

@@ -17,7 +17,7 @@ import java.util.stream.Collectors;
class ContainerRequestUtil {
private static Map<Byte, Boolean> buildDevicesEnabledMap(final Account account) {
return account.getDevices().stream().collect(Collectors.toMap(Device::getId, Device::isEnabled));
return account.getDevices().stream().collect(Collectors.toMap(Device::getId, Device::hasMessageDeliveryChannel));
}
/**

View File

@@ -31,7 +31,7 @@ public class OptionalAccess {
Optional<Device> targetDevice = targetAccount.get().getDevice(deviceId);
if (targetDevice.isPresent() && targetDevice.get().isEnabled()) {
if (targetDevice.isPresent() && targetDevice.get().hasMessageDeliveryChannel()) {
return;
}

View File

@@ -440,11 +440,11 @@ public class KeysController {
private List<Device> parseDeviceId(String deviceId, Account account) {
if (deviceId.equals("*")) {
return account.getDevices().stream().filter(Device::isEnabled).toList();
return account.getDevices().stream().filter(Device::hasMessageDeliveryChannel).toList();
}
try {
byte id = Byte.parseByte(deviceId);
return account.getDevice(id).filter(Device::isEnabled).map(List::of).orElse(List.of());
return account.getDevice(id).filter(Device::hasMessageDeliveryChannel).map(List::of).orElse(List.of());
} catch (NumberFormatException e) {
throw new WebApplicationException(Response.status(422).build());
}

View File

@@ -39,7 +39,7 @@ class KeysGrpcHelper {
: Flux.from(Mono.justOrEmpty(targetAccount.getDevice(targetDeviceId)));
return devices
.filter(Device::isEnabled)
.filter(Device::hasMessageDeliveryChannel)
.switchIfEmpty(Mono.error(Status.NOT_FOUND.asException()))
.flatMap(device -> Flux.merge(
Mono.fromFuture(() -> keysManager.takeEC(targetAccount.getIdentifier(identityType), device.getId())),

View File

@@ -296,14 +296,14 @@ public class Account {
requireNotStale();
return devices.stream()
.filter(Device::isEnabled)
.filter(Device::hasMessageDeliveryChannel)
.allMatch(device -> device.getCapabilities() != null && predicate.test(device.getCapabilities()));
}
public boolean isEnabled() {
requireNotStale();
return getPrimaryDevice().isEnabled();
return getPrimaryDevice().hasMessageDeliveryChannel();
}
public byte getNextDeviceId() {
@@ -327,7 +327,7 @@ public class Account {
return devices.stream()
.filter(d -> Device.PRIMARY_ID != d.getId())
.anyMatch(Device::isEnabled);
.anyMatch(Device::hasMessageDeliveryChannel);
}
public void setIdentityKey(final IdentityKey identityKey) {

View File

@@ -490,7 +490,7 @@ public class AccountsManager {
account.getDevices()
.stream()
.filter(Device::isEnabled)
.filter(Device::hasMessageDeliveryChannel)
.forEach(device -> device.setPhoneNumberIdentityRegistrationId(pniRegistrationIds.get(device.getId())));
account.setPhoneNumberIdentityKey(pniIdentityKey);

View File

@@ -11,7 +11,6 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.time.Duration;
import java.util.List;
import java.util.OptionalInt;
import java.util.UUID;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import javax.annotation.Nullable;
@@ -199,10 +198,8 @@ public class Device {
this.capabilities = capabilities;
}
public boolean isEnabled() {
boolean hasChannel = fetchesMessages || StringUtils.isNotEmpty(getApnId()) || StringUtils.isNotEmpty(getGcmId());
return (id == PRIMARY_ID && hasChannel) || (id != PRIMARY_ID && hasChannel && !isExpired());
public boolean hasMessageDeliveryChannel() {
return fetchesMessages || StringUtils.isNotEmpty(getApnId()) || StringUtils.isNotEmpty(getGcmId());
}
public boolean isExpired() {

View File

@@ -245,7 +245,7 @@ public class MessagePersister implements Managed {
// its messages) is unlinked
final Device deviceToDelete = account.getDevices()
.stream()
.filter(d -> !d.isPrimary() && !d.isEnabled())
.filter(d -> !d.isPrimary() && !d.hasMessageDeliveryChannel())
.min(Comparator.comparing(Device::getLastSeen))
.or(() ->
Flux.fromIterable(account.getDevices())

View File

@@ -89,7 +89,7 @@ public class DestinationDeviceValidator {
final Set<Byte> excludedDeviceIds) throws MismatchedDevicesException {
final Set<Byte> accountDeviceIds = account.getDevices().stream()
.filter(Device::isEnabled)
.filter(Device::hasMessageDeliveryChannel)
.map(Device::getId)
.filter(deviceId -> !excludedDeviceIds.contains(deviceId))
.collect(Collectors.toSet());

View File

@@ -130,7 +130,7 @@ public class ProcessPushNotificationFeedbackCommand extends AbstractSinglePassCr
@VisibleForTesting
boolean deviceNeedsUpdate(final Device device) {
return pushFeedbackIntervalElapsed(device) && (device.isEnabled() || device.getLastSeen() > device.getUninstalledFeedbackTimestamp());
return pushFeedbackIntervalElapsed(device) && (device.hasMessageDeliveryChannel() || device.getLastSeen() > device.getUninstalledFeedbackTimestamp());
}
@VisibleForTesting