Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings
This repository was archived by the owner on Feb 26, 2023. It is now read-only.

#475 - Rest Authentication#579

Merged
DayS merged 17 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dhleong:475_Rest_Authenticationdhleong/androidannotations:475_Rest_AuthenticationCopy head branch name to clipboard
Jul 12, 2013
Merged

#475 - Rest Authentication#579
DayS merged 17 commits into
androidannotations:developandroidannotations/androidannotations:developfrom
dhleong:475_Rest_Authenticationdhleong/androidannotations:475_Rest_AuthenticationCopy head branch name to clipboard

Conversation

@dhleong

@dhleong dhleong commented May 5, 2013

Copy link
Copy Markdown
Contributor

This is to fulfill my ticket, #475 Rest Authentication

It's been a while, but I finally took a stab at getting AndroidAnnotations to play nice, and it seems to be cooperating. Everything builds, and I even got it to work in my project!

This is my first pull request to an open source project, so please bear with me :) Let me know if there's anything I need to add.

dhleong added 14 commits May 4, 2013 15:38
@dhleong
Add new annotations
d23a232
@dhleong
Init some internal structures, add an annotation to tests
8122abf
@dhleong
Allow full urls in @post, etc. to be specified and handled correctly
37ea732
@dhleong
Give entity a var so we can pull cookies from it if needed
63d51ee
@dhleong
Implement SetsCookie, only using an Entity instance if needed
dd1397b
@dhleong
Implement RequiresCookieInUrl, and prepare for a few others
c377bb5
@dhleong
Implement RequiresCookie and RequiresHeader
d73107e
@dhleong
Implement RequiresAuthorization
0cab243
@dhleong
Implement setters and getters for cookies and headers and validate them
aae71df
@dhleong
Implement setAuthentication with validation
aac6b43
@dhleong
Implement setHttpBasicAuth method
0afc92d
@dhleong
Fix some validation; ensure HttpHeaders for auth
0d2dabd
@dhleong
Properly handle returned cookie values (extract only ACTUAL value)
b768498
@dhleong
Add license headers
a0986a2
@DayS

DayS commented May 21, 2013

Copy link
Copy Markdown
Contributor

That's quite a huge PR.
Could you add some functional tests to both help us understand the use case and check that everything compiles ? :)

@dhleong

dhleong commented May 26, 2013

Copy link
Copy Markdown
Contributor Author

I'm not very familiar with Mockito, but I think it has tolerable coverage and it demonstrates that it compiles.

Is there a way for the mock()' d RestTemplate to actually return a Response? I was hitting my head against the wall thinking the headers weren't being set until I realized that there was just no response being returned, causing the tests to fail, so I rewrote the tests to what you see so they don't use a mock()'d RestTemplate when they actually need the Response. If there's a way to get one (I didn't look very hard... it's already pretty late) I could write a test case that makes a bit more sense (a bit closer to an actual use case).

Thanks!

@DayS

DayS commented Jun 9, 2013

Copy link
Copy Markdown
Contributor

This PR needs to be updated

@dhleong

dhleong commented Jun 9, 2013

Copy link
Copy Markdown
Contributor Author

What do you want me to update?

@DayS

DayS commented Jun 9, 2013

Copy link
Copy Markdown
Contributor

We merged some PR since your last commit. Now you have two minor conflicts to resolve before we can test and merge it :)
Sorry for the late answer by the way.

@dhleong

dhleong commented Jun 9, 2013

Copy link
Copy Markdown
Contributor Author

Ah, okay. I'll pull the changes later today and look into it. Thanks for
the heads up :)
On Jun 9, 2013 4:15 PM, "Damien" notifications@github.com wrote:

We merged some PR since your last commit. Now you have two minor conflicts
to resolve before we can test and merge it :)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/579#issuecomment-19172679
.

dhleong added 2 commits June 9, 2013 17:06
@dhleong
Merge branch 'develop' of github.com:excilys/androidannotations into …
7bbac6c 
…develop

Conflicts:
	AndroidAnnotations/androidannotations/src/main/java/org/androidannotations/helper/ValidatorHelper.java
	AndroidAnnotations/functional-test-1-5/src/main/java/org/androidannotations/test15/rest/MyService.java
@dhleong
Merge branch 'develop' into 475_Rest_Authentication
01dc8c8
@dhleong

dhleong commented Jun 9, 2013

Copy link
Copy Markdown
Contributor Author

Okay, merged the changes and resolved the conflicts. I was able to successfully run mvn clean test once, but a few times this failure appeared:

Failed tests: cancellableSerializedBackgroundTasks(org.androidannotations.test15.ThreadActivityTest): Requested tasks should have completed execution

I was, however, also able to reproduce this failure on a clean clone of develop, so it's not my code causing it—it's a completely unrelated test, for that matter. Since I was able to test successfully once, I've concluded that my code still passes its tests, and this one is probably a race condition or related to timing.

@DayS

DayS commented Jun 9, 2013

Copy link
Copy Markdown
Contributor

Damn. Everything worked during my tests... I'll check that tomorrow now.

@DayS

DayS commented Jun 9, 2013

Copy link
Copy Markdown
Contributor

This is related to the newly merged PR #569.
On 10 consecutive runs, the test failed 2 times.

@rom1v

rom1v commented Jun 10, 2013

Copy link
Copy Markdown
Contributor

@DayS, I will analyze and work on it today.

@rom1v rom1v mentioned this pull request Jun 10, 2013
Merged
DayS added a commit that referenced this pull request Jul 12, 2013
@DayS
Merge pull request #579 from dhleong/475_Rest_Authentication
ca0588f 
#475 - Rest Authentication
@DayS DayS merged commit ca0588f into androidannotations:develop Jul 12, 2013
@DayS

DayS commented Jul 12, 2013

Copy link
Copy Markdown
Contributor

Seems great. I'm merging it... Finally :D
Thanks 👍

@dhleong

dhleong commented Jul 12, 2013

Copy link
Copy Markdown
Contributor Author

Awesome, thanks!
On Jul 12, 2013 10:40 AM, "Damien" notifications@github.com wrote:

Seems great. I'm merging it... Finally :D
Thanks [image: 👍]

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/579#issuecomment-20880435
.

@DayS DayS mentioned this pull request Jul 13, 2013
Closed
DayS pushed a commit to DayS/androidannotations that referenced this pull request Dec 17, 2013
@rom1v
Serialized @background task cancellation bugfix
8d0c9f5 
If a serialized @background task was cancelled after it had been
submitted to the executor but before its run() method was called, then
the following tasks with the same serial identifier were not executed.

Issue reported here:
androidannotations#579 (comment)

Detected by ThreadActivityTest#cancellableSerializedBackgroundTasks()
(but not always due to the race condition)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dhleong @DayS @rom1v
Morty Proxy This is a proxified and sanitized view of the page, visit original site.