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

Add openAPI #47

Merged
merged 14 commits into from
Jul 6, 2022
Merged

Add openAPI #47

merged 14 commits into from
Jul 6, 2022

Conversation

atsushii
Copy link
Contributor

@atsushii atsushii commented Jun 18, 2022

Addresses #9

What I did

Please check the doc!

step

1, mage run
2, access to http://localhost:8002/docs

@aqaurius6666
Copy link

how did you generate openapi.yaml file?

@atsushii
Copy link
Contributor Author

atsushii commented Jun 22, 2022

@decentralgabe
Copy link
Member

Thank you for the PR, @atsushii

I am weighing the tradeoff of managing this API file separately, as opposed to using annotations like https://github.com/swaggo/swag

Annotations have the benefit of being next to the API code, which I think will have the effect of keeping the docs more up to date, and also simpler to read - since you can see the API exposed by looking at the code.

I looked at stoplight but the documentation is a bit sparse. It's most important that we're able to keep these docs up to date and easy to read. What do you think?

@atsushii
Copy link
Contributor Author

@decentralgabe
I agree with your idea that keep these docs up to date and easy to read!
I will definitely fix my PR. Thanks for the advice!

@atsushii
Copy link
Contributor Author

@decentralgabe

I have fixed my code to generate spec yaml by annotations using https://github.com/swaggo/swag

Please review it!

@decentralgabe
Copy link
Member

thank you @atsushii this looks really good. I will take a more in depth look after the weekend.

Copy link
Member

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

minor changes - looks good!

cmd/main.go Outdated
Comment on lines 32 to 44
// @title Swagger Example API
// @version 1.0
// @description This is a sample server celler server.
// @termsOfService http://swagger.io/terms/

// @contact.name API Support
// @contact.url http://www.swagger.io/support
// @contact.email [email protected]

// @license.name Apache 2.0
// @license.url http://www.apache.org/licenses/LICENSE-2.0.html

// @host localhost:8080
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// @title Swagger Example API
// @version 1.0
// @description This is a sample server celler server.
// @termsOfService http://swagger.io/terms/
// @contact.name API Support
// @contact.url http://www.swagger.io/support
// @contact.email [email protected]
// @license.name Apache 2.0
// @license.url http://www.apache.org/licenses/LICENSE-2.0.html
// @host localhost:8080
// @title SSI Service API
// @version 0.1
// @description https://github.com/TBD54566975/ssi-service
// @contact.name TBD
// @contact.url https://github.com/TBD54566975/ssi-service/issues
// @contact.email [email protected]
// @license.name Apache 2.0
// @license.url http://www.apache.org/licenses/LICENSE-2.0.html
// @host localhost:8080

@@ -62,6 +63,17 @@ type CreateCredentialResponse struct {
Credential credsdk.VerifiableCredential `json:"credential"`
}

// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// CreateCredential godoc

@@ -87,6 +99,16 @@ type GetCredentialResponse struct {
Credential credsdk.VerifiableCredential `json:"credential"`
}

// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// GetCredential godoc

@@ -113,7 +135,19 @@ type GetCredentialsResponse struct {
Credentials []credsdk.VerifiableCredential `json:"credentials"`
}

// GetCredentials checks for the presence of a query parameter and calls the associated filtered get method
// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// GetCredentials godoc

@@ -174,6 +208,17 @@ func (cr CredentialRouter) getCredentialsBySchema(schema string, ctx context.Con
return framework.Respond(ctx, w, resp, http.StatusOK)
}

// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// DeleteCredential godoc

@@ -14,7 +15,14 @@ const (
HealthOK string = "OK"
)

// Health is a simple handler that always responds with a 200 OK
// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// Health godoc

// ShowAccount godoc
// @Summary Health Check
// @Description Health is a simple handler that always responds with a 200 OK
// @Tags NewSSIServer
Copy link
Member

Choose a reason for hiding this comment

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

what purpose does the tag server here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fix it to HealthCheck, thanks!

@@ -23,8 +24,15 @@ type GetReadinessResponse struct {
ServiceStatuses map[svcframework.Type]svcframework.Status `json:"serviceStatuses"`
}

// ready runs a number of application specific checks to see if all the
// relied upon service are healthy. Should return a 500 if not ready.
// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// Readiness godoc

@@ -40,6 +41,17 @@ type CreateSchemaResponse struct {
Schema schemalib.VCJSONSchema `json:"schema"`
}

// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// CreateSchema godoc

@@ -79,6 +100,16 @@ type GetSchemaResponse struct {
Schema schemalib.VCJSONSchema `json:"schema,omitempty"`
}

// ShowAccount godoc
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ShowAccount godoc
// GetSchemaByID godoc

docs/.swaggo Outdated
@@ -0,0 +1,2 @@
// TODO: read cryptosuite.LDKeyType from sdk properly
skip github.com/TBD54566975/ssi-sdk/did.VerificationMethod
Copy link
Member

Choose a reason for hiding this comment

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

this should be solved before merge, what error did you run into?

Copy link
Contributor Author

@atsushii atsushii Jul 2, 2022

Choose a reason for hiding this comment

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

@decentralgabe
ParseComment error in file {path to did.go} :cannot find type definition: cryptosuite.LDKeyType
the above error occurred when I ran swag init cmd.

Seems cannot read LDKeyType which define inside ssi-sdk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably need to fix the type definition in ssi-sdk..
What do you think?

@decentralgabe

Copy link
Member

Choose a reason for hiding this comment

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

let me take a look. do you mind if I push to the branch if I find a solution?

@atsushii atsushii requested a review from decentralgabe July 2, 2022 09:51
@decentralgabe
Copy link
Member

decentralgabe commented Jul 5, 2022

hi @atsushii I created a PR against your branch here that fixes the issues. if you merge this into your branch, I believe this PR should update automatically: atsushii#1

@atsushii
Copy link
Contributor Author

atsushii commented Jul 6, 2022

Hi @decentralgabe
Thank you for your help! Already Marged your PR and fixed all of your review comments.
Please review my PR!

Copy link
Member

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

awesome job, thank you for your contribution!

@decentralgabe decentralgabe merged commit bf83a59 into TBD54566975:main Jul 6, 2022
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.

3 participants