Skip to content

Commit

Permalink
PP-12107: Move BinRangeTest from pay-cardid-data (#1206)
Browse files Browse the repository at this point in the history
* PP-12107: Move BinRangeTest from pay-cardid-data

Moving BinRangeTest from pay-cardid-data into this repo as the build-and-push-cardid-candidate CI run will copy the real bin ranges files from s3 into src/main/resources/data-sources. Thus the test will run against real card data in a CI build.

To make the test work overlapping ranges in the current test data (in
src/main/resources/data-sources) needed to be amended/deleted.

Also removed the long comment in CardIdResourceITest as it claims the
usefulness of it's tests isn't clear. On the contrary the test is useful as it
verifies the API actually works.

Removed `-DskipTests` from the Dockerfile as we want to verify the bin ranges
don't overlap (via running BinRangesTest) whilst building the Docker image.
  • Loading branch information
oswaldquek committed May 10, 2024
1 parent 9707636 commit 8a17646
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 34 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ FROM maven:3.9.5-eclipse-temurin-11-alpine@sha256:5b8e35bd58fc6d6d78fa7faa34c0a8
WORKDIR /home/build
COPY . .

RUN ["mvn", "clean", "--no-transfer-progress", "package", "-DskipTests"]
RUN ["mvn", "clean", "--no-transfer-progress", "package"]

FROM eclipse-temurin:11-jre-alpine@sha256:77899ea444d0c219f3a4f500923dcae6b4b28db239b80c5214f26c1167ba74b5 AS final

Expand Down
3 changes: 1 addition & 2 deletions src/main/resources/data-sources/discover.csv
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
01,START,END,TYPE,BRAND
02,71221111,71221400,CD,DISCOVER
02,71231111,71241400,CD,DISCOVER
02,71231111,71241100,CD,DISCOVER
02,71241111,71251400,CD,DISCOVER
02,60110009,60110010,CD,DISCOVER
09
7 changes: 0 additions & 7 deletions src/main/resources/data-sources/test-cards.csv
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
01,START,END,TYPE,BRAND,PREPAID,CORPORATE
02,12200000000,12200000000,C,AIRPLUS,NOT_PREPAID,NOT_CORPORATE
02,22210000000,22210000000,C,MC,NOT_PREPAID,NOT_CORPORATE
02,40000000000,40000000000,CD,VISA,NOT_PREPAID,NOT_CORPORATE
02,40000025000,40000025000,C,VISA CREDIT,NOT_PREPAID,NOT_CORPORATE
Expand All @@ -13,17 +12,11 @@
02,41111166666,41111166666,C,VISA CREDIT,NOT_PREPAID,NOT_CORPORATE
02,42424242424,42424242424,C,VISA CREDIT,NOT_PREPAID,NOT_CORPORATE
02,44443333222,44443333222,C,VISA CREDIT,NOT_PREPAID,NOT_CORPORATE
02,49118300000,49118300000,C,VISA CREDIT,NOT_PREPAID,NOT_CORPORATE
02,49173008000,49173008000,D,VISA DEBIT,NOT_PREPAID,NOT_CORPORATE
02,49176100000,49176100000,C,VISA CREDIT,NOT_PREPAID,NOT_CORPORATE
02,50197170101,50197170101,C,DANKORT,NOT_PREPAID,NOT_CORPORATE
02,51051051051,51051051051,D,MC,NOT_PREPAID,NOT_CORPORATE
02,52008282828,52008282828,D,MC,NOT_PREPAID,NOT_CORPORATE
02,54545454545,54545454545,C,MC,NOT_PREPAID,NOT_CORPORATE
02,55555555555,55555555555,C,MC,NOT_PREPAID,NOT_CORPORATE
02,63049001774,63049001774,C,LASER,NOT_PREPAID,NOT_CORPORATE
02,63049506000,63049506000,C,LASER,NOT_PREPAID,NOT_CORPORATE
02,67596498264,67596498264,D,MAESTRO,NOT_PREPAID,NOT_CORPORATE
02,67999901000,67999901000,D,MAESTRO,NOT_PREPAID,NOT_CORPORATE
02,71221111000,71221400000,CD,DISCOVER,PREPAID,CORPORATE
02,37144963500,37144963500,C,AMEX,NOT_PREPAID,NOT_CORPORATE
Expand Down
6 changes: 0 additions & 6 deletions src/main/resources/data-sources/worldpay-v3.csv
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,17 @@
01,122000000000003000,122000000000003000,XX,Airplus,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,343434343434340000,343434343434340000,XX,American Express,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,555555555555444400,555555555555444400,XX,Cartebleue,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,501971701010374200,501971701010374200,XX,Dankort,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,367001020000000000,367001020000000000,XX,Diners,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,361489006479130000,361489006479130000,XX,Diners,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,601100040000000000,601100040000000000,XX,Discover card,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,352800070000000000,352800070000000000,XX,JCB,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,630495060000000000,630495060000000000,XX,Laser,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,630490017740292441,630490017740292441,XX,Laser,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,675964982643845300,675964982643845300,XX,Maestro,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,679999010000000001,679999010000000001,XX,Maestro,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,555555555555444400,555555555555444400,XX,Mastercard,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,545454545454545400,545454545454545400,XX,Mastercard,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,516361361361361300,516361361361361300,XX,Mastercard,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,444433332222111100,444433332222111100,XX,Visa,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,491183000000000000,491183000000000000,XX,Visa,UNKNOWN,XX,123,EXAMPLE COUNTRY,C,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,446203000000000000,446203000000000000,XX,Visa Debit,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,491761000000000000,491761000000000000,XX,Visa Debit,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,491730080000000000,491730080000000000,XX,Visa Electron,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,448407000000000000,448407000000000000,XX,Visa Purchasing,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,DCC allowed,ABC DEF,N,,,,,,,,,,,
01,123456789000000000,123456789000000000,XX,IT Test Example,UNKNOWN,XX,123,EXAMPLE COUNTRY,D,XXX,PREPAID,ABC DEF,N,,,,,,,,,,,
01,498808000000000000,498808999999999999,CP,VISA DEBIT,UNKNOWN,XX,356,EXAMPLE COUNTRY,D,XXX,DCC allowed,DECOM,U,,,,,,,,,,
Expand Down
134 changes: 134 additions & 0 deletions src/test/java/uk/gov/pay/card/bin_ranges/BinRangeTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package uk.gov.pay.card.bin_ranges;


import com.google.common.collect.Range;
import com.google.common.collect.RangeSet;
import com.google.common.collect.TreeRangeSet;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import uk.gov.pay.card.db.loader.BinRangeParser;

import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.net.URL;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;

import static java.lang.String.format;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.junit.jupiter.api.Assertions.fail;

class BinRangeTest {

private static final int CARD_RANGE_LENGTH = 18;
private static List<Range<Long>> worldpayData;
private static List<Range<Long>> testCardData;
private static List<Range<Long>> discoverData;

private static final Function<String[], Range<Long>> cardRangeExtractor = entry -> {
Long minCardDigit = BinRangeParser.calculateMinDigitForCardLength(Long.valueOf(entry[1]), CARD_RANGE_LENGTH);
Long maxCardDigit = BinRangeParser.calculateMaxDigitForCardLength(Long.valueOf(entry[2]), CARD_RANGE_LENGTH);
return Range.closed(minCardDigit, maxCardDigit);
};

/**
* In the cardid CI pipeline, real bin ranges files will be written to the data-sources directory
*/
@BeforeAll
static void beforeAll() {
URL worldpayDataSource = Thread.currentThread().getContextClassLoader().getResource("data-sources/worldpay-v3.csv");
URL testDataSource = Thread.currentThread().getContextClassLoader().getResource("data-sources/test-cards.csv");
URL discoverDataSource = Thread.currentThread().getContextClassLoader().getResource("data-sources/discover.csv");

worldpayData = loadData(worldpayDataSource, "01");
testCardData = loadData(testDataSource, "02");
discoverData = loadData(discoverDataSource, "02");
}

@Test
void shouldHaveNoOverlapBetweenTestAndWorldpayBinRanges() {
checkForBinRangeOverlaps("Worldpay", worldpayData, "Test Card", testCardData);
}

@Test
void shouldHaveNoOverlapBetweenTestAndDiscoverBinRanges() {
checkForBinRangeOverlaps("Discover", discoverData, "Test Card", testCardData);
}

@Test
void shouldHaveNoOverlapBetweenDiscoverAndWorldpayBinRanges() {
checkForBinRangeOverlaps("Worldpay", worldpayData, "Discover", discoverData);
}

private void checkForBinRangeOverlaps(String ranges1Type, List<Range<Long>> ranges1, String ranges2Type, List<Range<Long>> ranges2) {
RangeSet<Long> rangeSet = TreeRangeSet.create();
ranges1.forEach(rangeSet::add);

List<String> overlappingRanges = new ArrayList<>();
ranges2.forEach(cardRange -> {
if (rangeSet.intersects(cardRange)) {
overlappingRanges.add(String.format("%s range %s intersects %s ranges %s",
ranges1Type, cardRange, ranges2Type, rangeSet.subRangeSet(cardRange)));
}
});

if (!overlappingRanges.isEmpty()) {
fail("Found overlaps in ranges:\n" + String.join("\n" , overlappingRanges));
}
}

/**
We need to check there is no overlap between ranges across all data sources because some ranges are associated
with corporate card numbers, as identified by the 4th "CP" column in the example line entry from the Worldpay data source:
01,511226661120000000,511226661121000000,CP,DINERS DISCOVER,.... etc
Having no overlap ranges is essential to cardid figuring out with confidence if a card is a corporate card or not.
*/
@Test
void verifyNoOverlappingRanges() {
RangeSet<Long> setOfBinRanges = TreeRangeSet.create();
Map<Range<Long>, String> mapOfRanges = new HashMap<>();
List<Range<Long>> rangesNotInHashMap = new ArrayList<>();
worldpayData.forEach(cardRange -> {
setOfBinRanges.add(cardRange);
mapOfRanges.put(cardRange, "worldpay");
});
discoverData.forEach(cardRange -> {
setOfBinRanges.add(cardRange);
mapOfRanges.put(cardRange, "discover");
});
testCardData.forEach(cardRange -> {
setOfBinRanges.add(cardRange);
mapOfRanges.put(cardRange, "test");
});
setOfBinRanges.asRanges().forEach(range -> {
String contains = mapOfRanges.get(range);
if (contains == null) {
rangesNotInHashMap.add(range);
}
});
assertThat(rangesNotInHashMap.size(), is(0));
}

private static List<Range<Long>> loadData(URL source, String dataRowIdentifier) {
List<Range<Long>> listOfRanges = new ArrayList<>();

try (BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(source.openStream()))) {
bufferedReader.lines().forEach(line -> {
String[] entry = line.split(",");
if (dataRowIdentifier.equals(entry[0])) {
Range<Long> cardRange = cardRangeExtractor.apply(entry);
listOfRanges.add(cardRange);
}
});
return listOfRanges;
} catch (Exception e) {
throw new RuntimeException(format("Exception loading file at: %s", source == null ? "(source is null)" : source.toString()), e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,6 @@
import static org.hamcrest.core.Is.is;
import static uk.gov.pay.card.db.CardInformationStore.CARD_RANGE_LENGTH;

/**
* @deprecated The usefulness of many of these tests is unclear - move them to pay-cardid-data?
* <p>
* These tests used to make assertions about data in pay-cardid-data, but that lives in a private repo.
* <p>
* We want to be able to test pay-cardid in a CI system that doesn't have access to private repos,
* so the data needed for the tests to pass has been redacted to reduce its sensitivity and inlined
* in src/test/resources/card-id-resource-integration-test
* <p>
* The overall effect is that a lot of these tests are now just checking that "the tests do what the tests do",
* whereas they used to make some (fairly loose) assertions that the bin data in pay-cardid-data was correct.
* <p>
* If we care about testing pay-cardid-data we should set up CI for that repo separately in a system that
* has access to private repos and move the tests that use card data there.
* <p>
* A test that checks the general success case and the tests for the error conditions without relying on specific
* card details are probably still valid and should stay.
*/
@ExtendWith(DropwizardExtensionsSupport.class)
public class CardIdResourceITest {

Expand Down

0 comments on commit 8a17646

Please sign in to comment.