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

Add API endpoints for /project_metadata #702

Merged
merged 1 commit into from
Mar 20, 2017

Conversation

dsyer
Copy link
Contributor

@dsyer dsyer commented Mar 17, 2017

User can GET or delete a release via /{projectId}/{version} or
add/edit a release by POSTing the whole release to /{projectId}.

Authentication is via HTTP Basic with a Github API token (as the
username and an empty password, just like Github native APIs).
All the new endpoints are protected.

Fixes gh-701

User can GET or delete a release via /{projectId}/{version} or
add/edit a release by POSTing the whole release to /{projectId}.

Authentication is via HTTP Basic with a Github API token (as the
username and an empty password, just like Github native APIs).
All the new endpoints are protected.
@gregturn
Copy link
Contributor

I would prefer PUT over POST if we're talking about am including the ID in the URL. POST is usually used for the collective resource, and you usually get back a location header. Not the case with PUT.

@@ -33,4 +40,87 @@ public Project projectMetadata(@PathVariable("projectId") String projectId) thro
return service.getProject(projectId);
}

@RequestMapping(value = "/{projectId}/**", method = GET)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny snit, but I kind of prefer it when it reads...

@RequestMapping(method = GET, value = "/{projectId}/**")

That way, it reads like a REST call, GET /whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just the way the existing endpoint was defined. I didn't want to change a convention (I would prefer @GetMapping anyway).

throw new MetadataNotFoundException("Could not find " + version + " for " + projectId);
}

@RequestMapping(value = "/{projectId}", method = POST)
Copy link
Contributor

Choose a reason for hiding this comment

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

See previous comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, since this is coming with the ID, I think PUT is preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're reading it wrong. It's appending or updating a release to an existing project. I think POST is better.

@bclozel bclozel merged commit 0ca58a5 into spring-attic:master Mar 20, 2017
@dsyer dsyer deleted the feature/metadata branch March 20, 2017 09:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants