#100 moved pinnedKey to ApiEnvironmentType #101

tubbynl · 2018-06-15T15:17:18Z

moved the pinnedKeys to ApiEnvironmentType

OGKevin · 2018-06-17T06:38:07Z

@tubbynl please rebase your commits to follow the following pattern.

sdk_java/.zappr.yaml

Line 7 in 7ed15ad

- '([A-Za-z0-9 ]+)\. (\(bunq\/sdk_java#[0-9]+\))'

OGKevin · 2018-06-18T19:31:06Z

src/main/java/com/bunq/sdk/context/ApiEnvironmentType.java

@@ -5,18 +5,20 @@
 */
 public enum ApiEnvironmentType {

-  PRODUCTION("api.bunq.com", "v1"),
-  SANDBOX("public-api.sandbox.bunq.com", "v1");
+  PRODUCTION("api.bunq.com", "v1","sha256/nI/T/sDfioCBHB5mVppDPyLi2HXYanwk2arpZuHLOu0="),


missing space around ,

OGKevin · 2018-06-18T19:31:09Z

src/main/java/com/bunq/sdk/context/ApiEnvironmentType.java

-  PRODUCTION("api.bunq.com", "v1"),
-  SANDBOX("public-api.sandbox.bunq.com", "v1");
+  PRODUCTION("api.bunq.com", "v1","sha256/nI/T/sDfioCBHB5mVppDPyLi2HXYanwk2arpZuHLOu0="),
+  SANDBOX("public-api.sandbox.bunq.com", "v1","sha256/GhNvDokiMyXzhGft+xXWFGchUmmh8R5dQEnO4xu81NY=");


OGKevin · 2018-06-18T19:31:32Z

src/main/java/com/bunq/sdk/context/ApiEnvironmentType.java

@@ -30,4 +32,7 @@ public String getApiVersion() {
    return this.apiVersion;
  }

+  public String getPinnedKey() {
+    return pinnedKey;
+  }
 }


could you please the missing new line at EOF here 🙊

OGKevin · 2018-06-18T19:32:16Z

src/main/java/com/bunq/sdk/http/ApiClient.java

-            String.format(ERROR_AMI_ENVIRONMENT_NOT_EXPECTED, apiContext.getEnvironmentType().toString())
-        );
+  private static CertificatePinner determineCertificateToPin(ApiEnvironmentType environmentType) {
+    if(environmentType!=null && environmentType.getPinnedKey()!=null) {


missing space around operators.

OGKevin · 2018-06-18T19:32:56Z

src/main/java/com/bunq/sdk/http/ApiClient.java

+      ).build();
+    } else {
+      throw new BunqException(
+              String.format(ERROR_AMI_ENVIRONMENT_NOT_EXPECTED, Objects.toString(environmentType,"<null>"))


missing space after comma.

OGKevin · 2018-06-19T06:57:34Z

@tubbynl you can ignore the 80 line thing, the bot has been reconfigured and on your next push it should check for length of 100. Sorry for the inconvenience.

OGKevin

This one is almost there!

OGKevin · 2018-06-19T06:58:23Z

src/main/java/com/bunq/sdk/context/ApiEnvironmentType.java

+    return pinnedKey;
+  }
+}


Missing new line at end of class block.

OGKevin · 2018-06-19T06:59:06Z

src/main/java/com/bunq/sdk/http/ApiClient.java

+    } else {
+      throw new BunqException(
+              String.format(ERROR_AMI_ENVIRONMENT_NOT_EXPECTED, 
+				  Objects.toString(environmentType, "<null>"))


This formatting looks weird. It should be on the same level as String.format(ERROR_AMI_ENVIRONMENT_NOT_EXPECTED,

it then will be 109 chars :/

The same indent level as. My apologies.

OGKevin · 2018-06-19T06:59:41Z

src/main/java/com/bunq/sdk/http/ApiClient.java

+      throw new BunqException(
+              String.format(ERROR_AMI_ENVIRONMENT_NOT_EXPECTED, 
+				  Objects.toString(environmentType, "<null>"))
+      );
    }
  }


Could you be kind enough to add the missing new line at end of the class block here as well 👼

tubbynl · 2018-06-19T17:13:10Z

fails at
com.bunq.sdk.model.generated.endpoint.CardDebitTest > orderNewMaestroCardTest FAILED com.bunq.sdk.exception.PleaseContactBunqException at CardDebitTest.java:80

but that probably also fails on a build on the develop branch as-is

tubbynl · 2018-06-20T19:23:19Z

see #100 close as-you-wish

OGKevin · 2018-06-22T07:52:09Z

@patrickdw1991 please 👁 the failing test is offline HSM.

patrickdw1991 · 2018-07-09T19:21:28Z

src/main/java/com/bunq/sdk/context/ApiContext.java

@@ -228,7 +225,7 @@ public boolean ensureSessionActive() {

  public boolean isSessionActive() {
    return sessionContext != null &&
-      getTimeToSessionExpiryInSeconds() < TIME_TO_SESSION_EXPIRY_MINIMUM_SECONDS;
+      getTimeToSessionExpiryInSeconds() > TIME_TO_SESSION_EXPIRY_MINIMUM_SECONDS;


@OGKevin Just to be sure are you sure that this won't create any issues?

according to my testing no, It was recreating a session on every call 😅 now it seems to behave as expected. Pipeline should not fail with 429 errors on each test anymore.

patrickdw1991 · 2018-07-09T19:22:06Z

src/test/java/com/bunq/sdk/http/PaginationScenarioTest.java

@@ -42,9 +39,14 @@
  private static Gson gson = BunqGsonBuilder.buildDefault().create();

  private static void EnsureEnoughPayments() {
-    for (int i = NUMBER_ZERO; i < GetPaymentsMissingCount(); ++i) {
-      CreatePayment();
+    int paymentCount = GetPaymentsMissingCount();


missingPaymentCount?

patrickdw1991 · 2018-07-09T19:22:48Z

src/test/java/com/bunq/sdk/http/PaginationScenarioTest.java

-  private static void CreatePayment() {
-    Payment.create(new Amount(PAYMENT_AMOUNT_EUR, PAYMENT_CURRENCY), getPointerBravo(), PAYMENT_DESCRIPTION);
+  private static Payment createPayment() {
+    return  new Payment(


Two spaces 😱

OGKevin · 2018-07-10T06:57:50Z

@patrickdw1991 pushed.

OGKevin · 2018-07-10T09:41:42Z

@andrederoos

OGKevin self-requested a review June 17, 2018 06:38

tubbynl force-pushed the develop-100-ApiEnvironmentType branch from c55c1f6 to bff80d6 Compare June 17, 2018 14:59

OGKevin suggested changes Jun 18, 2018

View reviewed changes

OGKevin suggested changes Jun 19, 2018

View reviewed changes

bunq deleted a comment Jun 19, 2018

tubbynl added 4 commits June 20, 2018 09:36

ApiEnvironmentType#pinnedKey (bunq#100)

568d2ae

code formatting feedback (bunq#100)

4a81bf4

code formatting feedback (bunq#100)

d45f25d

code formatting feedback (bunq#100)

b1c11ac

tubbynl force-pushed the develop-100-ApiEnvironmentType branch from 795a7f1 to b1c11ac Compare June 20, 2018 07:37

bunq deleted a comment Jun 20, 2018

Formatting refactor. (bunq#100)

bb08d69

bunq deleted a comment Jun 22, 2018

Kevin Hellemun added 3 commits June 22, 2018 09:24

Fixed broken isSessionActive method. (bunq#100)

4d4351e

Fixed failing pagination test. (bunq#100)

b411f1f

Shortened long lines. (bunq#100)

9d52bc5

OGKevin added this to the 1.0.0 milestone Jun 22, 2018

OGKevin added the Can be merged label Jun 22, 2018

Merge branch 'develop' into develop-100-ApiEnvironmentType

d4892b0

bunq deleted a comment Jun 22, 2018

Kevin Hellemun added 2 commits June 22, 2018 09:48

Fixed method that starts with capital letter. (bunq#100)

6b544a0

Fixed failing session test. (bunq#100)

5e59165

bunq deleted a comment Jun 22, 2018

OGKevin requested a review from patrickdw1991 June 22, 2018 07:52

OGKevin previously approved these changes Jun 22, 2018

View reviewed changes

patrickdw1991 requested changes Jul 9, 2018

View reviewed changes

Renamed var name and fixed formatting issues. (bunq#100)

71ed9b7

OGKevin dismissed their stale review via 71ed9b7 July 10, 2018 06:57

bunq deleted a comment Jul 10, 2018

patrickdw1991 approved these changes Jul 10, 2018

View reviewed changes

OGKevin merged commit c0d16de into bunq:develop Jul 10, 2018

#100 moved pinnedKey to ApiEnvironmentType #101

#100 moved pinnedKey to ApiEnvironmentType #101

Conversation

tubbynl commented Jun 15, 2018

Uh oh!

OGKevin commented Jun 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OGKevin commented Jun 19, 2018

Uh oh!

OGKevin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tubbynl Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tubbynl commented Jun 19, 2018

Uh oh!

tubbynl commented Jun 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OGKevin commented Jun 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OGKevin commented Jul 10, 2018

Uh oh!

OGKevin commented Jul 10, 2018

Uh oh!

Uh oh!

tubbynl Jun 19, 2018 •

edited

Loading

tubbynl commented Jun 20, 2018 •

edited

Loading