Skip to content

Commit

Permalink
PTP API: Address review comments
Browse files Browse the repository at this point in the history 
License: MIT
Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
  • Loading branch information
@magik6k
magik6k committed Jun 7, 2017
1 parent 104268f commit e8a5bb3
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 100 deletions.
167 changes: 73 additions & 94 deletions core/commands/ptp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@ Note: this command is experimental and subject to change as usecases and APIs ar
},

Subcommands: map[string]*cmds.Command{
"ls": ptpLsCmd,
"streams": ptpStreamsCmd,
"dial": ptpDialCmd,
"listen": ptpListenCmd,
"close": ptpCloseCmd,
"ls": ptpLsCmd,
"streams": ptpStreamsCmd,
"dial": ptpDialCmd,
"listen": ptpListenCmd,
"close": ptpCloseCmd,
"closeStream": ptpCloseStreamCmd,
},
}

Expand All @@ -67,23 +68,13 @@ var ptpLsCmd = &cmds.Command{
cmds.BoolOption("headers", "v", "Print table headers (HandlerID, Protocol, Local, Remote).").Default(false),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := req.InvocContext().GetNode()
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

err = checkEnabled(n)
n, err := getNode(req)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

if !n.OnlineMode() {
res.SetError(errNotOnline, cmds.ErrClient)
return
}

output := &PTPLsOutput{}

for _, listener := range n.PTP.Listeners.Listeners {
Expand Down Expand Up @@ -124,23 +115,12 @@ var ptpStreamsCmd = &cmds.Command{
cmds.BoolOption("headers", "v", "Print table headers (HagndlerID, Protocol, Local, Remote).").Default(false),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := req.InvocContext().GetNode()
n, err := getNode(req)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

err = checkEnabled(n)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

if !n.OnlineMode() {
res.SetError(errNotOnline, cmds.ErrClient)
return
}

output := &PTPStreamsOutput{}

for _, s := range n.PTP.Streams.Streams {
Expand Down Expand Up @@ -194,23 +174,12 @@ Note that the connections originate from the ipfs daemon process.
cmds.StringArg("Address", true, false, "Request handling application address."),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := req.InvocContext().GetNode()
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

err = checkEnabled(n)
n, err := getNode(req)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

if !n.OnlineMode() {
res.SetError(errNotOnline, cmds.ErrClient)
return
}

proto := "/ptp/" + req.Arguments()[0]
if n.PTP.CheckProtoExists(proto) {
res.SetError(errors.New("protocol handler already registered"), cmds.ErrNormal)
Expand Down Expand Up @@ -255,23 +224,12 @@ transparently connect to a p2p service.
cmds.StringArg("BindAddress", false, false, "Address to listen for connection/s (default: /ip4/127.0.0.1/tcp/0)."),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := req.InvocContext().GetNode()
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

err = checkEnabled(n)
n, err := getNode(req)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

if !n.OnlineMode() {
res.SetError(errNotOnline, cmds.ErrClient)
return
}

addr, peer, err := ParsePeerParam(req.Arguments()[0])
if err != nil {
res.SetError(err, cmds.ErrNormal)
Expand Down Expand Up @@ -306,87 +264,108 @@ transparently connect to a p2p service.

var ptpCloseCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "Closes an active p2p stream or listener.",
Tagline: "Close active p2p listener.",
},
Arguments: []cmds.Argument{
cmds.StringArg("Identifier", false, false, "Stream HandlerID or p2p listener protocol"),
cmds.StringArg("Protocol", false, false, "P2P listener protocol"),
},
Options: []cmds.Option{
cmds.BoolOption("all", "a", "Close all streams and listeners.").Default(false),
cmds.BoolOption("all", "a", "Close all listeners.").Default(false),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := req.InvocContext().GetNode()
n, err := getNode(req)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

err = checkEnabled(n)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
closeAll, _, _ := req.Option("all").Bool()
var proto string

if !closeAll {
if len(req.Arguments()) == 0 {
res.SetError(errors.New("no protocol name specified"), cmds.ErrNormal)
return
}

proto = "/ptp/" + req.Arguments()[0]
}

if !n.OnlineMode() {
res.SetError(errNotOnline, cmds.ErrClient)
for _, listener := range n.PTP.Listeners.Listeners {
if !closeAll && listener.Protocol != proto {
continue
}
listener.Close()
if !closeAll {
break
}
}
},
}

var ptpCloseStreamCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "Close active p2p stream.",
},
Arguments: []cmds.Argument{
cmds.StringArg("HandlerID", false, false, "Stream HandlerID"),
},
Options: []cmds.Option{
cmds.BoolOption("all", "a", "Close all streams.").Default(false),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := getNode(req)
if err != nil {
res.SetError(err, cmds.ErrNormal)
return
}

closeAll, _, _ := req.Option("all").Bool()

var proto string
var handlerID uint64

useHandlerID := false

if !closeAll {
if len(req.Arguments()) == 0 {
res.SetError(errors.New("no handlerID nor listener protocol specified"), cmds.ErrNormal)
res.SetError(errors.New("no HandlerID specified"), cmds.ErrNormal)
return
}

handlerID, err = strconv.ParseUint(req.Arguments()[0], 10, 64)
if err != nil {
proto = "/ptp/" + req.Arguments()[0]
} else {
useHandlerID = true
res.SetError(err, cmds.ErrNormal)
return
}
}

if closeAll || useHandlerID {
for _, stream := range n.PTP.Streams.Streams {
if !closeAll && handlerID != stream.HandlerID {
continue
}
stream.Close()
if !closeAll {
break
}
for _, stream := range n.PTP.Streams.Streams {
if !closeAll && handlerID != stream.HandlerID {
continue
}
}

if closeAll || !useHandlerID {
for _, listener := range n.PTP.Listeners.Listeners {
if !closeAll && listener.Protocol != proto {
continue
}
listener.Close()
if !closeAll {
break
}
stream.Close()
if !closeAll {
break
}
}
},
}

func checkEnabled(n *core.IpfsNode) error {
func getNode(req cmds.Request) (*core.IpfsNode, error) {
n, err := req.InvocContext().GetNode()
if err != nil {
return nil, err
}

config, err := n.Repo.Config()
if err != nil {
return err
return nil, err
}

if !config.Experimental.Libp2pStreamMounting {
return errors.New("libp2p stream mounting not enabled")
return nil, errors.New("libp2p stream mounting not enabled")
}

if !n.OnlineMode() {
return nil, errNotOnline
}
return nil

return n, nil
}
7 changes: 5 additions & 2 deletions test/dependencies/ma-pipe-unidir/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ func app() int {
mode := args[0]
addr := args[1]

if mode != "send" && mode != "recv" {
fmt.Print(USAGE)
return 1
}

if len(opts.PidFile) > 0 {
data := []byte(strconv.Itoa(os.Getpid()))
err := ioutil.WriteFile(opts.PidFile, data, 0644)
Expand Down Expand Up @@ -80,8 +85,6 @@ func app() int {
case "send":
io.Copy(conn, os.Stdin)
default:
//TODO: a bit late
fmt.Print(USAGE)
return 1
}
return 0
Expand Down
19 changes: 15 additions & 4 deletions test/sharness/t0180-ptp.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ test_expect_success "'ipfs ptp streams' output looks good" '
test_cmp expected actual
'

test_expect_success "'ipfs ptp close' closes stream" '
ipfsi 0 ptp close 2 &&
test_expect_success "'ipfs ptp closeStream' closes stream" '
ipfsi 0 ptp closeStream 2 &&
ipfsi 0 ptp streams > actual &&
[ ! -f listener.pid ] && [ ! -f client.pid ] &&
test_must_be_empty actual
Expand Down Expand Up @@ -132,11 +132,22 @@ test_expect_success "'ipfs ptp streams' succeeds(2)" '
test_cmp expected actual
'

test_expect_success "'ipfs ptp close -a' closes streams and app handlers" '
test_expect_success "'ipfs ptp close -a' closes app handlers" '
ipfsi 0 ptp close -a &&
ipfsi 0 ptp ls > actual &&
test_must_be_empty actual
'

test_expect_success "'ipfs ptp closeStream -a' closes streams" '
ipfsi 0 ptp closeStream -a &&
ipfsi 0 ptp streams > actual &&
[ ! -f listener.pid ] && [ ! -f client.pid ] &&
test_must_be_empty actual &&
test_must_be_empty actual
'

test_expect_success "'ipfs ptp close' closes app numeric handlers" '
ipfsi 0 ptp listen 1234 /ip4/127.0.0.1/tcp/10101 &&
ipfsi 0 ptp close 1234 &&
ipfsi 0 ptp ls > actual &&
test_must_be_empty actual
'
Expand Down

0 comments on commit e8a5bb3

Please sign in to comment.