-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix(http): fix java.lang.IllegalStateException: STREAMED #5367
Changes from 6 commits
029c040
9003b0e
0bcaaad
0664c4a
e404b18
b1a2870
8bad304
b3400df
75d832e
304dba4
c8a0cc5
9cc6b5e
1d3036a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. For historical reasons, this logic does not have a unit test. Better add comprehensive unit tests for all situations after changing the whole logic of the param extraction to make sure all the new logic works well, such as: good requests for both form&json |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
package org.tron.core.services.http; | ||
|
||
import static org.junit.Assert.assertEquals; | ||
|
||
import com.alibaba.fastjson.JSONObject; | ||
|
||
import java.io.UnsupportedEncodingException; | ||
|
||
import org.junit.Assert; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.springframework.mock.web.MockHttpServletRequest; | ||
import org.springframework.mock.web.MockHttpServletResponse; | ||
|
||
public class GetBrokerageServletTest { | ||
|
||
private MockHttpServletRequest request; | ||
private MockHttpServletResponse response; | ||
|
||
@Before | ||
public void setUp() { | ||
request = new MockHttpServletRequest(); | ||
request.setMethod("POST"); | ||
request.setContentType("application/x-www-form-urlencoded"); | ||
request.setCharacterEncoding("UTF-8"); | ||
|
||
response = new MockHttpServletResponse(); | ||
} | ||
|
||
@Test | ||
public void getBrokerageTest() { | ||
request.addParameter("address", ""); | ||
GetRewardServlet getRewardServlet = new GetRewardServlet(); | ||
getRewardServlet.doPost(request, response); | ||
try { | ||
String contentAsString = response.getContentAsString(); | ||
JSONObject result = JSONObject.parseObject(contentAsString); | ||
int reward = (int)result.get("reward"); | ||
Assert.assertEquals(0, reward); | ||
String content = (String) result.get("Error"); | ||
Assert.assertNull(content); | ||
} catch (UnsupportedEncodingException e) { | ||
e.printStackTrace(); | ||
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. Assert.fail(); is a good choice 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. I'm not expecting an exception to be thrown; the end result of this use case is a normal return of 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. Does it mean that even if an UnsupportedEncodingException is thrown, it's still normal? 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. Using a statement when pointing out a problem is a good way to respond; if you use a rhetorical question, I can't be sure of your intentions and it doesn't help to accurately describe the problem. 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. The point of this use case is not to test for the UnsupportedEncodingException, but rather to determine the value of null. 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.
|
||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package org.tron.core.services.http; | ||
|
||
import com.alibaba.fastjson.JSONObject; | ||
|
||
import java.io.UnsupportedEncodingException; | ||
|
||
import org.junit.Assert; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.springframework.mock.web.MockHttpServletRequest; | ||
import org.springframework.mock.web.MockHttpServletResponse; | ||
|
||
public class GetRewardServletTest { | ||
|
||
private MockHttpServletRequest request; | ||
private MockHttpServletResponse response; | ||
|
||
@Before | ||
public void setUp() { | ||
request = new MockHttpServletRequest(); | ||
request.setMethod("POST"); | ||
request.setContentType("application/x-www-form-urlencoded"); | ||
request.setCharacterEncoding("UTF-8"); | ||
|
||
response = new MockHttpServletResponse(); | ||
} | ||
|
||
@Test | ||
public void getRewardTest() { | ||
request.addParameter("address", ""); | ||
GetRewardServlet getRewardServlet = new GetRewardServlet(); | ||
getRewardServlet.doPost(request, response); | ||
try { | ||
String contentAsString = response.getContentAsString(); | ||
JSONObject result = JSONObject.parseObject(contentAsString); | ||
int reward = (int)result.get("reward"); | ||
Assert.assertEquals(0, reward); | ||
String content = (String) result.get("Error"); | ||
Assert.assertNull(content); | ||
} catch (UnsupportedEncodingException e) { | ||
e.printStackTrace(); | ||
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. Please test all cases of checkGetParam. 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.
|
||
} | ||
} | ||
} |
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.
getParam
orcheckAndGetParam
?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.
Use
checkGetParam
as the method name, because there are some checking judgments