Skip to content
This repository has been archived by the owner on Oct 24, 2024. It is now read-only.

Implemented APIKeys, as well as getting to conform to check style #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jecortez
Copy link

@jecortez jecortez commented Oct 5, 2015

No description provided.

@jecortez jecortez force-pushed the APIKeyImpl_Client branch 2 times, most recently from 391312e to e9f4a38 Compare October 5, 2015 19:17
@jecortez
Copy link
Author

jecortez commented Oct 5, 2015

@bbeck @mozart27 Mind giving this one a look through? I want to make sure we're happy with the way I'm doing APIKey validation since I'll be basically copy-pasting this into the other services I'm working on (Hubster, Charon, Hollywood)

@jecortez jecortez force-pushed the APIKeyImpl_Client branch 2 times, most recently from da1d81f to bad94fc Compare October 6, 2015 14:54
@PUT
@Path("/{id}/password")
public Response updateUserPassword(@PathParam("id") long id, String password) {
public Response updateUserPassword(@Auth ApiKey key,
@PathParam("id") long id, String password) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: newline the String password?

@mozart27
Copy link
Contributor

mozart27 commented Oct 6, 2015

One point to discuss. @bbeck had previously brought up the question with me on whether or not we want to have references to mysql or any other db in the base project. The consensus at the time was no since folks leveraging this template could use MySQL, Mongo, DynamoDB or whatever. So would it be better to remove the project dependencies on mysql?

@jecortez jecortez force-pushed the APIKeyImpl_Client branch 3 times, most recently from c58b03f to 6d4140b Compare October 6, 2015 17:16
<source>1.8</source>
<target>1.8</target>
<compilerArgs>
<arg>-parameters</arg>
Copy link
Contributor

Choose a reason for hiding this comment

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

???

Copy link
Contributor

Choose a reason for hiding this comment

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

java 8 parameters name support. Makes using Jackson way easier (no more JsonProperty unless you want to use a different name)

Copy link
Contributor

Choose a reason for hiding this comment

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

This should only be added if its needed though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, gotcha. And I agree on only adding it where it's needed.

@bbeck
Copy link
Contributor

bbeck commented Oct 6, 2015

I +1 JB's comment about DBs above. Also would be nice if PRs were smaller, single purpose changes. Much easier to read that way vs doing 3 things at once.

@jecortez
Copy link
Author

jecortez commented Oct 6, 2015

Implemented @bbeck and @mozart27 changes. Left in mysql dependencies for now to enable app to run out of the box

@biglandy71
Copy link

@jecortez this is great stuff. are you ready to merge? lgtm.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants