Skip to content

removed unmotivated throwing of UnsupportedEncodingException#75

Merged
lbalmaceda merged 3 commits intoauth0:masterfrom
neshanjo:removed-unsupported-enconding-exception
Sep 8, 2017
Merged

removed unmotivated throwing of UnsupportedEncodingException#75
lbalmaceda merged 3 commits intoauth0:masterfrom
neshanjo:removed-unsupported-enconding-exception

Conversation

@neshanjo
Copy link
Copy Markdown
Contributor

@neshanjo neshanjo commented Sep 5, 2017

With this small fix, the API is easier to use, since the consumer does not have to handle the UnsupportedEncodingException which is neither motivated nor documented in javadoc.

@lbalmaceda lbalmaceda modified the milestones: 1.2.0, v1-Next Sep 6, 2017
@lbalmaceda
Copy link
Copy Markdown
Contributor

Thanks! Yeah, I know the encoding should be supported that's why I had that exception anyway, but it's better to make it runtime. 👍 Coverage check won't let me merge it without the corresponding tests. A simple "shouldThrowWhenQueryEncodingNotSupported" test on each class would suffice. You'll need to add a @VisibileForTesting package private method in the QueryFilter class to urlEncode(String value), which will return the output of URLEncoder.encode(value, "UTF-8"); but for a single test on each class we will mock that to throw an exception when called with that value and encoding. Tests will look like this:

@Rule
public ExpectedException exception = ExpectedException.none()

@Test
public void shouldThrow...() throws Exception {
  exception.expect(IllegalStateException)
  exception.expectMessage("the exception message goes here")
  String value = "my=test value"
  QueryFilter filter = Mockito.spy(new QueryFilter())
  Mockito.doThrow(UnsupportedEncodingException).when(filter.urlEncode(value, "UTF-8"))
  filter.withQuery(value)
}

I'm sure you'll find similar tests for other classes. Please repeat that for each class and the coverage will raise again.
Cheers

@neshanjo neshanjo force-pushed the removed-unsupported-enconding-exception branch from d7528e5 to 06fb87e Compare September 8, 2017 06:35
@neshanjo neshanjo force-pushed the removed-unsupported-enconding-exception branch from 06fb87e to 77f936d Compare September 8, 2017 06:41
@neshanjo
Copy link
Copy Markdown
Contributor Author

neshanjo commented Sep 8, 2017

Done. Couldn't find @VisibileForTesting somewhere in the project, so I've omitted it.

Copy link
Copy Markdown
Contributor

@lbalmaceda lbalmaceda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah.. that's an android annotation, my bad 😛 thanks again! 👏

@lbalmaceda lbalmaceda merged commit faf4adf into auth0:master Sep 8, 2017
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.3.0 Sep 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants