Fix transaction capping bug.

We don't want to give out credits if that would result in there being
too many credits in circulation or the app accumulating too many
credits, so we cap the transaction delta to stay within the limits. We
were accidentally taking away credits because of the capping, so this
makes sure we don't do that.

Bug: 158300259
Test: atest FrameworksMockingServicesTests:AgentTest
Change-Id: Idc82af353d1021577ead2e0038145dfb8cdffeab
This commit is contained in:
Kweku Adams
2021-08-25 07:04:31 -07:00
parent 25db6e5016
commit ba9da63453
2 changed files with 195 additions and 5 deletions

View File

@@ -28,6 +28,7 @@ import static com.android.server.tare.EconomicPolicy.eventToString;
import static com.android.server.tare.EconomicPolicy.getEventType;
import static com.android.server.tare.TareUtils.appToString;
import static com.android.server.tare.TareUtils.getCurrentTimeMillis;
import static com.android.server.tare.TareUtils.narcToString;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -460,8 +461,9 @@ class Agent {
Math.min(ongoingEvent.reward.maxDailyReward - rewardSum, computedDelta));
}
@VisibleForTesting
@GuardedBy("mLock")
private void recordTransactionLocked(final int userId, @NonNull final String pkgName,
void recordTransactionLocked(final int userId, @NonNull final String pkgName,
@NonNull Ledger ledger, @NonNull Ledger.Transaction transaction,
final boolean notifyOnAffordabilityChange) {
if (transaction.delta == 0) {
@@ -476,12 +478,14 @@ class Agent {
final long maxCirculationAllowed = mIrs.getMaxCirculationLocked();
final long newArcsInCirculation = mCurrentNarcsInCirculation + transaction.delta;
if (transaction.delta > 0 && newArcsInCirculation > maxCirculationAllowed) {
final long newDelta = maxCirculationAllowed - mCurrentNarcsInCirculation;
// Set lower bound at 0 so we don't accidentally take away credits when we were trying
// to _give_ the app credits.
final long newDelta = Math.max(0, maxCirculationAllowed - mCurrentNarcsInCirculation);
Slog.i(TAG, "Would result in too many credits in circulation. Decreasing transaction "
+ eventToString(transaction.eventId)
+ (transaction.tag == null ? "" : ":" + transaction.tag)
+ " for " + appToString(userId, pkgName)
+ " by " + (transaction.delta - newDelta));
+ " by " + narcToString(transaction.delta - newDelta));
transaction = new Ledger.Transaction(
transaction.startTimeMs, transaction.endTimeMs,
transaction.eventId, transaction.tag, newDelta);
@@ -490,12 +494,15 @@ class Agent {
if (transaction.delta > 0
&& originalBalance + transaction.delta
> mCompleteEconomicPolicy.getMaxSatiatedBalance()) {
final long newDelta = mCompleteEconomicPolicy.getMaxSatiatedBalance() - originalBalance;
// Set lower bound at 0 so we don't accidentally take away credits when we were trying
// to _give_ the app credits.
final long newDelta =
Math.max(0, mCompleteEconomicPolicy.getMaxSatiatedBalance() - originalBalance);
Slog.i(TAG, "Would result in becoming too rich. Decreasing transaction "
+ eventToString(transaction.eventId)
+ (transaction.tag == null ? "" : ":" + transaction.tag)
+ " for " + appToString(userId, pkgName)
+ " by " + (transaction.delta - newDelta));
+ " by " + narcToString(transaction.delta - newDelta));
transaction = new Ledger.Transaction(
transaction.startTimeMs, transaction.endTimeMs,
transaction.eventId, transaction.tag, newDelta);

View File

@@ -0,0 +1,183 @@
/*
* Copyright (C) 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.server.tare;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import android.app.AlarmManager;
import android.content.Context;
import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
import com.android.server.LocalServices;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoSession;
import org.mockito.quality.Strictness;
/** Tests various aspects of the Agent. */
@RunWith(AndroidJUnit4.class)
@SmallTest
public class AgentTest {
private MockitoSession mMockingSession;
@Mock
private CompleteEconomicPolicy mEconomicPolicy;
@Mock
private Context mContext;
@Mock
private InternalResourceService mIrs;
@Before
public void setUp() {
mMockingSession = mockitoSession()
.initMocks(this)
.strictness(Strictness.LENIENT)
.mockStatic(LocalServices.class)
.startMocking();
when(mIrs.getContext()).thenReturn(mContext);
when(mContext.getSystemService(Context.ALARM_SERVICE)).thenReturn(mock(AlarmManager.class));
}
@After
public void tearDown() {
if (mMockingSession != null) {
mMockingSession.finishMocking();
}
}
@Test
public void testRecordTransaction_UnderMax() {
Agent agent = new Agent(mIrs, mEconomicPolicy);
Ledger ledger = new Ledger();
doReturn(1_000_000L).when(mIrs).getMaxCirculationLocked();
doReturn(1_000_000L).when(mEconomicPolicy).getMaxSatiatedBalance();
Ledger.Transaction transaction = new Ledger.Transaction(0, 0, 0, null, 5);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(5, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, 995);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1000, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, -500);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(500, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, 999_500L);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1_000_000L, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, -1_000_001L);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(-1, ledger.getCurrentBalance());
}
@Test
public void testRecordTransaction_MaxCirculation() {
Agent agent = new Agent(mIrs, mEconomicPolicy);
Ledger ledger = new Ledger();
doReturn(1000L).when(mIrs).getMaxCirculationLocked();
doReturn(1000L).when(mEconomicPolicy).getMaxSatiatedBalance();
Ledger.Transaction transaction = new Ledger.Transaction(0, 0, 0, null, 5);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(5, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, 995);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1000, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, -500);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(500, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, 2000);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1000, ledger.getCurrentBalance());
// MaxCirculation can change as the battery level changes. Any already allocated ARCSs
// shouldn't be removed by recordTransaction().
doReturn(900L).when(mIrs).getMaxCirculationLocked();
transaction = new Ledger.Transaction(0, 0, 0, null, 100);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1000, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, -50);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(950, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, -200);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(750, ledger.getCurrentBalance());
doReturn(800L).when(mIrs).getMaxCirculationLocked();
transaction = new Ledger.Transaction(0, 0, 0, null, 100);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(800, ledger.getCurrentBalance());
}
@Test
public void testRecordTransaction_MaxSatiatedBalance() {
Agent agent = new Agent(mIrs, mEconomicPolicy);
Ledger ledger = new Ledger();
doReturn(1_000_000L).when(mIrs).getMaxCirculationLocked();
doReturn(1000L).when(mEconomicPolicy).getMaxSatiatedBalance();
Ledger.Transaction transaction = new Ledger.Transaction(0, 0, 0, null, 5);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(5, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, 995);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1000, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, -500);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(500, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, 999_500L);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1_000, ledger.getCurrentBalance());
// Shouldn't change in normal operation, but adding test case in case it does.
doReturn(900L).when(mEconomicPolicy).getMaxSatiatedBalance();
transaction = new Ledger.Transaction(0, 0, 0, null, 500);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(1_000, ledger.getCurrentBalance());
transaction = new Ledger.Transaction(0, 0, 0, null, -1001);
agent.recordTransactionLocked(0, "com.test", ledger, transaction, false);
assertEquals(-1, ledger.getCurrentBalance());
}
}