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

Proposal: Allow exported functions with receivers other than *Service #248

Open
eriktate opened this issue Dec 15, 2017 · 0 comments
Open

Comments

@eriktate
Copy link
Contributor

eriktate commented Dec 15, 2017

Suppose you have a handlers.go file that looks something like this:

package handlers

import (
	"context"
	"database/sql"

	pb "github.com/eriktate/thing"
)

// NewService returns a naïve, stateless implementation of Service.
func NewService() pb.ContextServer {
	db, err := connectToProdDB()
	if err != nil {
		panic("Failed to connect to database")
	}

	return BuildService(db)
}

// BuildService returns a dependency injected thingService. Useful for unit testing.
func BuildService(db *sql.DB) thingService {
	return thingService{db: db}
}

type thingService struct {
	db *sql.DB
}

// DoThing does a thing with the sql.DB pointer housed in thingService.
func (s thingService) DoThing(ctx context.Context, in *pb.ThingPayload) (*pb.ThingResponse, error) {
	return doSomethingWithDatbase(s.db)
}

This type of pattern doesn't seem to be possible with truss at the moment, as the BuildService function will be stripped on the next truss run. A workaround might be to add a file in the same package and define the BuildService function there, but it feels like it should be included with the service definition.

A solution I've found is a slight change to the pruneDecls function in gengokit/handlers/handlers.go:

func (m methodMap) pruneDecls(decls []ast.Decl, svcName string, allowExports bool) []ast.Decl {
	var newDecls []ast.Decl
	for _, d := range decls {
		switch x := d.(type) {
		case *ast.FuncDecl:
			name := x.Name.Name
			// Special case NewService and ignore unexported
			if name == ignoredFunc || !ast.IsExported(name) {
				log.WithField("Func", name).
					Debug("Ignoring")
				newDecls = append(newDecls, x)
				continue
			}
			if ok := isValidFunc(x, m, svcName); ok == true {
				updateParams(x, m[name])
				updateResults(x, m[name])
				newDecls = append(newDecls, x)
				delete(m, name)
			} else if allowExports {
				if ok := isIgnored(x, m, svcName); ok == true {
					newDecls = append(newDecls, x)
				}
			}

		default:
			newDecls = append(newDecls, d)
		}

	}
	return newDecls
}

// potential new function
func isIgnored(f *ast.FuncDecl, m methodMap, svcName string) bool {
	name := f.Name.String()
	rName := recvTypeToString(f.Recv)
	if rName != svcName+"Service" {
		log.WithField("Func", name).WithField("Receiver", rName).
			Info("Exported func is not on service receiver; ignoring")
		return true
	}
	return false
}

The allowExports bool in the function signature would allow for this functionality to be placed behind a flag. This change just checks if the exported function declaration has the service struct as a receiver. If it DOES have the service struct as a receiver than it will be handled normally. If it DOESN'T have the service struct as a receiver, than it will ignore that function when pruning.

Thoughts about adding this sort of functionality? Or the proposed solution? If the desired workflow for something like this is to use a different file than handlers.go that's fine too, just wondering if anyone else would benefit from something like this.

@eriktate eriktate changed the title Proposal: Allow exported functions with receivers other than *Server Proposal: Allow exported functions with receivers other than *Service Dec 15, 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

No branches or pull requests

1 participant