Skip to content
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

1246 use api controllers #1248

Merged
merged 18 commits into from
Mar 10, 2015
Merged

1246 use api controllers #1248

merged 18 commits into from
Mar 10, 2015

Conversation

0robustus1
Copy link
Contributor

Shall close #1246. My first idea (laid out in the issue-description) was not really appropriate, as we would've
needed to create multiple definitions (at least one for every api-version and one for the frontend) in order to provide outer grouping. Instead we'll do 'inner grouping' by extending the DSL in order to reroute
on mime-type (or arbitrary header, which is not in use yet) to another controller, in this case the api provided controller instead of the default frontend controller.

Additionally this moves every api-action into the appropriate api controller. This is based on the 1233-json_for_model_resources branch (which is in turn based on 1166-add_mimetype_info_to_controllers).
So please obey the following review-order:

  1. 1166 add mimetype info to controllers #1240
  2. 1233 json for model resources #1247
  3. 1246 use api controllers #1248 (this one)

def show
super do |format|
format.xml do
render :show, content_type: 'application/rdf+xml'

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@0robustus1 0robustus1 force-pushed the 1246-use_api_controllers branch 3 times, most recently from e444fc0 to 2d940e3 Compare March 9, 2015 12:53
@@ -10,14 +10,29 @@
# as per Loc/Id definition

# Special (/ref-based) Loc/Id routes
specified_get '/ref/:reference/:repository_id/*locid' => 'api/v1/ontology_versions#show',
as: :ontology_iri_versioned,
constraints: [

Choose a reason for hiding this comment

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

Align the elements of a hash literal if they span more than one line.

@eugenk eugenk self-assigned this Mar 9, 2015
@0robustus1 0robustus1 force-pushed the 1246-use_api_controllers branch 3 times, most recently from 122d235 to 2f6a526 Compare March 9, 2015 15:07
@eugenk
Copy link
Member

eugenk commented Mar 9, 2015

The seeds fail at the proof statuses step: Hets can't retrieve the proof statuses file from the running ontohub instance.

❯ wget --header='Accept: */*; q=0.1, text/plain' --no-check-certificate -O - http://localhost:3000/ref/1/meta/proof_statuses 2> /dev/null
{"iri":"localhost:3000/ref/1/meta/proof_statuses","evaluation_state":"failed","number":1,"commit_oid":"0ae3c897ea23e8729f681c147d6fd19006c0c879","basepath":"proof_statuses","file_extension":".owl","ontology":{"iri":"localhost:3000/meta/proof_statuses","name":"Proof_statuses"}}

(The wget call is copied from the Hets source)

@eugenk
Copy link
Member

eugenk commented Mar 9, 2015

http://ontohub.dev/meta should open the file browser, but it shows the repository show-page.

@0robustus1
Copy link
Contributor Author

Jep that is correct.

@0robustus1
Copy link
Contributor Author

http://ontohub.dev/meta should now again show the file browser.
Additionally the seed-problem should be fixed.

@eugenk
Copy link
Member

eugenk commented Mar 9, 2015

Yes, it's fixed. I'm reviewing the last commits right now.

@eugenk
Copy link
Member

eugenk commented Mar 9, 2015

I'm not sure if this is a problem with curl, specroutes, rails routing or I got something wrong and this is a request for whom no support is intended:

❯ curl -H 'Accept: application/json' http://ontohub.dev/default/partial_order//strict_partial_order//irreflexive
ActiveRecord::RecordNotFound at /default/partial_order/strict_partial_order/irreflexive
=======================================================================================

> Couldn't find Sentence without an ID
[...]

The same goes for http://ontohub.dev/default/partial_order//strict_partial_order//elem

@0robustus1
Copy link
Contributor Author

Jep, when i adjusted the routes for the MIME-type override i forgot to set the id in the router-constraint. So it is actually an 'issue' in rails routing ;)

@eugenk
Copy link
Member

eugenk commented Mar 10, 2015

👍 after merging #1247 and rebasing staging

We use ActiveModel::Serializers instead and this
does not require particular views.
This is for the newly implemented reroutes feature, which
allows rerouting to a different controller based on mime-type.
Additionally this gets us the allstar feature and a breaking reversal
of standard allstar handling (it is deactivated by default).
This will actually support a show on ontology-versions
via json.
It seems that the parse-call was done before
an ontology-version was created (which seems crazy to me).
However using do_not_parse and manually calling parse works.
0robustus1 added a commit that referenced this pull request Mar 10, 2015
@0robustus1 0robustus1 merged commit 4771580 into staging Mar 10, 2015
@0robustus1 0robustus1 deleted the 1246-use_api_controllers branch March 10, 2015 13:01
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.

Group API versions together with constraint-namespace
3 participants