Skip to content

Commit

Permalink
revel/revel#1057 code improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeevatkm
jeevatkm committed Jun 9, 2016
1 parent 514b078 commit 5a57eaa
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 71 deletions.
10 changes: 7 additions & 3 deletions harness/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ type App struct {
cmd AppCmd // The last cmd returned.
}

// NewApp returns app instance with binary path in it
func NewApp(binPath string) *App {
return &App{BinaryPath: binPath}
}

// Return a command to run the app server using the current configuration.
// Cmd returns a command to run the app server using the current configuration.
func (a *App) Cmd() AppCmd {
a.cmd = NewAppCmd(a.BinaryPath, a.Port)
return a.cmd
Expand All @@ -41,6 +42,7 @@ type AppCmd struct {
*exec.Cmd
}

// NewAppCmd returns the AppCmd with parameters initialized for running app
func NewAppCmd(binPath string, port int) AppCmd {
cmd := exec.Command(binPath,
fmt.Sprintf("-port=%d", port),
Expand Down Expand Up @@ -70,6 +72,8 @@ func (cmd AppCmd) Start() error {
case <-listeningWriter.notifyReady:
return nil
}

// TODO remove this unreachable code and document it
panic("Impossible")
}

Expand All @@ -81,7 +85,7 @@ func (cmd AppCmd) Run() {
}
}

// Terminate the app server if it's running.
// Kill terminates the app server if it's running.
func (cmd AppCmd) Kill() {
if cmd.Cmd != nil && (cmd.ProcessState == nil || !cmd.ProcessState.Exited()) {
revel.TRACE.Println("Killing revel server pid", cmd.Process.Pid)
Expand All @@ -96,7 +100,7 @@ func (cmd AppCmd) Kill() {
func (cmd AppCmd) waitChan() <-chan struct{} {
ch := make(chan struct{}, 1)
go func() {
cmd.Wait()
_ = cmd.Wait()
ch <- struct{}{}
}()
return ch
Expand Down
28 changes: 19 additions & 9 deletions harness/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ func Build(buildFlags ...string) (app *App, compileError *revel.Error) {
"ImportPaths": calcImportAliases(sourceInfo),
"TestSuites": sourceInfo.TestSuites(),
}
genSource("tmp", "main.go", MAIN, templateArgs)
genSource("routes", "routes.go", ROUTES, templateArgs)
genSource("tmp", "main.go", RevelMainTemplate, templateArgs)
genSource("routes", "routes.go", RevelRoutesTemplate, templateArgs)

// Read build config.
buildTags := revel.Config.StringDefault("build.tags", "")
Expand Down Expand Up @@ -126,6 +126,8 @@ func Build(buildFlags ...string) (app *App, compileError *revel.Error) {

// Success getting the import, attempt to build again.
}

// TODO remove this unreachable code and document it
revel.ERROR.Fatalf("Not reachable")
return nil, nil
}
Expand Down Expand Up @@ -179,7 +181,10 @@ func cleanDir(dir string) {
revel.ERROR.Println("Failed to clean dir:", err)
}
} else {
defer f.Close()
defer func() {
_ = f.Close()
}()

infos, err := f.Readdir(0)
if err != nil {
if !os.IsNotExist(err) {
Expand Down Expand Up @@ -221,12 +226,14 @@ func genSource(dir, filename, templateSource string, args map[string]interface{}

// Create the file
file, err := os.Create(filepath.Join(tmpPath, filename))
defer file.Close()
if err != nil {
revel.ERROR.Fatalf("Failed to create file: %v", err)
}
_, err = file.WriteString(sourceCode)
if err != nil {
defer func() {
_ = file.Close()
}()

if _, err = file.WriteString(sourceCode); err != nil {
revel.ERROR.Fatalf("Failed to write to file: %v", err)
}
}
Expand Down Expand Up @@ -345,7 +352,8 @@ func newCompileError(output []byte) *revel.Error {
return compileError
}

const MAIN = `// GENERATED CODE - DO NOT EDIT
// RevelMainTemplate template for app/tmp/main.go
const RevelMainTemplate = `// GENERATED CODE - DO NOT EDIT
package main
import (
Expand Down Expand Up @@ -399,7 +407,9 @@ func main() {
revel.Run(*port)
}
`
const ROUTES = `// GENERATED CODE - DO NOT EDIT

// RevelRoutesTemplate template for app/conf/routes
const RevelRoutesTemplate = `// GENERATED CODE - DO NOT EDIT
package routes
import "github.com/revel/revel"
Expand All @@ -415,7 +425,7 @@ func (_ t{{$c.StructName}}) {{.Name}}({{range .Args}}
args := make(map[string]string)
{{range .Args}}
revel.Unbind(args, "{{.Name}}", {{.Name}}){{end}}
return revel.MainRouter.Reverse("{{$c.StructName}}.{{.Name}}", args).Url
return revel.MainRouter.Reverse("{{$c.StructName}}.{{.Name}}", args).URL
}
{{end}}
{{end}}
Expand Down
33 changes: 22 additions & 11 deletions harness/harness.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,14 @@ func NewHarness() *Harness {
// Prefer the app's views/errors directory, and fall back to the stock error pages.
revel.MainTemplateLoader = revel.NewTemplateLoader(
[]string{filepath.Join(revel.RevelPath, "templates")})
revel.MainTemplateLoader.Refresh()
if err := revel.MainTemplateLoader.Refresh(); err != nil {
revel.ERROR.Println(err)
}

addr := revel.HttpAddr
addr := revel.HTTPAddr
port := revel.Config.IntDefault("harness.port", 0)
scheme := "http"
if revel.HttpSsl {
if revel.HTTPSsl {
scheme = "https"
}

Expand All @@ -110,7 +112,7 @@ func NewHarness() *Harness {
proxy: httputil.NewSingleHostReverseProxy(serverURL),
}

if revel.HttpSsl {
if revel.HTTPSsl {
harness.proxy.Transport = &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
}
Expand Down Expand Up @@ -166,13 +168,16 @@ func (h *Harness) Run() {
watcher.Listen(h, paths...)

go func() {
addr := fmt.Sprintf("%s:%d", revel.HttpAddr, revel.HttpPort)
addr := fmt.Sprintf("%s:%d", revel.HTTPAddr, revel.HTTPPort)
revel.INFO.Printf("Listening on %s", addr)

var err error
if revel.HttpSsl {
err = http.ListenAndServeTLS(addr, revel.HttpSslCert,
revel.HttpSslKey, h)
if revel.HTTPSsl {
err = http.ListenAndServeTLS(
addr,
revel.HTTPSslCert,
revel.HTTPSslKey,
h)
} else {
err = http.ListenAndServe(addr, h)
}
Expand Down Expand Up @@ -213,7 +218,7 @@ func proxyWebsocket(w http.ResponseWriter, r *http.Request, host string) {
d net.Conn
err error
)
if revel.HttpSsl {
if revel.HTTPSsl {
// since this proxy isn't used in production,
// it's OK to set InsecureSkipVerify to true
// no need to add another configuration option.
Expand All @@ -236,8 +241,14 @@ func proxyWebsocket(w http.ResponseWriter, r *http.Request, host string) {
revel.ERROR.Printf("Hijack error: %v", err)
return
}
defer nc.Close()
defer d.Close()
defer func() {
if err = nc.Close(); err != nil {
revel.ERROR.Println(err)
}
if err = d.Close(); err != nil {
revel.ERROR.Println(err)
}
}()

err = r.Write(d)
if err != nil {
Expand Down
64 changes: 37 additions & 27 deletions harness/reflect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ type methodCall struct {
Names []string
}

// MethodSpec holds the information of one Method
type MethodSpec struct {
Name string // Name of the method, e.g. "Index"
Args []*MethodArg // Argument descriptors
RenderCalls []*methodCall // Descriptions of Render() invocations from this Method.
}

// MethodArg holds the information of one argument
type MethodArg struct {
Name string // Name of the argument.
TypeExpr TypeExpr // The name of the type, e.g. "int", "*pkg.UserType"
Expand All @@ -80,8 +82,9 @@ type embeddedTypeName struct {
// receiver.
type methodMap map[string][]*MethodSpec

// Parse the app controllers directory and return a list of the controller types found.
// Returns a CompileError if the parsing fails.
// ProcessSource parses the app controllers directory and
// returns a list of the controller types found.
// Otherwise CompileError if the parsing fails.
func ProcessSource(roots []string) (*SourceInfo, *revel.Error) {
var (
srcInfo *SourceInfo
Expand Down Expand Up @@ -120,7 +123,7 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) {
}, 0)
if err != nil {
if errList, ok := err.(scanner.ErrorList); ok {
var pos token.Position = errList[0].Pos
var pos = errList[0].Pos
compileError = &revel.Error{
SourceType: ".go source",
Title: "Go Compilation Error",
Expand All @@ -139,6 +142,8 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) {

return compileError
}

// This is exception, err alredy checked above. Here just a print
ast.Print(nil, err)
log.Fatalf("Failed to parse dir: %s", err)
}
Expand Down Expand Up @@ -490,7 +495,7 @@ func appendAction(fset *token.FileSet, mm methodMap, decl ast.Decl, pkgImportPat
})

var recvTypeName string
var recvType ast.Expr = funcDecl.Recv.List[0].Type
var recvType = funcDecl.Recv.List[0].Type
if recvStarType, ok := recvType.(*ast.StarExpr); ok {
recvTypeName = recvStarType.X.(*ast.Ident).Name
} else {
Expand Down Expand Up @@ -673,13 +678,17 @@ func (s *SourceInfo) TypesThatEmbed(targetType string) (filtered []*TypeInfo) {
return
}

// ControllerSpecs returns the all the contollers that embeds
// `revel.Controller`
func (s *SourceInfo) ControllerSpecs() []*TypeInfo {
if s.controllerSpecs == nil {
s.controllerSpecs = s.TypesThatEmbed(revel.RevelImportPath + ".Controller")
}
return s.controllerSpecs
}

// TestSuites returns the all the Application tests that embeds
// `testing.TestSuite`
func (s *SourceInfo) TestSuites() []*TypeInfo {
if s.testSuites == nil {
s.testSuites = s.TypesThatEmbed(revel.RevelImportPath + "/testing.TestSuite")
Expand All @@ -705,7 +714,7 @@ func (e TypeExpr) TypeName(pkgOverride string) string {
return e.Expr[:e.pkgIndex] + pkgName + "." + e.Expr[e.pkgIndex:]
}

// This returns the syntactic expression for referencing this type in Go.
// NewTypeExpr returns the syntactic expression for referencing this type in Go.
func NewTypeExpr(pkgName string, expr ast.Expr) TypeExpr {
switch t := expr.(type) {
case *ast.Ident:
Expand All @@ -731,31 +740,32 @@ func NewTypeExpr(pkgName string, expr ast.Expr) TypeExpr {
return TypeExpr{Valid: false}
}

var _BUILTIN_TYPES = map[string]struct{}{
"bool": struct{}{},
"byte": struct{}{},
"complex128": struct{}{},
"complex64": struct{}{},
"error": struct{}{},
"float32": struct{}{},
"float64": struct{}{},
"int": struct{}{},
"int16": struct{}{},
"int32": struct{}{},
"int64": struct{}{},
"int8": struct{}{},
"rune": struct{}{},
"string": struct{}{},
"uint": struct{}{},
"uint16": struct{}{},
"uint32": struct{}{},
"uint64": struct{}{},
"uint8": struct{}{},
"uintptr": struct{}{},
var builtInTypes = map[string]struct{}{
"bool": {},
"byte": {},
"complex128": {},
"complex64": {},
"error": {},
"float32": {},
"float64": {},
"int": {},
"int16": {},
"int32": {},
"int64": {},
"int8": {},
"rune": {},
"string": {},
"uint": {},
"uint16": {},
"uint32": {},
"uint64": {},
"uint8": {},
"uintptr": {},
}

// IsBuiltinType checks the given type is built-in types of Go
func IsBuiltinType(name string) bool {
_, ok := _BUILTIN_TYPES[name]
_, ok := builtInTypes[name]
return ok
}

Expand Down
36 changes: 18 additions & 18 deletions harness/reflect_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,18 @@ func TestGetValidationKeys(t *testing.T) {
}

var TypeExprs = map[string]TypeExpr{
"int": TypeExpr{"int", "", 0, true},
"*int": TypeExpr{"*int", "", 1, true},
"[]int": TypeExpr{"[]int", "", 2, true},
"...int": TypeExpr{"[]int", "", 2, true},
"[]*int": TypeExpr{"[]*int", "", 3, true},
"...*int": TypeExpr{"[]*int", "", 3, true},
"MyType": TypeExpr{"MyType", "pkg", 0, true},
"*MyType": TypeExpr{"*MyType", "pkg", 1, true},
"[]MyType": TypeExpr{"[]MyType", "pkg", 2, true},
"...MyType": TypeExpr{"[]MyType", "pkg", 2, true},
"[]*MyType": TypeExpr{"[]*MyType", "pkg", 3, true},
"...*MyType": TypeExpr{"[]*MyType", "pkg", 3, true},
"int": {"int", "", 0, true},
"*int": {"*int", "", 1, true},
"[]int": {"[]int", "", 2, true},
"...int": {"[]int", "", 2, true},
"[]*int": {"[]*int", "", 3, true},
"...*int": {"[]*int", "", 3, true},
"MyType": {"MyType", "pkg", 0, true},
"*MyType": {"*MyType", "pkg", 1, true},
"[]MyType": {"[]MyType", "pkg", 2, true},
"...MyType": {"[]MyType", "pkg", 2, true},
"[]*MyType": {"[]*MyType", "pkg", 3, true},
"...*MyType": {"[]*MyType", "pkg", 3, true},
}

func TestTypeExpr(t *testing.T) {
Expand All @@ -126,10 +126,10 @@ func TestTypeExpr(t *testing.T) {
}

if array {
expr = &ast.ArrayType{expr.Pos(), nil, expr}
expr = &ast.ArrayType{Lbrack: expr.Pos(), Len: nil, Elt: expr}
}
if ellipsis {
expr = &ast.Ellipsis{expr.Pos(), expr}
expr = &ast.Ellipsis{Ellipsis: expr.Pos(), Elt: expr}
}

actual := NewTypeExpr("pkg", expr)
Expand All @@ -146,11 +146,11 @@ func TestProcessBookingSource(t *testing.T) {
t.Fatal("Failed to process booking source with error:", err)
}

CONTROLLER_PKG := "github.com/revel/samples/booking/app/controllers"
controllerPackage := "github.com/revel/samples/booking/app/controllers"
expectedControllerSpecs := []*TypeInfo{
{"GorpController", CONTROLLER_PKG, "controllers", nil, nil},
{"Application", CONTROLLER_PKG, "controllers", nil, nil},
{"Hotels", CONTROLLER_PKG, "controllers", nil, nil},
{"GorpController", controllerPackage, "controllers", nil, nil},
{"Application", controllerPackage, "controllers", nil, nil},
{"Hotels", controllerPackage, "controllers", nil, nil},
}
if len(sourceInfo.ControllerSpecs()) != len(expectedControllerSpecs) {
t.Errorf("Unexpected number of controllers found. Expected %d, Found %d",
Expand Down
Loading

0 comments on commit 5a57eaa

Please sign in to comment.