adding interfaces for public api by SomkaPe · Pull Request #39 · AzureAD/microsoft-authentication-library-for-java

SomkaPe · 2019-05-16T22:10:50Z

No description provided.

sangonzal

Discussed other questions offline.

sangonzal · 2019-05-17T18:18:22Z

src/integrationtest/java/com.microsoft.aad.msal4j/ClientCredentialsIT.java

                build();

-        AuthenticationResult result = cca.acquireToken(ClientCredentialParameters
+        AuthenticationResult result = (AuthenticationResult)cca.acquireToken(ClientCredentialParameters


IAuthenticationResult instead of casting for consistency ?

will change

sangonzal · 2019-05-17T18:19:05Z

src/integrationtest/java/com.microsoft.aad.msal4j/RefreshTokenIT.java

                build();

-        AuthenticationResult result = pca.acquireToken(UserNamePasswordParameters
+        AuthenticationResult result = (AuthenticationResult)pca.acquireToken(UserNamePasswordParameters


Same as above

henrik-me · 2019-05-17T18:42:22Z

src/integrationtest/java/com.microsoft.aad.msal4j/NationalCloudIT.java


        Assert.assertNotNull(result);
        Assert.assertNotNull(result.accessToken());
-        Assert.assertNotNull(result.refreshToken());


Assert.assertNotNull(result.refreshToken()); [](start = 8, length = 44)

Assuming for ROPC where we don't get the RT, however what changed other places made the RT be null now?

we do not expose RT through IAuthenticationResult

henrik-me · 2019-05-17T18:44:46Z

src/integrationtest/java/com.microsoft.aad.msal4j/RefreshTokenIT.java


        Assert.assertNotNull(result);
        Assert.assertNotNull(result.accessToken());
-        Assert.assertNotNull(result.refreshToken());


Assert.assertNotNull(result.refreshToken()); [](start = 8, length = 44)

Why?

we do not expose RT through IAuthenticationResult

henrik-me · 2019-05-17T18:47:53Z

src/main/java/com/microsoft/aad/msal4j/Account.java

-                account.clientInfo().toAccountIdentifier());
-        account.environment(environment);
+@AllArgsConstructor
+public class Account implements IAccount {


Account [](start = 13, length = 7)

what happened to the other fields on account? not sure I understand why those are only for the cache entity

This change was to separate Account cache entity from IAccount - part of public api

henrik-me · 2019-05-17T18:48:46Z

src/main/java/com/microsoft/aad/msal4j/AccountCacheEntity.java

+@Getter
+@Setter
+@EqualsAndHashCode
+class AccountCacheEntity {


should this implement IAccount ?

This change was to separate Account cache entity from IAccount - part of public api

henrik-me · 2019-05-17T18:51:09Z

a number of field/class comments has been removed. How are the api docs created?
Will the comments automatically be available on the classes based on the comments in the interface?

henrik-me · 2019-05-17T18:54:11Z

src/main/java/com/microsoft/aad/msal4j/ITokenCache.java

+package com.microsoft.aad.msal4j;
+
+public interface ITokenCache {
+    void deserializeAndLoadToCache(String data);


perhaps just deserialize or load? And add a comment to clarify what actually happens?

may be ... ok will rename

henrik-me · 2019-05-17T18:54:19Z

src/main/java/com/microsoft/aad/msal4j/ITokenCache.java

+public interface ITokenCache {
+    void deserializeAndLoadToCache(String data);
+
+    String serialize();


henrik-me · 2019-05-17T18:54:39Z

src/main/java/com/microsoft/aad/msal4j/ITokenCacheAccessAspect.java

@@ -25,8 +25,8 @@

 public interface ITokenCacheAccessAspect {


henrik-me · 2019-05-17T18:55:12Z

src/main/java/com/microsoft/aad/msal4j/ITokenCacheAccessContext.java

+
+    IAccount getAccount();
+
+    boolean isCacheChanged();


is [](start = 12, length = 2)

nit: understand why you call this is ... perhaps has?

ok, will rename

henrik-me · 2019-05-17T18:55:28Z

src/main/java/com/microsoft/aad/msal4j/ITokenCacheAccessContext.java

+
+    IAccount getAccount();
+
+    boolean isCacheChanged();


henrik-me · 2019-05-17T18:57:05Z

src/test/java/com/microsoft/aad/msal4j/TestConfiguration.java


    public static String INSTANCE_DISCOVERY_RESPONSE = "{" +
-            "\"tenant_discovery_endpoint\":\"https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-configuration\"," +
+            "\"tenant_discovery_endpoint\":\"https://login.microsoftonline.com/organizations/v2.0/.well-known/openid-appConfiguration\"," +


app [](start = 117, length = 3)

why adding app?

will fix - side effect of refactoring with IDE

sangonzal · 2019-05-20T20:05:11Z

src/main/java/com/microsoft/aad/msal4j/IAccount.java

+
+public interface IAccount {
+
+    String homeAccountId();


Consider adding comments to public interfaces

sangonzal · 2019-05-20T20:05:18Z

src/main/java/com/microsoft/aad/msal4j/IAuthenticationResult.java

+/**
+ * Represents the results of token acquisition operation.
+ */
+public interface IAuthenticationResult {


Consider adding comments to public interfaces

SomkaPe · 2019-05-23T01:59:19Z

a number of field/class comments has been removed. How are the api docs created?
Will the comments automatically be available on the classes based on the comments in the interface?

@henrik-me Yes, comments are inherited from interfaces

adding interfaces for public api

e5641a9

SomkaPe requested review from henrik-me and sangonzal May 16, 2019 22:10

sangonzal approved these changes May 17, 2019

View reviewed changes

henrik-me reviewed May 17, 2019

View reviewed changes

sangonzal reviewed May 20, 2019

View reviewed changes

SomkaPe added 4 commits May 21, 2019 18:26

delombok before java doc generation, fixing javadoc warnings

ea4149a

minor change

05a8357

downgrading lombok plugin to make maven 3.2.2 happy

c90f723

pr comments, new java doc

a25ec2d

SomkaPe merged commit bcf5d5d into dev May 23, 2019

SomkaPe deleted the somkape/apiUpdate branch June 6, 2019 01:42

Conversation

SomkaPe commented May 16, 2019

Uh oh!

sangonzal 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henrik-me May 17, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

henrik-me May 17, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

henrik-me commented May 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

SomkaPe commented May 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

henrik-me May 17, 2019 •

edited

Loading

henrik-me May 17, 2019 •

edited

Loading

henrik-me commented May 17, 2019 •

edited

Loading

SomkaPe commented May 23, 2019 •

edited

Loading