Skip to content
This repository has been archived by the owner on Feb 26, 2023. It is now read-only.

#475 - Rest Authentication #579

Merged
merged 17 commits into from Jul 12, 2013
Merged

#475 - Rest Authentication #579

merged 17 commits into from Jul 12, 2013

Conversation

dhleong
Copy link
Contributor

@dhleong dhleong commented May 5, 2013

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
Copy link
Contributor

DayS commented May 21, 2013

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
Copy link
Contributor Author

dhleong commented May 26, 2013

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
Copy link
Contributor

DayS commented Jun 9, 2013

This PR needs to be updated

@dhleong
Copy link
Contributor Author

dhleong commented Jun 9, 2013

What do you want me to update?

@DayS
Copy link
Contributor

DayS commented Jun 9, 2013

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
Copy link
Contributor Author

dhleong commented Jun 9, 2013

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
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
Copy link
Contributor Author

dhleong commented Jun 9, 2013

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
Copy link
Contributor

DayS commented Jun 9, 2013

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

@DayS
Copy link
Contributor

DayS commented Jun 9, 2013

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

@rom1v
Copy link
Contributor

rom1v commented Jun 10, 2013

@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
Copy link
Contributor

DayS commented Jul 12, 2013

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

@dhleong
Copy link
Contributor Author

dhleong commented Jul 12, 2013

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.

None yet
3 participants
@dhleong @DayS @rom1v