Skip to content

Added the "endpointURLParam" option #117

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

Merged
merged 7 commits into from
Jan 20, 2013
Merged

Added the "endpointURLParam" option #117

merged 7 commits into from
Jan 20, 2013

Conversation

marbetschar
Copy link

To add Java servlet spec compatibility whenever Taffy is used as a subpart of an application. This adds the calling capability of Taffy endpoints via a regular queryString variable as discussed in #116.

Marco Betschart added 2 commits January 4, 2013 21:18
@marbetschar
Copy link
Author

Hey Adam! Can you please provide me some additonal info so I may add some Tests to Taffy? Would be great to see the pull request merged into the official version soon :)

@atuttle
Copy link
Owner

atuttle commented Jan 6, 2013

Sorry, hectic few days around the house. There are a couple of ways that I generally run the tests:

  • ANT should work -- just type ant in terminal inside the Taffy folder, the default target will run the tests
  • Web runner at: http://localhost/taffy/tests/tests/

Both require that you have MXUnit installed either in the web root at /mxunit or via a CF mapping. I think having it in the web root works better, as there are images, css, js, etc that it needs to load for the web runner.

One last bit of advice: before running the tests, go to: http://localhost/taffy/tests/?dashboard and make sure everything seems fine there.

@marbetschar
Copy link
Author

Nevermind :)

I've setup mxUnit inside of the webroot because of the reason you have mentioned - I feel like it should work.

But I can't run http://localhost/Taffy/tests/tests/ as the only thing I can see there is the "Taffy is up and running!" output. I've seen you have added this inside of the Application.cfc, maybe there's something wrong with the code there?

When I open up http://localhost/Taffy/tests/?dashboard I see the following output:

{"DETAIL":"","ERROR":"invalid component definition, can't find customJsonRepresentation","TAGCONTEXT":"\/Users\/marbetschar\/Sites\/Taffy\/core\/api.cfc [Line 799]"}

@atuttle
Copy link
Owner

atuttle commented Jan 6, 2013

I get the "Taffy is up and running!" message if I open /Taffy/tests/tests/ before opening /Taffy/tests/?dashboard. But after the latter, the former runs fine for me.

@atuttle
Copy link
Owner

atuttle commented Jan 6, 2013

So there's some problem that's preventing the test-api (/taffy/tests/?dashboard) from running properly for you. We'll have to get that figured out first.

Modify your /taffy/core/api.cfc's onError method to dump the exception and abort, and use the stack trace to see where the problem is coming from...

@marbetschar
Copy link
Author

Finally I got the time to digg into the issue. The problem was laying in paths :)

After a few modifications on the test environment to guarantee the API is loaded before any tests are called and adding some application specific mappings the testSuite runs just fine (see /Taffy/tests/tests/Application.cfc for changes). Additionally I had to modify your tests.tests.base.apiCall() to ensure the newly created endpointURLParam is used correctly.

I've added one test for each HTTP verb to your test suite for testing the endpointURLParam and tested the testSuite in the following environments (with builtin webservers): ACF 9.0.1, ACF 10 and Railo 4.0.3.

Unfortunately the following tests are failing on Railo 4.0.3 and I can't see any reason why:

returns_error_when_default_mime_not_implemented
returns_error_when_requested_mime_not_supported
returns_error_when_requested_mime_not_supported
put_body_is_mime_content
put_body_is_url_encoded_params
basic_auth_credentials_found
can_upload_a_file

Railo 4.0.3 isn't stable yet, so it may be related to the engine itself. If you agree I would just open a new issue for this.

@@ -0,0 +1,17 @@
<cfcomponent>

<cfset this.name = 'Taffy_testSuite' />
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a brief comment here indicating that the application name is the same as the application in the parent folder so that application contexts are the same. This tripped me up for a minute. :)

Copy link
Author

Choose a reason for hiding this comment

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

I'll add the comment

@atuttle
Copy link
Owner

atuttle commented Jan 17, 2013

Overall, looking good. Please see my comments about adding a (code) comment to the new Application.cfc and clarifying the change to the representation class setting.

Unfortunately the following tests are failing on Railo 4.0.3 and I can't see any reason why:
...
Railo 4.0.3 isn't stable yet, so it may be related to the engine itself. If you agree I would just open a new issue for this.

Just wanted to confirm that these tests that are failing on Railo 4.0.3 are passing on earlier versions of Railo (E.g. 4.0.2, 3.2.x)? I think the last time I tested with Railo it was against 3.2.x.

@marbetschar
Copy link
Author

Unfortunately I'm not able to run the tests on an earlier version of Railo here :(

@atuttle
Copy link
Owner

atuttle commented Jan 17, 2013

No sweat. You've done a ton here and I'm quite grateful. I'll see if I can get my railo test environment back up and running and check everything out there. Assuming the tests pass on the latest Railo 3.3.x then I'll be thrilled to merge this in.

@marbetschar
Copy link
Author

I'm happy to contribute and help taking great software further, so you're welcome :)

Am 17.01.2013 um 21:10 schrieb Adam Tuttle [email protected]:

No sweat. You've done a ton here and I'm quite grateful. I'll see if I can get my railo test environment back up and running and check everything out there. Assuming the tests pass on the latest Railo 3.3.x then I'll be thrilled to merge this in.


Reply to this email directly or view it on GitHub.

atuttle pushed a commit that referenced this pull request Jan 20, 2013
Added the "endpointURLParam" option
@atuttle atuttle merged commit 6aab3f2 into atuttle:1.3-dev Jan 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants