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

add capability to create asset types via API #191

Merged
merged 8 commits into from
Aug 12, 2014
Merged

Conversation

byxorna
Copy link
Contributor

@byxorna byxorna commented Aug 7, 2014

Like the title says, this allows API users to create, delete, and list AssetTypes.

I still need to add tests for this, but this will enable us to create new asset types to support different usage of collins assets (i.e. SERVICE or REPLICA_CONTROLLERs).

@Primer42 @maddalab

@yl3w
Copy link
Contributor

yl3w commented Aug 7, 2014

Lets get more feedback on this. With some specific examples of why we are doing this. May be even have a review internally first.

@byxorna
Copy link
Contributor Author

byxorna commented Aug 7, 2014

The test suite is working, except one method:

[info] Asset Type Validation should
[info] + Reject empty asset tags
[info]
[info] The REST API should
[info]   Support creation
[info]   + via PUT
[info]   + error if no label given
[info]
[info]   Support getting
[error]   x all asset types
[error]      Response is not JSON or does not contain a 'data' key: {"status":"success:ok","data":[{"ID":1,"NAME":"SERVER_NODE","LABEL":"Server Node"},{"ID":2,"NAME":"SERVER_CHASSIS","LABEL":"Server Chassis"},{"ID":3,"NAME":"RACK","LABEL":"Rack"},{"ID":4,"NAME":"SWITCH","LABEL":"Switch"},{"ID":5,"NAME":"ROUTER","LABEL":"Router"},{"ID":6,"NAME":"POWER_CIRCUIT","LABEL":"Power Circuit"},{"ID":7,"NAME":"POWER_STRIP","LABEL":"Power Strip"},{"ID":8,"NAME":"DATA_CENTER","LABEL":"Data Center"},{"ID":9,"NAME":"CONFIGURATION","LABEL":"Configuration"},{"ID":10,"NAME":"SERVICE","LABEL":"testlabel"}]} (RequestHelpers.scala:111)
[info]   + by name
[info]
[info]   Support deleting
[info]   + by name

I am not sure why its not finding the data key, because it is sending Content-Type: application/json, and data is clearly in the response...

-> $ curl -v --basic -u blake:admin:first -XGET localhost:9000/api/assettypes                                                                                                [4/3888]
* Adding handle: conn: 0x7f853b803a00
* Adding handle: send: 0
* Adding handle: recv: 0
* Curl_addHandleToPipeline: length: 1
* - Conn 0 (0x7f853b803a00) send_pipe: 1, recv_pipe: 0
* About to connect() to localhost port 9000 (#0)
*   Trying ::1...
* Connected to localhost (::1) port 9000 (#0)
* Server auth using Basic with user 'blake'
> GET /api/assettypes HTTP/1.1
> Authorization: Basic Ymxha2U6YWRtaW46Zmlyc3Q=
> User-Agent: curl/7.30.0
> Host: localhost:9000
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: application/json; charset=utf-8
< Content-Language: en
< Content-Length: 480
<
* Connection #0 to host localhost left intact
{"status":"success:ok","data":[{"ID":1,"NAME":"SERVER_NODE","LABEL":"Server Node"},{"ID":2,"NAME":"SERVER_CHASSIS","LABEL":"Server Chassis"},{"ID":3,"NAME":"RACK","LABEL":"Rack"},{"D":4,"NAME":"SWITCH","LABEL":"Switch"},{"ID":5,"NAME":"ROUTER","LABEL":"Router"},{"ID":6,"NAME":"POWER_CIRCUIT","LABEL":"Power Circuit"},{"ID":7,"NAME":"POWER_STRIP","LABEL":"Power
 Strip"},{"ID":8,"NAME":"DATA_CENTER","LABEL":"Data Center"},{"ID":9,"NAME":"CONFIGURATION","LABEL":"Configuration"}]}

@yl3w
Copy link
Contributor

yl3w commented Aug 9, 2014

the data field is an array play.api.libs.json.JsArray of play.api.libs.json.JsObject It is not a JsObject itself.

You can change RequestHelpers.scala JsonDataMatcher

      val data = (parsed \ "data")
      val matchResult = data.isInstanceOf[JsObject] || data.isInstanceOf[JsArray]

and

[collins] $ test-only controllers.AssetTypeApiSpec
[info] Compiling 1 Scala source to /Users/bhaskar/workspace/collins/target/scala-2.9.1/test-classes...
[WARN] [08/09/2014 08:52:33.558] [pool-252-thread-3] [Dispatchers] Dispatcher [akka.actor.promises-dispatcher] not configured, using default-dispatcher
[WARN] [08/09/2014 08:52:33.573] [play-akka.actor.default-dispatcher-1] [Dispatchers] Dispatcher [akka.actor.actions-dispatcher] not configured, using default-dispatcher
[info] Asset Type API Specification
[info]
[info] Asset Type Validation should
[info] + Reject empty asset tags
[info]
[info] The REST API should
[info]   Support creation
[info]   + via PUT
[info]   + error if no label given
[info]
[info]   Support getting
[info]   + all asset types
[info]   + by name
[info]
[info]   Support deleting
[info]   + by name
[info]
[info]
[info]
[info] Total for specification Asset Type API Specification
[info] Finished in 4 seconds, 666 ms
[info] 6 examples, 0 failure, 0 error
[info]
[info] Passed: : Total 6, Failed 0, Errors 0, Passed 6, Skipped 0
[success] Total time: 7 s, completed Aug 9, 2014 8:52:34 AM

But please make sure you verify everything once more.

@@ -19,6 +19,7 @@ case class AssetType(name: String, label: String, id: Int = 0) extends Validated
object AssetType extends Schema with AnormAdapter[AssetType] {

override val tableDef = table[AssetType]("asset_type")
val reservedNames = List("SERVER_NODE","SERVER_CHASSIS","RACK","SWITCH","ROUTER","POWER_CIRCUIT","POWER_STRIP","DATA_CENTER","CONFIGURATION")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, these should be externalized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are all hardcoded in the evolutions, and are in some way or another used by collins internally.

@byxorna
Copy link
Contributor Author

byxorna commented Aug 9, 2014

@maddalab thanks for scoping that out, fix committed for the tests.

Ive added support into collins_shell and collins_client to support assettype API, and confirmed they are both functional. This is RFR @Primer42 @maddalab

@william-richard
Copy link
Contributor

I haven't tested, but this LGTM.

byxorna added a commit that referenced this pull request Aug 12, 2014
add capability to create asset types via API
@byxorna byxorna merged commit d48e9cc into master Aug 12, 2014
@william-richard william-richard deleted the gabe-arbitrary-assets branch September 9, 2014 19:55
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.

4 participants