Skip to content

Commit

Permalink
fix: address highly critical errorprone issues
Browse files Browse the repository at this point in the history
> Task :compileDebugJavaWithJavac
• 1 src/com/github/iusmac/sevensim/scheduler/SubscriptionScheduleEntity.java:104: error: [EqualsHashCode] Classes that override equals should also override hashCode.
    public boolean equals(final Object o) {
                   ^
    (see https://errorprone.info/bugpattern/EqualsHashCode)

• 2 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:71: error: [GuardedBy] Invalid @GuardedBy expression: static member guarded by instance
    private static Locale sDefaultLocaleCache;
                          ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 3 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:73: error: [GuardedBy] Invalid @GuardedBy expression: static member guarded by instance
    private static String[] sDaysOfWeekNarrowStrings;
                            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 4 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:269: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
        if (sDaysOfWeekNarrowStrings == null || !loc.equals(sDefaultLocaleCache)) {
            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 5 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:269: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
        if (sDaysOfWeekNarrowStrings == null || !loc.equals(sDefaultLocaleCache)) {
                                                            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 6 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:270: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
            sDaysOfWeekNarrowStrings = DateFormatSymbols.getInstance(loc)
            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 7 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:272: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
            sDefaultLocaleCache = loc;
            ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 8 src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java:274: error: [GuardedBy] This access should be guarded by 'DaysOfWeek'; instead found: 'this'
        return sDaysOfWeekNarrowStrings[dayOfWeek];
               ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 9 src/com/github/iusmac/sevensim/telephony/Subscription.java:143: error: [BoxedPrimitiveEquality] Comparison using reference equality instead of value equality. Reference equality of boxed primitive types is usually not useful, as they are value objects, and it is bug-prone, as instances are cached for some values but not others.
            && mKeepDisabledAcrossBoots == subToCompare.mKeepDisabledAcrossBoots;
                                        ^
    (see https://errorprone.info/bugpattern/BoxedPrimitiveEquality)
  Did you mean '&& Objects.equals(mKeepDisabledAcrossBoots, subToCompare.mKeepDisabledAcrossBoots);' or '&& mKeepDisabledAcrossBoots.equals(subToCompare.mKeepDisabledAcrossBoots);'?

• 10 src/com/github/iusmac/sevensim/telephony/TelephonyController.java:369: error: [GuardedBy] This access should be guarded by 'TelephonyController.this', which is not currently held
            final Subscription sub = BundleCompat.getParcelable(mRequestMetadata, KEY_SUBSCRIPTION,
                                                                ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 11 src/com/github/iusmac/sevensim/telephony/PinStorage.java:55: error: [GuardedBy] Invalid @GuardedBy expression: static member guarded by instance
    private static long sLastKeystoreAuthTimestamp;
                        ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 12 src/com/github/iusmac/sevensim/telephony/PinStorage.java:381: error: [GuardedBy] This access should be guarded by 'PinStorage'; instead found: 'this'
        final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 : sLastKeystoreAuthTimestamp +
                                 ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 13 src/com/github/iusmac/sevensim/telephony/PinStorage.java:381: error: [GuardedBy] This access should be guarded by 'PinStorage'; instead found: 'this'
        final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 : sLastKeystoreAuthTimestamp +
                                                                       ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 14 src/com/github/iusmac/sevensim/telephony/PinStorage.java:394: error: [GuardedBy] This access should be guarded by 'PinStorage'; instead found: 'PinStorage.class'
        sLastKeystoreAuthTimestamp = timestamp;
        ^
    (see https://errorprone.info/bugpattern/GuardedBy)

• 15 src/com/github/iusmac/sevensim/ui/components/ItemAdapter.java:456: error: [EqualsHashCode] Classes that override equals should also override hashCode.
        public boolean equals(final Object o) {
                       ^
    (see https://errorprone.info/bugpattern/EqualsHashCode)

Signed-off-by: iusmac <[email protected]>
  • Loading branch information
iusmac committed Aug 21, 2024
1 parent 46f7f21 commit 5ca206f
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 40 deletions.
7 changes: 4 additions & 3 deletions src/com/github/iusmac/sevensim/scheduler/DaysOfWeek.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,10 @@ public final class DaysOfWeek implements Iterable<Integer>, Comparable<DaysOfWee
/** The sum of all days of the week bit masks. */
private static final int ALL_DAYS_OF_WEEK_BITS = 0x7f;

@GuardedBy("this")
@GuardedBy("DaysOfWeek.class")
private static Locale sDefaultLocaleCache;
@GuardedBy("this")

@GuardedBy("DaysOfWeek.class")
private static String[] sDaysOfWeekNarrowStrings;

/** An encoded form of a weekly repeat schedule. */
Expand Down Expand Up @@ -266,7 +267,7 @@ public String getDisplayName(final @DayOfWeek int dayOfWeek, final boolean useLo
* @param dayOfWeek Any of {@link DayOfWeek} values.
* @return Single-character weekday name; e.g.: 'S', 'M', 'T', 'W', 'T', 'F', 'S'.
*/
public synchronized @NonNull String getNarrowDisplayName(final @DayOfWeek int dayOfWeek) {
public synchronized static @NonNull String getNarrowDisplayName(final @DayOfWeek int dayOfWeek) {
final Locale loc = Locale.getDefault();
if (sDaysOfWeekNarrowStrings == null || !loc.equals(sDefaultLocaleCache)) {
sDaysOfWeekNarrowStrings = DateFormatSymbols.getInstance(loc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import androidx.room.PrimaryKey;

import java.time.LocalTime;
import java.util.Objects;

/**
* This class is a data transfer object (DTO), representing a weekly repeat schedule for managing
Expand Down Expand Up @@ -115,6 +116,12 @@ public boolean equals(final Object o) {
&& compareTo.mTime.equals(mTime);
}

@Override
public int hashCode() {
return Objects.hash(mId, mSubscriptionId, mSubscriptionEnabled, mLabel, mEnabled,
mDaysOfWeek, mTime);
}

@Override
public String toString() {
return "SubscriptionScheduleEntity {"
Expand Down
14 changes: 8 additions & 6 deletions src/com/github/iusmac/sevensim/telephony/PinStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public final class PinStorage {
* The {@link SystemClock#elapsedRealtime()}-based time, when the hardware-backed KeyStore has
* been unlocked.
*/
@GuardedBy("this")
@GuardedBy("PinStorage.class")
private static long sLastKeystoreAuthTimestamp;

/** The default duration, in seconds, of the user authentication bound secret key. */
Expand Down Expand Up @@ -379,11 +379,13 @@ private void deleteSecretKey() {
* Check whether the user should be authenticated with their credentials in order to unlock the
* hardware-backed KeyStore for further crypto operations.
*/
public synchronized boolean isAuthenticationRequired() {
final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 : sLastKeystoreAuthTimestamp +
DEFAULT_AUTHENTICATION_VALIDITY_DURATION_SECONDS * 1000L;
final boolean isKeystoreAuthExpired = authTimeout - SystemClock.elapsedRealtime() < 0;
return mKeyguardManagerLazy.get().isDeviceSecure() && isKeystoreAuthExpired;
public boolean isAuthenticationRequired() {
synchronized (PinStorage.class) {
final long authTimeout = sLastKeystoreAuthTimestamp == 0 ? 0 :
sLastKeystoreAuthTimestamp + DEFAULT_AUTHENTICATION_VALIDITY_DURATION_SECONDS * 1000L;
final boolean isKeystoreAuthExpired = authTimeout - SystemClock.elapsedRealtime() < 0;
return mKeyguardManagerLazy.get().isDeviceSecure() && isKeystoreAuthExpired;
}
}

/**
Expand Down
36 changes: 21 additions & 15 deletions src/com/github/iusmac/sevensim/telephony/SimPinFeeder.java
Original file line number Diff line number Diff line change
Expand Up @@ -168,22 +168,28 @@ public void run() {
// Wait to be notified about new SIM state changes or finish on timeout
synchronized (simStatusChangedListener) {
if (!mSimStatusChanged) {
try {
// Note that for reliability, we want to wait a maximum of 10 seconds
// for more SIM card state change events before finishing. Normally,
// these events are delivered within a second, but in some edge cases,
// such as under high memory pressure, delivery may be delayed even by
// 2-3 seconds. This also serves as a "window" to give time to the SIM
// cards to enter the PIN state in case we started slightly earlier
simStatusChangedListener.wait(10_000L);
if (!mSimStatusChanged) {
// Timed out. No more events
break threadLoop;
}
} catch (InterruptedException ignored) {
if (mReleased) {
break threadLoop;
long nowMillis = System.currentTimeMillis();
// Note that for reliability, we want to wait a maximum of 10 seconds for
// more SIM card state change events before finishing. Normally, these
// events are delivered within a second, but in some edge cases, such as
// under high memory pressure, delivery may be delayed even by 2-3 seconds.
// This also serves as a "window" to give time to the SIM cards to enter the
// PIN state in case we started slightly earlier
final long deltaMillis = nowMillis + 10_000L;
do {
try {
simStatusChangedListener.wait(deltaMillis - nowMillis);
} catch (InterruptedException ignored) {
if (mReleased) {
break threadLoop;
}
}
nowMillis = System.currentTimeMillis();
} while (!mSimStatusChanged && nowMillis < deltaMillis);

if (!mSimStatusChanged) {
// Timed out. No more events
break threadLoop;
}
}
mSimStatusChanged = false;
Expand Down
2 changes: 1 addition & 1 deletion src/com/github/iusmac/sevensim/telephony/Subscription.java
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public boolean equals(final Object o) {
&& mName.equals(subToCompare.mName)
&& mLastActivatedTime.equals(subToCompare.mLastActivatedTime)
&& mLastDeactivatedTime.equals(subToCompare.mLastDeactivatedTime)
&& mKeepDisabledAcrossBoots == subToCompare.mKeepDisabledAcrossBoots;
&& Objects.equals(mKeepDisabledAcrossBoots, subToCompare.mKeepDisabledAcrossBoots);
}

@Override
Expand Down
41 changes: 26 additions & 15 deletions src/com/github/iusmac/sevensim/telephony/TelephonyController.java
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,22 @@ public void setSimState(final int slotIndex, final boolean enabled,

setSimPowerStateForSlot(slotIndex, simStateInt(enabled));

try {
wait(SET_SIM_POWER_STATE_REQUEST_TIMEOUT_MILLIS);
} catch (InterruptedException e) {
mLogger.w(logPrefix + "Acquire wait interrupted.");
}
// Wait for the SIM power request to complete or timeout
long nowMillis = System.currentTimeMillis();
final long deadlineMillis = nowMillis + SET_SIM_POWER_STATE_REQUEST_TIMEOUT_MILLIS;
do {
try {
wait(deadlineMillis - nowMillis);
} catch (InterruptedException e) {
mLogger.w(logPrefix + "Acquire wait interrupted.");
break;
}
if (mRequestMetadata.isEmpty()) {
// Request metadata consumed, so we break here as this wasn't a spurious wakeup
break;
}
nowMillis = System.currentTimeMillis();
} while (nowMillis < deadlineMillis);

if (Utils.IS_OLDER_THAN_S) {
mSubscriptions.removeOnSimStatusChangedListener(mSimStatusChangedListener);
Expand Down Expand Up @@ -369,17 +380,17 @@ public void onSimStatusChanged(final int slotIndex,
default: return;
}

final Subscription sub = BundleCompat.getParcelable(mRequestMetadata, KEY_SUBSCRIPTION,
Subscription.class);

if (sub.getSlotIndex() != slotIndex) {
// Since we're listening to state mutations of all available SIM cards, we can
// hypothetically receive a concurrent update for a different SIM card than the one
// whose SIM power state we expect to change
return;
}

synchronized (TelephonyController.this) {
final Subscription sub = BundleCompat.getParcelable(mRequestMetadata,
KEY_SUBSCRIPTION, Subscription.class);

if (sub.getSlotIndex() != slotIndex) {
// Since we're listening to state mutations of all available SIM cards, we can
// hypothetically receive a concurrent update for a different SIM card than the
// one whose SIM power state we expect to change
return;
}

handleOnSetSimPowerStateForSlotFinished(state);
TelephonyController.this.notifyAll();
}
Expand Down
6 changes: 6 additions & 0 deletions src/com/github/iusmac/sevensim/ui/components/ItemAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;

/**
* Base adapter class for displaying a collection of items. Provides functionality for
Expand Down Expand Up @@ -455,6 +456,11 @@ public void onRestoreInstanceState(Bundle bundle) {
// for subclassers
}

@Override
public int hashCode() {
return Objects.hash(itemId, item);
}

@Override
public boolean equals(final Object o) {
if (o == this) return true;
Expand Down

0 comments on commit 5ca206f

Please sign in to comment.