Skip to content

Documentation#23

Merged
mak438 merged 2 commits intojnm92:GEOWAVE-rest-projectfrom
mak438:Docs
May 10, 2017
Merged

Documentation#23
mak438 merged 2 commits intojnm92:GEOWAVE-rest-projectfrom
mak438:Docs

Conversation

@mak438
Copy link
Copy Markdown
Collaborator

@mak438 mak438 commented May 8, 2017

See https://drive.google.com/file/d/0B5xyQKoKRPyXMDVyMTZXQlVaT2s/view?usp=sharing (click download and then open with a web browser).

Copy link
Copy Markdown
Owner

@jnm92 jnm92 left a comment

Choose a reason for hiding this comment

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

looks good overall, just a couple questions/comments

Comment thread docs/content/devguide/097-rest.adoc Outdated
To enable access to the REST server for a new command, perform the following steps:

. Ensure that the `geowave-service-rest` project has a dependency on the new command. All classes which the REST server can access are visible from the homepage at `http://localhost:5152`, and should appear in [blue]#blue# if they are not yet implemented.
. Ensure that the class implementing the operation extends `DefaultOperation`. This is necessary because `DefaultOperation` extends `ServerResource`, which restlet requires for all endpoints.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

'which restlet requires' -> this is the first mention of restlet, which people might be unfamiliar with, I think just 'which the REST server requires' could be more clear

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Comment thread docs/content/devguide/097-rest.adoc Outdated
----

[start=5]
. Common code that is invoked from the http request should be implemented in the operation's `computeResults` method (inheriting from `DefaultOperation#computeResults`). This is the method that will be called with the operation is invoked via REST, and should return a Bean object that will be serialized to JSON for the response. If the operation produces no response (for example, in the case of `CopyStoreCommand`), it should return `null`, which can be enforced by specifying a return type of https://docs.oracle.com/javase/8/docs/api/java/lang/Void.html[Void]. The execute method should call `computeResults`, and then display the results (if any) to the console.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what is a 'Bean object' in 'should return a bean object that will be serialized to JSON for the response'

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's a standard notion, but I updated it to make it more clear.

Comment thread docs/content/devguide/097-rest.adoc Outdated
----

[start=5]
. Common code that is invoked from the http request should be implemented in the operation's `computeResults` method (inheriting from `DefaultOperation#computeResults`). This is the method that will be called with the operation is invoked via REST, and should return a Bean object that will be serialized to JSON for the response. If the operation produces no response (for example, in the case of `CopyStoreCommand`), it should return `null`, which can be enforced by specifying a return type of https://docs.oracle.com/javase/8/docs/api/java/lang/Void.html[Void]. The execute method should call `computeResults`, and then display the results (if any) to the console.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

might be worth mentioning that the computeResults logic can be taken directly from the existing execute and then just having execute call that. this is hinted at with 'the execute method shoudl call computeResults' but it might be nice to be more explicit about this, as it could save people a lot of time from reimplementing execute

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added.

@mak438 mak438 merged commit a492c69 into jnm92:GEOWAVE-rest-project May 10, 2017
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