Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 4
Add Tests#22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base:develop
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Add Tests #22
Changes from all commits
49f1e853e0e051ccaf40f5de15a289edb0d3eeb7e0ab419d101c93904bfb604bf3f533cb0c84b6e56ebde4b2f6511090f885a50f6f89235501b98a2File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package com.toopher; | ||
| import org.apache.http.NameValuePair; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| public abstract class ApiRequestDetails { | ||
| private List<NameValuePair> params; | ||
| private Map<String, String> extras; | ||
| public ApiRequestDetails(List<NameValuePair> params, Map<String, String> extras) { | ||
| this.params = params; | ||
| this.extras = extras; | ||
| } | ||
| public List<NameValuePair> getParams() { return params; } | ||
| public Map<String, String> getExtras() { return extras; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package com.toopher; | ||
| import org.apache.http.NameValuePair; | ||
| import org.apache.http.message.BasicNameValuePair; | ||
| import java.util.ArrayList; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| public class AuthenticationRequestDetails extends ApiRequestDetails { | ||
| public AuthenticationRequestDetails(List<NameValuePair> params, Map<String, String> extras) { | ||
| super(params, extras); | ||
| } | ||
| public static class Builder { | ||
| private List<NameValuePair> params = new ArrayList<NameValuePair>(); | ||
| private Map<String, String> extras = new HashMap<String, String>(); | ||
| public Builder() { | ||
| this(null); | ||
| } | ||
| public Builder(Map<String, String> extras) { | ||
| this.extras = extras == null ? new HashMap<String, String>() : extras; | ||
| } | ||
| public Builder addExtra(String key, String value) { | ||
| extras.put(key, value); | ||
| return this; | ||
| } | ||
| public Builder setActionName(String actionName) { | ||
| if (actionName != null && actionName.length() > 0) { | ||
| params.add(new BasicNameValuePair("action_name", actionName)); | ||
| } | ||
| return this; | ||
| } | ||
| public Builder setPairingId(String pairingId) { | ||
| if (pairingId != null) { | ||
| params.add(new BasicNameValuePair("pairing_id", pairingId)); | ||
| } | ||
| return this; | ||
| } | ||
| public Builder setTerminalName(String terminalName) { | ||
| if (terminalName != null) { | ||
| params.add(new BasicNameValuePair("terminal_name", terminalName)); | ||
| } | ||
| return this; | ||
| } | ||
| public Builder setTerminalNameExtra(String terminalNameExtra) { | ||
| return addExtra("terminal_name_extra", terminalNameExtra); | ||
| } | ||
| public Builder setUserName(String userName) { | ||
| return addExtra("user_name", userName); | ||
| } | ||
| public AuthenticationRequestDetails build() { | ||
| return new AuthenticationRequestDetails(params, extras); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,7 +2,6 @@ | ||
| import org.json.JSONException; | ||
| import org.json.JSONObject; | ||
| import java.util.Map; | ||
| /** | ||
| * Provide information about the status of an authentication request | ||
| @@ -44,9 +43,8 @@ public class AuthenticationStatus extends ApiResponseObject { | ||
| * The descriptive name for the terminal associated with the request | ||
| */ | ||
| public String terminalName; | ||
| public AuthenticationStatus(JSONObject json) throws JSONException{ | ||
| public AuthenticationStatus(JSONObject json) throws JSONException { | ||
| super(json); | ||
| this.id = json.getString("id"); | ||
| @@ -58,13 +56,30 @@ public AuthenticationStatus(JSONObject json) throws JSONException{ | ||
| JSONObject terminal = json.getJSONObject("terminal"); | ||
| this.terminalId = terminal.getString("id"); | ||
| this.terminalName = terminal.getString("name"); | ||
| } | ||
| public AuthenticationStatus( | ||
| String id, | ||
| boolean pending, | ||
| boolean granted, | ||
| boolean automated, | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to make this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe, but probably not. Our API should be sending correctly-serialized data ( | ||
| String reason, | ||
| String terminalId, | ||
| String terminalName, | ||
| JSONObject json) { | ||
| super(json); | ||
| this.id = id; | ||
| this.pending = pending; | ||
| this.granted = granted; | ||
| this.automated = automated; | ||
| this.reason = reason; | ||
| this.terminalId = terminalId; | ||
| this.terminalName = terminalName; | ||
| } | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
| ||
| @Override | ||
| public String toString() { | ||
| return String.format("[AuthenticationStatus: id=%s; pending=%b; granted=%b; automated=%b; reason=%s; terminalId=%s; terminalName=%s]", | ||
| id, pending, granted, automated, reason, terminalId, terminalName); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package com.toopher; | ||
| import org.json.JSONException; | ||
| import org.json.JSONObject; | ||
| public class AuthenticationStatusFactory { | ||
| public AuthenticationStatusFactory() {} | ||
| public AuthenticationStatus create(JSONObject json) throws JSONException { | ||
| final JSONObject terminal = json.getJSONObject("terminal"); | ||
| return new AuthenticationStatus( | ||
| json.getString("id"), | ||
| json.getBoolean("pending"), | ||
| json.getBoolean("granted"), | ||
| json.getBoolean("automated"), | ||
| json.getString("reason"), | ||
| terminal.getString("id"), | ||
| terminal.getString("name"), | ||
| json); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not check for
nulland not empty for this and the following property?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is code from ToopherAPI that I refactored to use builders and factories. There wasn't a
nullcheck in the original code and I didn't want to add a null check for fear of changing the behavior, since we didn't have tests around it.