From a79a043146068f3b44d0ecd9baa0098dff8e7622 Mon Sep 17 00:00:00 2001 From: nalcalag Date: Mon, 2 Jun 2025 12:54:39 +0100 Subject: [PATCH 1/3] Refactor getBillingPeriodInDays to support all values --- .../subscriptions/impl/SubscriptionsManager.kt | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt b/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt index eabfbc268b3e..577829ca732d 100644 --- a/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt +++ b/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt @@ -1105,13 +1105,21 @@ data class PricingPhase( val billingPeriod: String, ) { - internal fun getBillingPeriodInDays(): Int? = - when (billingPeriod) { - "P1W" -> 7 - "P1M" -> 30 - "P1Y" -> 365 + internal fun getBillingPeriodInDays(): Int? { + val regex = Regex("""P(\d+)([DWMY])""") + val match = regex.matchEntire(billingPeriod) ?: return null + + val (amountStr, unit) = match.destructured + val amount = amountStr.toIntOrNull() ?: return null + + return when (unit) { + "D" -> amount + "W" -> amount * 7 + "M" -> amount * 30 + "Y" -> amount * 365 else -> null } + } } data class ValidatedTokenPair( From fd5b777af63afa0b802a858b765968e953bc0e46 Mon Sep 17 00:00:00 2001 From: nalcalag Date: Mon, 2 Jun 2025 12:55:03 +0100 Subject: [PATCH 2/3] Added tests --- .../impl/billing/PricingPhaseTest.kt | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt diff --git a/subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt b/subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt new file mode 100644 index 000000000000..366460611fcb --- /dev/null +++ b/subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2025 DuckDuckGo + * + * 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.duckduckgo.subscriptions.impl.billing + +import com.duckduckgo.subscriptions.impl.PricingPhase +import junit.framework.TestCase.assertEquals +import junit.framework.TestCase.assertNull +import org.junit.Test + +class PricingPhaseTest { + @Test + fun returnsCorrectDaysForDays() { + val phase = PricingPhase("Free", "P10D") + assertEquals(10, phase.getBillingPeriodInDays()) + } + + @Test + fun returnsCorrectDaysForWeeks() { + val phase = PricingPhase("Free", "P2W") + assertEquals(14, phase.getBillingPeriodInDays()) + } + + @Test + fun returnsCorrectDaysForMonths() { + val phase = PricingPhase("Free", "P3M") + assertEquals(90, phase.getBillingPeriodInDays()) + } + + @Test + fun returnsCorrectDaysForYears() { + val phase = PricingPhase("Free", "P1Y") + assertEquals(365, phase.getBillingPeriodInDays()) + } + + @Test + fun returnsNullForInvalidFormat() { + val phase = PricingPhase("Free", "XYZ") + assertNull(phase.getBillingPeriodInDays()) + } + + @Test + fun returnsNullForMissingNumber() { + val phase = PricingPhase("Free", "PM") + assertNull(phase.getBillingPeriodInDays()) + } + + @Test + fun returnsNullForEmptyString() { + val phase = PricingPhase("Free", "") + assertNull(phase.getBillingPeriodInDays()) + } + + @Test + fun returnsNullForMixedPeriods() { + val phase = PricingPhase("Free", "P1M2W") + assertNull(phase.getBillingPeriodInDays()) // Not supported in simplified version + } +} From 19316c8349d26450fe17dce0369ae1a54d5d7e61 Mon Sep 17 00:00:00 2001 From: nalcalag Date: Mon, 2 Jun 2025 15:31:52 +0100 Subject: [PATCH 3/3] Changes after review --- .../impl/SubscriptionsManager.kt | 21 +++++----- .../impl/billing/PricingPhaseTest.kt | 39 ++++++++----------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt b/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt index 577829ca732d..85e6ab0cfe0d 100644 --- a/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt +++ b/subscriptions/subscriptions-impl/src/main/java/com/duckduckgo/subscriptions/impl/SubscriptionsManager.kt @@ -81,6 +81,8 @@ import dagger.SingleInstanceIn import java.io.IOException import java.time.Duration import java.time.Instant +import java.time.Period +import java.time.format.DateTimeParseException import javax.inject.Inject import kotlin.time.Duration.Companion.milliseconds import kotlinx.coroutines.CoroutineScope @@ -95,6 +97,7 @@ import kotlinx.coroutines.flow.onSubscription import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import logcat.LogPriority.ERROR +import logcat.asLog import logcat.logcat import retrofit2.HttpException @@ -1106,18 +1109,12 @@ data class PricingPhase( ) { internal fun getBillingPeriodInDays(): Int? { - val regex = Regex("""P(\d+)([DWMY])""") - val match = regex.matchEntire(billingPeriod) ?: return null - - val (amountStr, unit) = match.destructured - val amount = amountStr.toIntOrNull() ?: return null - - return when (unit) { - "D" -> amount - "W" -> amount * 7 - "M" -> amount * 30 - "Y" -> amount * 365 - else -> null + return try { + val period = Period.parse(billingPeriod) + return period.days + period.months * 30 + period.years * 365 + } catch (e: DateTimeParseException) { + logcat { "Subs: Failed to parse billing period \"$billingPeriod\": ${e.asLog()}" } + null } } } diff --git a/subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt b/subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt index 366460611fcb..0d8a98e16721 100644 --- a/subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt +++ b/subscriptions/subscriptions-impl/src/test/java/com/duckduckgo/subscriptions/impl/billing/PricingPhaseTest.kt @@ -23,50 +23,45 @@ import org.junit.Test class PricingPhaseTest { @Test - fun returnsCorrectDaysForDays() { - val phase = PricingPhase("Free", "P10D") + fun billingPeriodParsesDaysCorrectly() { + val phase = PricingPhase(formattedPrice = "Free", billingPeriod = "P10D") assertEquals(10, phase.getBillingPeriodInDays()) } @Test - fun returnsCorrectDaysForWeeks() { - val phase = PricingPhase("Free", "P2W") + fun billingPeriodParsesWeeksCorrectly() { + val phase = PricingPhase(formattedPrice = "Free", billingPeriod = "P2W") assertEquals(14, phase.getBillingPeriodInDays()) } @Test - fun returnsCorrectDaysForMonths() { - val phase = PricingPhase("Free", "P3M") - assertEquals(90, phase.getBillingPeriodInDays()) + fun billingPeriodParsesMonthsCorrectly() { + val phase = PricingPhase(formattedPrice = "Free", billingPeriod = "P2M") + assertEquals(60, phase.getBillingPeriodInDays()) } @Test - fun returnsCorrectDaysForYears() { - val phase = PricingPhase("Free", "P1Y") + fun billingPeriodParsesYearsCorrectly() { + val phase = PricingPhase(formattedPrice = "Free", billingPeriod = "P1Y") assertEquals(365, phase.getBillingPeriodInDays()) } @Test - fun returnsNullForInvalidFormat() { - val phase = PricingPhase("Free", "XYZ") - assertNull(phase.getBillingPeriodInDays()) + fun billingPeriodParsesMixedPeriodCorrectly() { + val phase = PricingPhase(formattedPrice = "Free", billingPeriod = "P1Y2M10D") + val expectedDays = 1 * 365 + 2 * 30 + 10 // 365 + 60 + 10 = 435 + assertEquals(expectedDays, phase.getBillingPeriodInDays()) } @Test - fun returnsNullForMissingNumber() { - val phase = PricingPhase("Free", "PM") + fun billingPeriodEmptyReturnsNull() { + val phase = PricingPhase(formattedPrice = "$0", billingPeriod = "") assertNull(phase.getBillingPeriodInDays()) } @Test - fun returnsNullForEmptyString() { - val phase = PricingPhase("Free", "") + fun billingPeriodReturnsNullForInvalidFormat() { + val phase = PricingPhase(formattedPrice = "Free", billingPeriod = "INVALID") assertNull(phase.getBillingPeriodInDays()) } - - @Test - fun returnsNullForMixedPeriods() { - val phase = PricingPhase("Free", "P1M2W") - assertNull(phase.getBillingPeriodInDays()) // Not supported in simplified version - } }