Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aec2604c authored by James.cf Lin's avatar James.cf Lin
Browse files

[UCE] Fix the CTS ImsServiceTest#testRcsPublishThrottle failed

The UCE framework received two consecutive publishing request in a short period of time.
It caused the invalidation of the publish allowed check.

Fix this issue by using a publish lock to protect the process of the publish request.

Bug: 185201813
Test: atest -c CtsTelephonyTestCases:android.telephony.ims.cts.ImsServiceTest
Change-Id: I825ffef4dd358edc8b01fcba3ff0f7d9c693b814
parent e2913d09
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ import android.util.LocalLog;
import android.util.Log;

import com.android.ims.rcs.uce.presence.publish.PublishController.PublishControllerCallback;
import com.android.ims.rcs.uce.presence.publish.PublishController.PublishTriggerType;
import com.android.ims.rcs.uce.util.UceUtils;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.telephony.util.HandlerExecutor;
@@ -135,7 +136,8 @@ public class DeviceCapabilityListener {
            sendEmptyMessage(EVENT_UNREGISTER_IMS_CHANGE);
        }

        public void sendTriggeringPublishMessage(int type) {
        public void sendTriggeringPublishMessage(@PublishTriggerType int type) {
            logd("sendTriggeringPublishMessage: type=" + type);
            // Remove the existing message and resend a new message.
            removeMessages(EVENT_REQUEST_PUBLISH);
            Message message = obtainMessage();
+270 −133

File changed.

Preview size limit exceeded, changes collapsed.

+65 −39
Original line number Diff line number Diff line
@@ -57,10 +57,13 @@ public class PublishProcessor {
    private PublishProcessorState mProcessorState;

    // The information of the device's capabilities.
    private DeviceCapabilityInfo mDeviceCapabilities;
    private final DeviceCapabilityInfo mDeviceCapabilities;

    // The callback of the PublishController
    private PublishControllerCallback mPublishCtrlCallback;
    private final PublishControllerCallback mPublishCtrlCallback;

    // The lock of processing the pending request.
    private final Object mPendingRequestLock = new Object();

    private final LocalLog mLocalLog = new LocalLog(UceUtils.LOG_SIZE);

@@ -108,43 +111,55 @@ public class PublishProcessor {
     * @param triggerType The type of triggering the publish request.
     */
    public void doPublish(@PublishTriggerType int triggerType) {
        if (mIsDestroyed) return;
        mProcessorState.setPublishingFlag(true);
        if (!doPublishInternal(triggerType)) {
            // Reset the publishing flag if the request cannot be sent to the IMS service.
            mProcessorState.setPublishingFlag(false);
        }
    }
    /**
     * Execute the publish request internally.
     * @param triggerType The type of triggering the publish request.
     * @return true if the publish is sent to the IMS service successfully, false otherwise.
     */
    private boolean doPublishInternal(@PublishTriggerType int triggerType) {
        if (mIsDestroyed) return false;

        mLocalLog.log("doPublish: trigger type=" + triggerType);
        logi("doPublish: trigger type=" + triggerType);
        mLocalLog.log("doPublishInternal: trigger type=" + triggerType);
        logi("doPublishInternal: trigger type=" + triggerType);

        // Return if this request is not allowed to be executed.
        if (!isRequestAllowed(triggerType)) {
            mLocalLog.log("doPublish: The request is not allowed.");
            return;
            mLocalLog.log("doPublishInternal: The request is not allowed.");
            return false;
        }

        // Get the latest device's capabilities.
        RcsContactUceCapability deviceCapability =
                mDeviceCapabilities.getDeviceCapabilities(CAPABILITY_MECHANISM_PRESENCE, mContext);
        if (deviceCapability == null) {
            logw("doPublish: device capability is null");
            return;
            logw("doPublishInternal: device capability is null");
            return false;
        }

        // Convert the device's capabilities to pidf format.
        String pidfXml = PidfParser.convertToPidf(deviceCapability);
        if (TextUtils.isEmpty(pidfXml)) {
            logw("doPublish: pidfXml is empty");
            return;
            logw("doPublishInternal: pidfXml is empty");
            return false;
        }

        // Set the pending request and return if RCS is not connected. When the RCS is connected
        // afterward, it will send a new request if there's a pending request.
        RcsFeatureManager featureManager = mRcsFeatureManager;
        if (featureManager == null) {
            logw("doPublish: RCS is not connected.");
            logw("doPublishInternal: RCS is not connected.");
            setPendingRequest(triggerType);
            return;
            return false;
        }

        // Publish to the Presence server.
        publishCapabilities(featureManager, pidfXml);
        return publishCapabilities(featureManager, pidfXml);
    }

    /*
@@ -172,14 +187,6 @@ public class PublishProcessor {
            return false;
        }

        // Set the pending flag if there's already a request running now. When the running request
        // is finished and there is a pending request, it will send a new request.
        if (mProcessorState.isPublishingNow()) {
            logd("isPublishAllowed: There is already a request running now");
            setPendingRequest(triggerType);
            return false;
        }

        // Skip this request if the PUBLISH is not allowed at current time. Resend the PUBLISH
        // request and it will be triggered with an appropriate delay time.
        if (!mProcessorState.isPublishAllowedAtThisTime()) {
@@ -191,15 +198,12 @@ public class PublishProcessor {
    }

    // Publish the device capabilities with the given pidf.
    private void publishCapabilities(@NonNull RcsFeatureManager featureManager,
    private boolean publishCapabilities(@NonNull RcsFeatureManager featureManager,
            @NonNull String pidfXml) {
        PublishRequestResponse requestResponse = null;
        try {
            // Set publishing flag
            mProcessorState.setPublishingFlag(true);

            // Clear the pending flag because it is going to send the latest device's capabilities.
            mProcessorState.clearPendingRequest();
            clearPendingRequest();

            // Generate a unique taskId to track this request.
            long taskId = mProcessorState.generatePublishTaskId();
@@ -213,13 +217,14 @@ public class PublishProcessor {

            // Send a request canceled timer to avoid waiting too long for the response callback.
            mPublishCtrlCallback.setupRequestCanceledTimer(taskId, RESPONSE_CALLBACK_WAITING_TIME);

            return true;
        } catch (RemoteException e) {
            mLocalLog.log("publish capability exception: " + e.getMessage());
            logw("publishCapabilities: exception=" + e.getMessage());
            // Exception occurred, end this request.
            setRequestEnded(requestResponse);
            checkAndSendPendingRequest();
            return false;
        }
   }

@@ -307,7 +312,7 @@ public class PublishProcessor {
        mProcessorState.increaseRetryCount();

        // Reset the pending flag because it is going to resend a request.
        mProcessorState.clearPendingRequest();
        clearPendingRequest();

        // Finish this request and resend a new publish request
        setRequestEnded(requestResponse);
@@ -369,13 +374,16 @@ public class PublishProcessor {
     * Set the pending flag when it cannot be executed now.
     */
    public void setPendingRequest(@PublishTriggerType int triggerType) {
        synchronized (mPendingRequestLock) {
            mProcessorState.setPendingRequest(triggerType);
        }
    }

    /**
     * Check and trigger a new publish request if there is a pending request.
     */
    public void checkAndSendPendingRequest() {
        synchronized (mPendingRequestLock) {
            if (mIsDestroyed) return;
            if (mProcessorState.hasPendingRequest()) {
                // Retrieve the trigger type of the pending request
@@ -388,6 +396,17 @@ public class PublishProcessor {
                mPublishCtrlCallback.requestPublishFromInternal(type);
            }
        }
    }

    /**
     * Clear the pending request. It means that the publish request is triggered and this flag can
     * be removed.
     */
    private void clearPendingRequest() {
        synchronized (mPendingRequestLock) {
            mProcessorState.clearPendingRequest();
        }
    }

    /**
     * Update the publishing allowed time with the given trigger type. This method wil be called
@@ -413,6 +432,13 @@ public class PublishProcessor {
        mProcessorState.updatePublishThrottle(publishThrottle);
    }

    /**
     * @return true if the publish request is running now.
     */
    public boolean isPublishingNow() {
        return mProcessorState.isPublishingNow();
    }

    @VisibleForTesting
    public void setProcessorState(PublishProcessorState processorState) {
        mProcessorState = processorState;
+18 −11
Original line number Diff line number Diff line
@@ -19,13 +19,11 @@ package com.android.ims.rcs.uce.presence.publish;
import static com.android.ims.rcs.uce.presence.publish.PublishController.PUBLISH_TRIGGER_RETRY;
import static com.android.ims.rcs.uce.presence.publish.PublishController.PUBLISH_TRIGGER_VT_SETTING_CHANGE;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

import android.os.Handler;
@@ -45,6 +43,7 @@ import com.android.ims.rcs.uce.presence.publish.PublishControllerImpl.DeviceCapL
import com.android.ims.rcs.uce.presence.publish.PublishControllerImpl.PublishProcessorFactory;
import com.android.ims.ImsTestBase;

import java.time.Instant;
import java.util.Optional;

import org.junit.After;
@@ -83,9 +82,11 @@ public class PublishControllerImplTest extends ImsTestBase {
    @Test
    @SmallTest
    public void testRcsConnected() throws Exception {
        PublishController publishController = createPublishController();
        PublishControllerImpl publishController = createPublishController();

        publishController.onRcsConnected(mFeatureManager);
        Handler handler = publishController.getPublishHandler();
        waitForHandlerAction(handler, 1000);

        verify(mPublishProcessor).onRcsConnected(mFeatureManager);
    }
@@ -93,9 +94,11 @@ public class PublishControllerImplTest extends ImsTestBase {
    @Test
    @SmallTest
    public void testRcsDisconnected() throws Exception {
        PublishController publishController = createPublishController();
        PublishControllerImpl publishController = createPublishController();

        publishController.onRcsDisconnected();
        Handler handler = publishController.getPublishHandler();
        waitForHandlerAction(handler, 1000);

        verify(mPublishProcessor).onRcsDisconnected();
    }
@@ -103,26 +106,28 @@ public class PublishControllerImplTest extends ImsTestBase {
    @Test
    @SmallTest
    public void testDestroyed() throws Exception {
        PublishController publishController = createPublishController();
        PublishControllerImpl publishController = createPublishController();

        publishController.onDestroy();

        verify(mPublishProcessor).onDestroy();
        verify(mDeviceCapListener).onDestroy();
        verify(mPublishProcessor, never()).doPublish(anyInt());
    }

    @Test
    @SmallTest
    public void testGetPublishState() throws Exception {
        PublishController publishController = createPublishController();
        PublishControllerImpl publishController = createPublishController();

        int initState = publishController.getUcePublishState();
        assertEquals(RcsUceAdapter.PUBLISH_STATE_NOT_PUBLISHED, initState);

        publishController.onDestroy();
        publishController.getPublishControllerCallback().updatePublishRequestResult(
                RcsUceAdapter.PUBLISH_STATE_OK, Instant.now(), null);
        Handler handler = publishController.getPublishHandler();
        waitForHandlerAction(handler, 1000);

        int destroyedState = publishController.getUcePublishState();
        assertEquals(RcsUceAdapter.PUBLISH_STATE_OTHER_ERROR, destroyedState);
        int latestState = publishController.getUcePublishState();
        assertEquals(RcsUceAdapter.PUBLISH_STATE_OK, latestState);
    }

    @Test
@@ -173,6 +178,8 @@ public class PublishControllerImplTest extends ImsTestBase {

        IImsCapabilityCallback callback = publishController.getRcsCapabilitiesCallback();
        callback.onCapabilitiesStatusChanged(RcsUceAdapter.CAPABILITY_TYPE_PRESENCE_UCE);
        waitForHandlerAction(handler, 1000);

        verify(mPublishProcessor).checkAndSendPendingRequest();
    }

+0 −12
Original line number Diff line number Diff line
@@ -106,18 +106,6 @@ public class PublishProcessorTest extends ImsTestBase {
        verify(mRcsFeatureManager, never()).requestPublication(any(), any());
    }

    @Test
    @SmallTest
    public void testNotPublishWhenAlreadyPublishing() throws Exception {
        doReturn(true).when(mProcessorState).isPublishingNow();
        PublishProcessor publishProcessor = getPublishProcessor();

        publishProcessor.doPublish(PublishController.PUBLISH_TRIGGER_RETRY);

        verify(mProcessorState).setPendingRequest(PublishController.PUBLISH_TRIGGER_RETRY);
        verify(mRcsFeatureManager, never()).requestPublication(any(), any());
    }

    @Test
    @SmallTest
    public void testNotPublishWhenReachMaximumRetries() throws Exception {