OICMSG interfaces, abstract class, enums #1

lyzhang27 · 2018-04-10T23:25:03Z

No description provided.

manu-sinha · 2018-04-16T15:34:22Z

lib/src/main/java/com/auth0/msg/AbstractMessage.java

+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+


please add class level javadoc

manu-sinha · 2018-04-16T15:35:04Z

lib/src/main/java/com/auth0/msg/AbstractMessage.java

@@ -0,0 +1,180 @@
+package com.auth0.msg;


I am going to check with Lee what package name we should use.. I don't think we will use com.auth0

manu-sinha · 2018-04-16T15:37:56Z

lib/src/main/java/com/auth0/msg/AbstractMessage.java

+     * @param urlEncoded the urlEncoded String representation of a message
+     * @return a map of the key value pairs encoded in the string parameter
+     */
+    private static Map<String, Object> claimsFromUrlEncoded(String urlEncoded) throws Exception {


Having a private static method in an abstract class doesn't make sense. No other class will see this method at all. If it is an utility method - create a new class MessageUtil and there you add it as a public static method.

manu-sinha · 2018-04-16T21:27:49Z

lib/src/main/java/com/auth0/msg/AbstractMessage.java

+     */
+    public String toUrlEncoded() {
+        // Serialize the content of this instance (the claims map) into an UrlEncoded string
+        return "";


Please add TODOs - so that we don't we don't miss out anything.

manu-sinha · 2018-04-16T21:29:20Z

lib/src/main/java/com/auth0/msg/AbstractMessage.java

+    }
+
+    /**
+     * Logic to extract from the string the values


Can you please update the javadoc to mention json string?

manu-sinha · 2018-04-16T21:31:36Z

lib/src/main/java/com/auth0/msg/AbstractMessage.java

+     *
+     * @return a JSON String representation in the form of a hashMap mapping string -> string
+     */
+    public String toJson() {


All the 'to' methods toJson, toJwt and toUrlEncoded, show have throws SerializationException so that caller will know that it can happen and can handle it.

manu-sinha · 2018-04-16T21:35:15Z

lib/src/main/java/com/auth0/msg/AbstractMessage.java

+    /**
+     * @param input the jwt String representation of a message
+     * @param Key   that might contain the necessary key
+     * @return a ResponseMessage representation of the JWT


please update ResponseMessage to Message everywhere in this class

manu-sinha · 2018-04-16T21:37:34Z

lib/src/main/java/com/auth0/msg/AlgorithmEnum.java

@@ -0,0 +1,11 @@
+package com.auth0.msg;
+


Add javadoc. Also Justin and you should share the enum Algorithm enum.

@jdahmubed - please use just one enum for algorithms. At this time, you an Leo have two different copies.
@lccodes FYI

@jdahmubed if you can import and use my AlgorithmEnum we should just go with that

i dont agree w including "Enum" in the enum name. mine leaves it out.

manu-sinha · 2018-04-16T21:40:06Z

lib/src/main/java/com/auth0/msg/ClaimType.java

+
+import java.util.Arrays;
+import java.util.List;
+


Please add Javadocs. As we discussed previously - we shoould look at how we will use this enum to capture the type of the claim. I am talking about Claim type that you have mentioned here https://docs.google.com/document/d/1-N0n7UopFaIhzA5X-j1fhBgAR-kImbKoqgSVTUmixEI/edit?ts=5aac5b78#bookmark=id.u4q83iah8gx9

If we need to validate the type, we should have type information in the enum itself.

manu-sinha · 2018-04-16T21:42:33Z

lib/src/main/java/com/auth0/msg/DataLocation.java

@@ -0,0 +1,5 @@
+package com.auth0.msg;
+


Add javadoc.
@jdahmubed I think you will be reusing this enum.

manu-sinha · 2018-04-16T21:44:24Z

lib/src/main/java/com/auth0/msg/DataLocation.java

+package com.auth0.msg;
+
+public enum DataLocation {
+    FRAGMENT, QUERYPART


please change QUERYPART to QUERY_STRING (more explicit). Also add one more value FORM_POST

manu-sinha · 2018-04-16T21:44:40Z

lib/src/main/java/com/auth0/msg/InvalidClaimsException.java

@@ -0,0 +1,11 @@
+package com.auth0.msg;
+


Add Javadoc

manu-sinha · 2018-04-16T21:46:02Z

lib/src/main/java/com/auth0/msg/Key.java

@@ -0,0 +1,4 @@
+package com.auth0.msg;
+
+public class Key {


Please add class level Javadoc
Aren't you going to extend from auth0 class?

manu-sinha · 2018-04-16T21:46:31Z

lib/src/main/java/com/auth0/msg/KeyJar.java

@@ -0,0 +1,4 @@
+package com.auth0.msg;
+


Please add class level Javadoc
Aren't you going to extend from auth0 class? If so, please add extends

manu-sinha · 2018-04-16T21:47:45Z

lib/src/main/java/com/auth0/msg/Message.java

+/**
+ * This interface all the methods related to message processing.
+ */
+public interface Message {


Please add throws SerializationException to all the to methods. I have explained the reason in the abstract class.

manu-sinha · 2018-04-16T21:48:25Z

lib/src/main/java/com/auth0/msg/SerializationException.java

@@ -0,0 +1,11 @@
+package com.auth0.msg;
+


Add javadoc

manu-sinha

Add Javadocs
Update interface and abstract class so that 'to' methods 'throws exeption'
Common enum for Algorithm and DataLocation for you and Justin
Create a util class MessageUtil for the static method.

OICMSG interfaces, abstract class, enums

70ec60f

lyzhang27 requested review from manu-sinha and lccodes and removed request for manu-sinha April 11, 2018 00:10

manu-sinha requested changes Apr 16, 2018

View reviewed changes

manu-sinha reviewed Apr 16, 2018

View reviewed changes

manu-sinha requested changes Apr 16, 2018

View reviewed changes

Yexuan Leo Zhang added 2 commits April 17, 2018 08:59

Respond to comments from PR review

dc83e42

Dependencies for OICService

7c7bc11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OICMSG interfaces, abstract class, enums #1

OICMSG interfaces, abstract class, enums #1

lyzhang27 commented Apr 10, 2018

manu-sinha Apr 16, 2018

lyzhang27 Apr 16, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018 •

edited

Loading

lyzhang27 Apr 16, 2018

manu-sinha Apr 16, 2018

lyzhang27 Apr 16, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018 •

edited

Loading

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018

lyzhang27 Apr 17, 2018

jdahmubed Apr 17, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018

manu-sinha Apr 16, 2018 •

edited

Loading

manu-sinha Apr 16, 2018

manu-sinha left a comment

OICMSG interfaces, abstract class, enums #1

Are you sure you want to change the base?

OICMSG interfaces, abstract class, enums #1

Conversation

lyzhang27 commented Apr 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manu-sinha Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manu-sinha Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manu-sinha Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manu-sinha left a comment

Choose a reason for hiding this comment

manu-sinha Apr 16, 2018 •

edited

Loading

manu-sinha Apr 16, 2018 •

edited

Loading

manu-sinha Apr 16, 2018 •

edited

Loading