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

Proof of concept on admin query interface #2120

Merged
merged 37 commits into from Jun 3, 2019

Conversation

@mikena-truss
Copy link
Contributor

commented May 10, 2019

Description

In the Admin API Design we defined a black box of a query builder. The reason being, admin queries need to be more flexible, but more consistent between models. Opposed to the current query patterns, where queries are written one off and don't follow a pattern across handlers.

This is the first take at defining what that query interface can be.

It uses reflection on the models and takes advantage of the db struct field tags to build queries. There are some security checks that need to be here that I will talk about in line.

The general idea is that we can pass any model with a set of filters to a set of functions. Not only does this provide us with consistent query interface, it is also the first step to hiding pop from our services and handlers. The query builder would become the dependency rather than the db connection.

This is a bit experimental, not set in stone, and I would love some feedback.

Reviewer Notes

In line

Setup

Right now this isn't hooked up to a handler, so running the service tests is the best way to check it out.

Code Review Verification Steps

References

return fmt.Sprintf("%v is not valid input", e.input)
}

func getDBColumn(t reflect.Type, field string) (string, bool) {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

Here's the reflection code checking the column names. We can extend this to other functions in the future for things like associations or create our own struct fields for access patterns.


func (p *popQueryBuilder) FetchOne(model interface{}, field string, value interface{}) error {
// todo: pointer check on type
column, ok := getDBColumn(reflect.TypeOf(model).Elem(), field)

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

Notice we always use the returned name here. This is part of the SQL injection defense, since we're interpolating using Sprintf. Regardless of what the user inputs, we use our predefined struct field tags.

Note that this seems a little complex at first. But having reusable code that rarely changes vs sparse code that is constantly changing (our current typed model fetch methods) actually makes it much easier to control access patterns.

https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.md#defense-option-3-whitelist-input-validation

return "", false
}

func (p *popQueryBuilder) FetchOne(model interface{}, field string, value interface{}) error {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

Note that passing an interface{} isn't providing any less type control than pop. The pop fetch methods already take interface{} rather than some other model interface.

return query.First(model)
}

func (p *popQueryBuilder) FetchMany(model interface{}, filters map[string]interface{}) error {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

This filter map will need to be expanded. For example, we'll need to add an enum for comparators.

This is also where the other API specs from the doc can be applied. Such as sort, pagination, associations, etc.

}

func (p *popQueryBuilder) FetchOne(model interface{}, field string, value interface{}) error {
// todo: pointer check on type

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

I made some notes about reflect type checking. Calling Elem() can cause a runtime panic. We'll want to enforce that we're getting a pointer and in the FetchMany case a pointer to a slice.

builder officeUserQueryBuilder
}

func (o officeUserFetcher) FetchOfficeUser(field string, value interface{}) (models.OfficeUser, error) {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

This methods becomes pretty light. In the future more will happen here. Like more advanced querying or authorization patterns.

This comment has been minimized.

Copy link
@jim

jim May 16, 2019

Contributor

We could use code generation to generate model-specific funcs that return a specific type. This would get us away from passing in a pointer to work around the type system.


func (o officeUserFetcher) FetchOfficeUser(field string, value interface{}) (models.OfficeUser, error) {
var officeUser models.OfficeUser
error := o.builder.FetchOne(&officeUser, field, value)

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

Notice that the FetchOne doesn't return the office user. This is similar to how pop does things (modifying the pointer). The reason being, with reflection, you would have to return interface{} and then typecast it (I think).

This comment has been minimized.

Copy link
@macrael

macrael May 15, 2019

Contributor

That's right, and I like that this function does return a model, that way we keep the reflection stuff contained a bit.

return m
}

func (suite *UserServiceSuite) TestFetchOfficeUser() {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

The service tests now just become a reaction to what is declared in them. In this case if the dependency succeeds/fails, what should the service do?

Similar to the main method, it's pretty lightweight for the time being.

suite.T().Run("if the user it fetched, it should be returned", func(t *testing.T){
id , err := uuid.NewV4()
suite.NoError(err)
fakeFetchOne := func(model interface{}) error {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

In order to simulate the dependency behavior, we test what happens when it modifies the passed (empty) office user in the tested method. We can pass this to the fake dependency and force it to be called by the real method.

fakeFetchOne func(model interface{}) error
}

func (t *testOfficeUserQueryBuilder) FetchOne(model interface{}, field string, value interface{}) error {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

Notice there's no more pop in the services.

@mikena-truss mikena-truss reopened this May 10, 2019

@mikena-truss mikena-truss added the wip label May 10, 2019

}

// NewPopQueryBuilder returns a new query builder implemented with pop
func NewPopQueryBuilder(db *pop.Connection) *PopQueryBuilder {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 10, 2019

Author Contributor

Constructor is for dependency injection that requires a function.

This comment has been minimized.

Copy link
@kahlouie

kahlouie May 15, 2019

Contributor

What about adding this to the comment section above the function definition? And in all future places that do similar things? I imagine that's still important to know as we move forward

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 15, 2019

Author Contributor

Will do. I also have to add a lot more comments when I polish this all up

func getDBColumn(t reflect.Type, field string) (string, bool) {
for i := 0; i < t.NumField(); i++ {
dbTag, ok := t.Field(i).Tag.Lookup("db")
if ok && dbTag == field {

This comment has been minimized.

Copy link
@reggieriser

reggieriser May 10, 2019

Contributor

Just a note that it looks like Pop doesn't require the db tag to be there:

Note, for each table field, we defined a pop tag matching the field name, but it's not required. If you don't provide a name, Pop will use the name of the struct field to generate one.

We'll either have to handle that possibility or make sure we always use the tag on our models (probably safer to do the former). Might be able to leverage the same API used in Pop.

This comment has been minimized.

Copy link
@reggieriser

reggieriser May 13, 2019

Contributor

We talked about this a little, but just wanted to add a note as a reminder -- would we need a way to indicate that a field is not able to be used as a filter? If so, maybe we can add additional metadata to the tag to note if that column is a legal filter or not.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 15, 2019

Author Contributor

I think we'll need to either flavor the struct tags more or add some authorization checks that do something similar. I don't think this matters for the office user, but maybe we can test it out on a field?

This comment has been minimized.

Copy link
@jim

jim May 16, 2019

Contributor

What is the scenario where we'd need to disallow a field from being used to query? I worry a little about adding too much functionality to the query builder, at least in the beginning.

func getDBColumn(t reflect.Type, field string) (string, bool) {
for i := 0; i < t.NumField(); i++ {
dbTag, ok := t.Field(i).Tag.Lookup("db")
if ok && dbTag == field {

This comment has been minimized.

Copy link
@reggieriser

reggieriser May 13, 2019

Contributor

We talked about this a little, but just wanted to add a note as a reminder -- would we need a way to indicate that a field is not able to be used as a filter? If so, maybe we can add additional metadata to the tag to note if that column is a legal filter or not.

}

// FetchOne fetches a single model record using pop's First method
func (p *PopQueryBuilder) FetchOne(model interface{}, field string, value interface{}) error {

This comment has been minimized.

Copy link
@reggieriser

reggieriser May 13, 2019

Contributor

Any reason why FetchOne shouldn't take multiple filters like the FetchMany? The two methods can probably share the core filter-processing logic and just differ in taking/returning a model vs. a slice.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 15, 2019

Author Contributor

It was the first one I wrote and modeled it off FetchByID. There's no reason we can't expand it though. It should probably follow the same pattern. We can even just have it be a shorthand for calling FetchMany().First() with some checks. Although on a SQL level I imagine pop is using LIMIT

This comment has been minimized.

Copy link
@jim

jim May 16, 2019

Contributor

I think it would be nice for there to be a unified interface between FetchOne and FetchMany, although I can see that if a single field is being checked most often a shortcut could be nice.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 16, 2019

Author Contributor

I'll see how I can rework some of this in a followup commit.

@macrael
Copy link
Contributor

left a comment

I am very pleased with this move towards wrapping Pop in an interface that we define and control. I think that implementing this across some more services will be very illuminating and will help define exactly what the right interface should be. A couple use cases I'm curious about:

  1. Eager Loading. This is a part of Pop we use a lot, we need to figure out how/if to abstract that out
  2. Pagination. This will require some sort of where clause based on date or count instead of just '='

I think that the interface work is very nicely done, it makes good sense how the three different layers work together.

return &popQueryBuilder{db}
}

type InvalidInputError struct {

This comment has been minimized.

Copy link
@macrael

macrael May 15, 2019

Contributor

In my project, I've been defining the errors in top-level domain, along with the interfaces that my services implement. That way the package can be completely mocked out if need be without importing it.


func (o officeUserFetcher) FetchOfficeUser(field string, value interface{}) (models.OfficeUser, error) {
var officeUser models.OfficeUser
error := o.builder.FetchOne(&officeUser, field, value)

This comment has been minimized.

Copy link
@macrael

macrael May 15, 2019

Contributor

That's right, and I like that this function does return a model, that way we keep the reflection stuff contained a bit.

if !ok {
invalidFields = append(invalidFields, field)
}
columnQuery := fmt.Sprintf("%s = ?", column)

This comment has been minimized.

Copy link
@macrael

macrael May 15, 2019

Contributor

I like the ability to pass multiple filters in, but I wonder if in the future we are going to want more comparisons than "=" here, in the future. I don't think it's something we should add until we need, but this is where I fear that our query builder could one day become more complicated than just writing the SQL by hand for every query.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 15, 2019

Author Contributor

It looks like GitHub marked one of my original comments about this as outdated: #2120 (comment)

I think we'll need some sort of Comparator enum.

To the latter point, I don't think there's anything stopping the query builder from evolving into raw sql. Somehow the handler params need to be translated into sql though. The nice part about interfacing it away, is that we can do whatever we want in the implementation.

This comment has been minimized.

Copy link
@jim

jim May 16, 2019

Contributor

I like the flexibility of using an interface{} as the map values, but I dislike that it doesn't help users of this function know what to pass in. It would be nice to develop this a bit further so our development tooling can help us out a bit.

If we used a specific interface, we could then have better autocomplete and also provide more options for queries:

type Filter interface {
  SQL() string
}

We could then have helper functions defined for folks to use:

// qb.Contains returns a struct that implements SQL()
var users []*User
err := qb.FetchMany(users, map[string]queryBuilder.Filter{
    "email": qb.Contains("gmail.com"),
}

We could also get rid of the map by moving the column name into the filter and using variadic functions:

var users []*User
err := qb.FetchMany(users, qb.Contains("email", "gmail.com"), qb.Equal("active", true))

Another idea is to make the filters type-specific to remove additional uses of interface{} in their definitions, which has an additional side effect of forcing type conversion to happen in Go land and not in the database:

var users []*User
err := qb.FetchMany(users, qb.StringContains("email", "gmail.com"), qb.BoolEqual("active", true))

I'm not sure how far to go in this regard (I think sort, etc. could be incorporated, but it will definitely break down at some point), and I have nothing against interface{} generally, but I would love for us to provide a better API than Pop does if it is possible to do so without a ton of additional work.

This comment has been minimized.

Copy link
@kahlouie

kahlouie May 16, 2019

Contributor

I like @jim's suggestion here. It also prevents us from abstracting too much, which allows for us to fully consider the impact of the queries that we can make in the queries we make from SM, TSP, and Office apps.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 16, 2019

Author Contributor

The filters definitely have to be thrown into a struct at the very least. I'll see what I can do here. I worry about expanding the exported functions on the query builder too much. The nice thing about exposing one function is that the dependency surface is much smaller. It goes hand in hand with Go's one function interface pattern. Building up the struct will probably be a decent tradeoff.

This comment has been minimized.

Copy link
@macrael

macrael May 16, 2019

Contributor

I can't tell if you mean that the filters need to be in a struct because it's the only technical option or because it only makes sense that way. I do think that it would be technically possible to use variadic args to implement the interface @jim sketched out.

"github.com/transcom/mymove/pkg/services"
)

type officeUserQueryBuilder interface {

This comment has been minimized.

Copy link
@macrael

macrael May 15, 2019

Contributor

I think I like not exporting until there is a clear reason to export.

Re: where to define your interfaces, I think that it can make sense to define interfaces outside of the package you are implementing them in for mocking purposes. I like this article: https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1 where all "domain models" are defined in the top level package for your app, including service interfaces. That way you can mock services without importing them at all.

That said, I'm not sure that's an important consideration in this case.

suite.Equal(id, officeUser.ID)
})

suite.T().Run("if there is an error, we get it with zero office user", func(t *testing.T) {

This comment has been minimized.

Copy link
@macrael

macrael May 15, 2019

Contributor

The only trouble with these tests is that they are no longer actually testing that the database is configured correctly. Until performance becomes a problem I tend to think that writing tests that actually hit the database is more useful than mocking it out. Somewhere there should be a test that a user with all the required data can be saved to the database, with these layers of indirection it's not clear where that test should live.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 15, 2019

Author Contributor

Performance has been an issue with us, especially how we truncate the DB before tests (I think that could be fixed by itself though). I was alluding to this here without outright saying it: #2120 (comment). We'll probably want some golden path integration tests to make sure everything is wired up correctly. I think we can separate these into their own testing package, rather than conflating them with handlers, models, services, etc.

)

// Logger is an interface that describes the logging requirements of this package.
type Logger interface {

This comment has been minimized.

Copy link
@macrael

macrael May 15, 2019

Contributor

Is this a pattern being used across mil move? Since it's only being used by the tests, it feels a bit overkill to me to not just use a zap logger.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 15, 2019

Author Contributor

It has become a pattern: #1775

@codecov

This comment has been minimized.

Copy link

commented May 24, 2019

Codecov Report

Merging #2120 into master will increase coverage by 0.31%.
The diff coverage is 95.15%.

@@            Coverage Diff             @@
##           master    #2120      +/-   ##
==========================================
+ Coverage   58.67%   58.98%   +0.31%     
==========================================
  Files         245      250       +5     
  Lines       13960    14077     +117     
==========================================
+ Hits         8191     8303     +112     
- Misses       4767     4773       +6     
+ Partials     1002     1001       -1
value string
}

func (f testQueryFilter) Column() string {

This comment has been minimized.

Copy link
@garrettqmartin8

garrettqmartin8 May 28, 2019

Contributor

I'm curious about the difference between this and defining public fields on the struct.

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

What actually gets passed to the handler and service is the QueryFilter interface. This allows us to change up the implementation of the QueryFilter without changing all of the places where it's been passed as a dependency. I was playing with this interface for a bit, seeing if I could parse the DB query string from within it and that's part of why it was an interface at first.

I could be convinced otherwise, but I tend to favor using an interface when it's being passed over multiple packages like this. That way, we can see any inconsistencies where the packages meet each other, rather than where the struct values are accessed.

@mikena-truss

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

ADR has been added, per the discussion had around this PR: #2190

}

// Handle retrieves a list of office users
func (h IndexOfficeUsersHandler) Handle(params officeuserop.IndexOfficeUsersParams) middleware.Responder {
return officeuserop.NewIndexOfficeUsersOK()
// Here is where NewQueryFilter will be used to create Filters from the 'filter' query param
queryFilters := []services.QueryFilter{

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

So, I realized that in order to utilize the type benefits in the DB connector, these column values still need a type. So the param to a Where query may be interface{}, but passing a date in string format or a date in time format do different things. Which makes sense.

The consequence, is that we'll need to either:

  1. explicitly list types when creating filters
  2. marshall the data using reflection and type info in the model

2 seems like a totally reasonable thing to do. In fact, it's what all the swagger generated code is doing anyways. The only difference here is that we hit the limits of Open API specs and have to do some of the work ourselves.

More info on the OpenAPI issues here: OAI/OpenAPI-Specification#1706

suite.IsType(&officeuserop.IndexOfficeUsersOK{}, response)
okResponse := response.(*officeuserop.IndexOfficeUsersOK)
suite.Len(okResponse.Payload, 1)
suite.Equal(uuidString, okResponse.Payload[0].ID.String())

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

let me circle around and remove the uuidString in favor of id

params := officeuserop.IndexOfficeUsersParams{
HTTPRequest: req,
}
// test that everything is wired up

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

This is really an integration test, not a handler unit test. I don't want to try and solve every problem in this PR, however I'm going to circle back later with an ADR about how we might want to handle backend integration tests. Now that we've decoupled our code, this is more necessary.

)

// allowed comparators for this query builder implementation
const equals = "="

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

If someone has a fancier enum way of doing this, please let me know. I messed around with it a bit, but didn't love with what I ended up with. Essentially an enum with a .Comparator method that was a switch statement returning the symbols.

}

// Lookup to check if a specific string is inside the db field tags of the type
func getDBColumn(t reflect.Type, field string) (string, bool) {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

The other option to the above marshalling discussion is we just do a type conversion here. It'd be cheap/easy, however seems like the wrong place for that.

// Will return error if model is not pointer to struct
func (p *Builder) FetchOne(model interface{}, filters []services.QueryFilter) error {
t := reflect.TypeOf(model)
if t.Kind() != reflect.Ptr {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

I've added type checks here. Note that we currently don't have these checks when using pop directly. We just test to make sure this doesn't happen. It didn't seem to hurt to give that info though, so we don't panic.

}

// OfficeUserListFetcher is the exported interface for fetching multiple office users
type OfficeUserListFetcher interface {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

I could use some insight from others here. Add, delete, modify, and get are all going to be very similar to the main apps. However, the list/query features may vary from the existing specs. Do we call this OfficeUserAdminListFetcher or something similar to avoid collisions? This is more apparent when you look at something like Shipments that will already have services built.

@@ -0,0 +1,13 @@
package services

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

Note this isn't following the service object pattern, but this seems ok in services. It's a dependency used for handlers/services to communicate.

Originally, I had it in its own package. However, that mean that the QueryFilter was defined there and in services so that services could pass the filters to the builder. Interfaces with the same methods don't satisfy each other though and it caused a leaky abstraction of sorts for them to communicate.

@mikena-truss mikena-truss removed the wip label May 29, 2019

// IndexOfficeUsersHandler returns a list of office users via GET /office_users
type IndexOfficeUsersHandler struct {
handlers.HandlerContext
services.NewQueryFilter
services.OfficeUserListFetcher
}

// Handle retrieves a list of office users
func (h IndexOfficeUsersHandler) Handle(params officeuserop.IndexOfficeUsersParams) middleware.Responder {

This comment has been minimized.

Copy link
@mikena-truss

mikena-truss May 29, 2019

Author Contributor

This PR needs to be broken up a bit, so we should handle API marshalling into this handler here: https://www.pivotaltracker.com/story/show/164884924

)

type officeUserListQueryBuilder interface {
FetchMany(model interface{}, fitlers []services.QueryFilter) error

This comment has been minimized.

Copy link
@garrettqmartin8

garrettqmartin8 May 30, 2019

Contributor
Suggested change
FetchMany(model interface{}, fitlers []services.QueryFilter) error
FetchMany(model interface{}, filters []services.QueryFilter) error
)

type officeUserQueryBuilder interface {
FetchOne(model interface{}, fitlers []services.QueryFilter) error

This comment has been minimized.

Copy link
@garrettqmartin8

garrettqmartin8 May 30, 2019

Contributor
Suggested change
FetchOne(model interface{}, fitlers []services.QueryFilter) error
FetchOne(model interface{}, filters []services.QueryFilter) error
@garrettqmartin8
Copy link
Contributor

left a comment

Just a couple of typo comments, but otherwise this looks good to go. Very excited to use this.

@reggieriser
Copy link
Contributor

left a comment

I noted a few typos/nitpicks, but this looks like a great foundation for us to build upon!

}

func (suite *UserServiceSuite) TestFetchOfficeUserList() {
suite.T().Run("if the user it fetched, it should be returned", func(t *testing.T) {

This comment has been minimized.

Copy link
@reggieriser

reggieriser Jun 3, 2019

Contributor

Nitpick: "If the user is fetched..." (another similar typo in the other test).

suite.Empty(actualUsers)
})

suite.T().Run("fails with invalid column", func(t *testing.T) {

This comment has been minimized.

Copy link
@reggieriser

reggieriser Jun 3, 2019

Contributor

Nitpick: "fails with invalid comparator"

}
t = t.Elem()
if t.Kind() != reflect.Struct {
return errors.New("Model should be pointer to slice of structs")

This comment has been minimized.

Copy link
@reggieriser

reggieriser Jun 3, 2019

Contributor

Maybe put this error message (or error instance) in a constant so we don't repeat it?

@chrisgilmerproj
Copy link
Contributor

left a comment

🚀 - This is going to be slick.

@mikena-truss mikena-truss merged commit 5e25065 into master Jun 3, 2019

19 checks passed

ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tasks Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_api Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details
codecov/patch 95.15% of diff hit (target 58.67%)
Details
codecov/project/go 58.8% (+0.32%) compared to 6c8944e
Details

@mikena-truss mikena-truss deleted the mw-add-query-interface branch Jun 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.