From b42e7dd06aab499ffc45fe03d272bf4e844c9a91 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 24 Apr 2015 21:02:08 -0700 Subject: [PATCH 01/10] cmd/ipfs/init: Replace coreunix.Add with importer.BuildDagFromReader coreunix.Add is basically a wrapper around BuildDagFromReader that returns a string-ified key instead of a DAG node. Since we'll need the DAG node to add to the directory, converting to a key and then looking that key back up in dirbuilder.AddChild is just extra work. --- cmd/ipfs/init.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cmd/ipfs/init.go b/cmd/ipfs/init.go index 56085442619..75434199c93 100644 --- a/cmd/ipfs/init.go +++ b/cmd/ipfs/init.go @@ -10,12 +10,13 @@ import ( assets "github.com/ipfs/go-ipfs/assets" cmds "github.com/ipfs/go-ipfs/commands" core "github.com/ipfs/go-ipfs/core" - coreunix "github.com/ipfs/go-ipfs/core/coreunix" + importer "github.com/ipfs/go-ipfs/importer" + chunk "github.com/ipfs/go-ipfs/importer/chunk" + merkledag "github.com/ipfs/go-ipfs/merkledag" namesys "github.com/ipfs/go-ipfs/namesys" config "github.com/ipfs/go-ipfs/repo/config" fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo" - uio "github.com/ipfs/go-ipfs/unixfs/io" - u "github.com/ipfs/go-ipfs/util" + unixfs "github.com/ipfs/go-ipfs/unixfs" ) const nBitsForKeypairDefault = 2048 @@ -132,23 +133,24 @@ func addDefaultAssets(out io.Writer, repoRoot string) error { } defer nd.Close() - dirb := uio.NewDirectory(nd.DAG) + dir := &merkledag.Node{Data: unixfs.FolderPBData()} // add every file in the assets pkg for fname, file := range assets.Init_dir { buf := bytes.NewBufferString(file) - s, err := coreunix.Add(nd, buf) + dagNode, err := importer.BuildDagFromReader( + buf, + nd.DAG, + nd.Pinning.GetManual(), + chunk.DefaultSplitter) if err != nil { return err } - - k := u.B58KeyDecode(s) - if err := dirb.AddChild(fname, k); err != nil { + if err := dir.AddNodeLink(fname, dagNode); err != nil { return err } } - dir := dirb.GetNode() dkey, err := nd.DAG.Add(dir) if err != nil { return err From c6bf6a8c2fa3cbbac6c2b2262341a89f37fd45f1 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 27 Apr 2015 09:44:23 -0700 Subject: [PATCH 02/10] cmd/ipfswatch/main: Replace coreunix.Add with coreunix.AddR No need to convert from a path to a reader locally, when AddR will do that for us. This also future-proofs us from AddR enhancements that will pull other information about the added file from the filesystem besides the data (e.g. access mode, owner and group ID, ...). --- cmd/ipfswatch/main.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/cmd/ipfswatch/main.go b/cmd/ipfswatch/main.go index 87570e35643..c133039eb3d 100644 --- a/cmd/ipfswatch/main.go +++ b/cmd/ipfswatch/main.go @@ -121,12 +121,7 @@ func run(ipfsPath, watchPath string) error { } } proc.Go(func(p process.Process) { - file, err := os.Open(e.Name) - if err != nil { - log.Println(err) - } - defer file.Close() - k, err := coreunix.Add(node, file) + k, err := coreunix.AddR(node, e.Name) if err != nil { log.Println(err) } From 9deaa044bafa928c57e4ebf10698fd55b592c82a Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 25 Apr 2015 20:23:18 -0700 Subject: [PATCH 03/10] cmd/ipfs/init: bytes.NewBufferString -> strings.NewReader From the docs for strings.NewReader [1]: It is similar to bytes.NewBufferString but more efficient and read-only. [1]: https://golang.org/pkg/strings/#NewReader --- cmd/ipfs/init.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/ipfs/init.go b/cmd/ipfs/init.go index 75434199c93..84866cee11a 100644 --- a/cmd/ipfs/init.go +++ b/cmd/ipfs/init.go @@ -1,10 +1,10 @@ package main import ( - "bytes" "errors" "fmt" "io" + "strings" context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" assets "github.com/ipfs/go-ipfs/assets" @@ -137,9 +137,9 @@ func addDefaultAssets(out io.Writer, repoRoot string) error { // add every file in the assets pkg for fname, file := range assets.Init_dir { - buf := bytes.NewBufferString(file) + reader := strings.NewReader(file) dagNode, err := importer.BuildDagFromReader( - buf, + reader, nd.DAG, nd.Pinning.GetManual(), chunk.DefaultSplitter) From 1d404a7b0ce4576adf3ec2e1275b1b00280c7392 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 26 Apr 2015 15:57:15 -0700 Subject: [PATCH 04/10] util/testutil/addcat/addcat: Factor out add/cat/verify helper Save some repetitive logic in the integration tests. While we're at it, replace coreunix.Add with a directo call to BuildDagFromReader. coreunix.Add is basically a wrapper around BuildDagFromReader that returns a string-ified key instead of a DAG node. That's what we want here, so it's a few more lines to inline the key extraction (mostly due to Go's explicit error handling [1], which makes method chaining not idomatic [2]). But I think a bit of local error-checking is a reasonable price for a more concise API, where there's only one function for adding file nodes from a reader. Previous versions of this commit used used a string (instead of []byte) for AddCat's data, because we don't need to mutate the value, and Go's strings are just immutable byte arrays. Jeromy pointed out that most of our callers have []byte data that they've read from io.Readers [3], so it's better to use []byte to avoid the extra copy [4]. I'd planned to add the AddCat helper to the testutil package, but that triggered some cyclic imports: $ make test cd cmd/ipfs && go build -i import cycle not allowed package github.com/ipfs/go-ipfs/cmd/ipfs imports github.com/ipfs/go-ipfs/commands imports github.com/ipfs/go-ipfs/core imports github.com/ipfs/go-ipfs/blockservice imports github.com/ipfs/go-ipfs/exchange/bitswap imports github.com/ipfs/go-ipfs/exchange/bitswap/testnet imports github.com/ipfs/go-ipfs/p2p/net/mock imports github.com/ipfs/go-ipfs/p2p/test/util imports github.com/ipfs/go-ipfs/util/testutil imports github.com/ipfs/go-ipfs/core/coreunix imports github.com/ipfs/go-ipfs/importer imports github.com/ipfs/go-ipfs/importer/balanced imports github.com/ipfs/go-ipfs/importer/helpers imports github.com/ipfs/go-ipfs/merkledag imports github.com/ipfs/go-ipfs/blockservice import cycle not allowed package github.com/ipfs/go-ipfs/cmd/ipfs imports github.com/ipfs/go-ipfs/commands imports github.com/ipfs/go-ipfs/core imports github.com/ipfs/go-ipfs/blockservice imports github.com/ipfs/go-ipfs/exchange/bitswap imports github.com/ipfs/go-ipfs/exchange/bitswap/testnet imports github.com/ipfs/go-ipfs/p2p/net/mock imports github.com/ipfs/go-ipfs/p2p/test/util imports github.com/ipfs/go-ipfs/util/testutil imports github.com/ipfs/go-ipfs/core Makefile:27: recipe for target 'build' failed make: *** [build] Error 1 I think the proper workaround is to split tests that import test-only libraries out into their own packages. For example, exchange/bitswap/bitswap_test.go is in the bitswap package, but imports the exchange/bitswap/testnet package, which in turn pulls in mock, testutil, etc. The easiest workaround seems to be using external tests [5] (e.g. bitswap_test for exchange/bitswap/bitswap_test.go). I tried to apply this approach, but the current codebase had too many crosslinks (e.g. internal tests that import test-only packages but also use internal utilities from their current package), so I gave up. I think gradually converting test files into external tests is a good thing, but it's too much to bite off for this feature branch. [1]: https://golang.org/doc/faq#exceptions [2]: http://stackoverflow.com/a/27300387 [3]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29549480 [4]: http://stackoverflow.com/a/9649061 [5]: http://dave.cheney.net/2014/12/01/five-suggestions-for-setting-up-a-go-project --- test/integration/addcat_test.go | 17 ++------- test/integration/bench_cat_test.go | 19 ++-------- test/integration/grandcentral_test.go | 20 ++--------- test/integration/three_legged_cat_test.go | 18 ++-------- util/testutil/addcat/addcat.go | 43 +++++++++++++++++++++++ 5 files changed, 51 insertions(+), 66 deletions(-) create mode 100644 util/testutil/addcat/addcat.go diff --git a/test/integration/addcat_test.go b/test/integration/addcat_test.go index df38fbbdf95..cd2473451d2 100644 --- a/test/integration/addcat_test.go +++ b/test/integration/addcat_test.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "io" "math" "os" "testing" @@ -14,11 +13,11 @@ import ( context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" "github.com/ipfs/go-ipfs/core" - coreunix "github.com/ipfs/go-ipfs/core/coreunix" mocknet "github.com/ipfs/go-ipfs/p2p/net/mock" "github.com/ipfs/go-ipfs/p2p/peer" "github.com/ipfs/go-ipfs/thirdparty/unit" testutil "github.com/ipfs/go-ipfs/util/testutil" + addcat "github.com/ipfs/go-ipfs/util/testutil/addcat" ) const kSeed = 1 @@ -126,22 +125,10 @@ func DirectAddCat(data []byte, conf testutil.LatencyConfig) error { return err } - added, err := coreunix.Add(adder, bytes.NewReader(data)) + err = addcat.AddCat(adder, catter, data) if err != nil { return err } - - readerCatted, err := coreunix.Cat(catter, added) - if err != nil { - return err - } - - // verify - var bufout bytes.Buffer - io.Copy(&bufout, readerCatted) - if 0 != bytes.Compare(bufout.Bytes(), data) { - return errors.New("catted data does not match added data") - } return nil } diff --git a/test/integration/bench_cat_test.go b/test/integration/bench_cat_test.go index 23931b3db6e..48f4e8e144b 100644 --- a/test/integration/bench_cat_test.go +++ b/test/integration/bench_cat_test.go @@ -1,19 +1,17 @@ package integrationtest import ( - "bytes" "errors" - "io" "math" "testing" context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" "github.com/ipfs/go-ipfs/core" - coreunix "github.com/ipfs/go-ipfs/core/coreunix" mocknet "github.com/ipfs/go-ipfs/p2p/net/mock" "github.com/ipfs/go-ipfs/p2p/peer" "github.com/ipfs/go-ipfs/thirdparty/unit" testutil "github.com/ipfs/go-ipfs/util/testutil" + addcat "github.com/ipfs/go-ipfs/util/testutil/addcat" ) func BenchmarkCat1MB(b *testing.B) { benchmarkVarCat(b, unit.MB*1) } @@ -74,22 +72,9 @@ func benchCat(b *testing.B, data []byte, conf testutil.LatencyConfig) error { return err } - added, err := coreunix.Add(adder, bytes.NewReader(data)) + err = addcat.AddCat(adder, catter, data) if err != nil { return err } - - b.StartTimer() - readerCatted, err := coreunix.Cat(catter, added) - if err != nil { - return err - } - - // verify - var bufout bytes.Buffer - io.Copy(&bufout, readerCatted) - if 0 != bytes.Compare(bufout.Bytes(), data) { - return errors.New("catted data does not match added data") - } return nil } diff --git a/test/integration/grandcentral_test.go b/test/integration/grandcentral_test.go index afc3a668804..47fcbb7d3f4 100644 --- a/test/integration/grandcentral_test.go +++ b/test/integration/grandcentral_test.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "io" "math" "testing" @@ -14,7 +13,6 @@ import ( core "github.com/ipfs/go-ipfs/core" "github.com/ipfs/go-ipfs/core/corerouting" - "github.com/ipfs/go-ipfs/core/coreunix" mocknet "github.com/ipfs/go-ipfs/p2p/net/mock" "github.com/ipfs/go-ipfs/p2p/peer" "github.com/ipfs/go-ipfs/thirdparty/iter" @@ -22,6 +20,7 @@ import ( "github.com/ipfs/go-ipfs/util" ds2 "github.com/ipfs/go-ipfs/util/datastore2" testutil "github.com/ipfs/go-ipfs/util/testutil" + addcat "github.com/ipfs/go-ipfs/util/testutil/addcat" ) func TestSupernodeBootstrappedAddCat(t *testing.T) { @@ -54,25 +53,10 @@ func RunSupernodeBootstrappedAddCat(data []byte, conf testutil.LatencyConfig) er adder := clients[0] catter := clients[1] - log.Info("adder is", adder.Identity) - log.Info("catter is", catter.Identity) - - keyAdded, err := coreunix.Add(adder, bytes.NewReader(data)) + err = addcat.AddCat(adder, catter, data) if err != nil { return err } - - readerCatted, err := coreunix.Cat(catter, keyAdded) - if err != nil { - return err - } - - // verify - var bufout bytes.Buffer - io.Copy(&bufout, readerCatted) - if 0 != bytes.Compare(bufout.Bytes(), data) { - return errors.New("catted data does not match added data") - } return nil } diff --git a/test/integration/three_legged_cat_test.go b/test/integration/three_legged_cat_test.go index 0a5b9c62f7d..ba0133f9818 100644 --- a/test/integration/three_legged_cat_test.go +++ b/test/integration/three_legged_cat_test.go @@ -1,9 +1,7 @@ package integrationtest import ( - "bytes" "errors" - "io" "math" "testing" "time" @@ -11,11 +9,11 @@ import ( context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" core "github.com/ipfs/go-ipfs/core" - coreunix "github.com/ipfs/go-ipfs/core/coreunix" mocknet "github.com/ipfs/go-ipfs/p2p/net/mock" "github.com/ipfs/go-ipfs/p2p/peer" "github.com/ipfs/go-ipfs/thirdparty/unit" testutil "github.com/ipfs/go-ipfs/util/testutil" + addcat "github.com/ipfs/go-ipfs/util/testutil/addcat" ) func TestThreeLeggedCatTransfer(t *testing.T) { @@ -106,21 +104,9 @@ func RunThreeLeggedCat(data []byte, conf testutil.LatencyConfig) error { return err } - added, err := coreunix.Add(adder, bytes.NewReader(data)) + err = addcat.AddCat(adder, catter, data) if err != nil { return err } - - readerCatted, err := coreunix.Cat(catter, added) - if err != nil { - return err - } - - // verify - var bufout bytes.Buffer - io.Copy(&bufout, readerCatted) - if 0 != bytes.Compare(bufout.Bytes(), data) { - return errors.New("catted data does not match added data") - } return nil } diff --git a/util/testutil/addcat/addcat.go b/util/testutil/addcat/addcat.go new file mode 100644 index 00000000000..b6f803c75e5 --- /dev/null +++ b/util/testutil/addcat/addcat.go @@ -0,0 +1,43 @@ +package testutil + +import ( + "bytes" + "errors" + + core "github.com/ipfs/go-ipfs/core" + coreunix "github.com/ipfs/go-ipfs/core/coreunix" + importer "github.com/ipfs/go-ipfs/importer" + chunk "github.com/ipfs/go-ipfs/importer/chunk" +) + +func AddCat(adder *core.IpfsNode, catter *core.IpfsNode, data []byte) error { + dagNode, err := importer.BuildDagFromReader( + bytes.NewBuffer(data), + adder.DAG, + adder.Pinning.GetManual(), + chunk.DefaultSplitter) + if err != nil { + return err + } + key, err := dagNode.Key() + if err != nil { + return err + } + added := key.String() + + reader, err := coreunix.Cat(catter, added) + if err != nil { + return err + } + + // verify + var buf bytes.Buffer + _, err = buf.ReadFrom(reader) + if err != nil { + return err + } + if !bytes.Equal(buf.Bytes(), data) { + return errors.New("catted data does not match added data") + } + return nil +} From f91e3264fcc999ef9b4598275d50647436ad6c4b Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 28 Apr 2015 21:56:16 -0700 Subject: [PATCH 05/10] shell/unixfs: Add a high-level filesystem-node package Starting to use the core/... (low-level, all the knobs you need) shell/... (high-level, handle the common case easily) disctinction laid out in [1]. Creating a file node from a reader is hard to do right, because there's a lot of filesystem metadata that we don't have access to (access mode, ownership, etc.). You can guess at those based on the adding process's umask, effective user, etc., but figuring out what you want guessed at or what you want set explicitly, or whether you want wrapping metadata at all is complicated. This function isn't going to do any of that [2], it's just a high-level wrapper to create a minimal file object with the default chunking, pinning, etc. all taken care of in ways that will probably work for you ;). I'm not clear on where the 'unix' part of the name comes from (these seem like pretty generic filesystem nodes to me, or if anything, they're POSIX filesystem nodes), but: On Wed, Apr 29, 2015 at 01:30:04PM -0700, Juan Batiz-Benet wrote [3]: > > package fsnode > > i think this package should be called unixfs as that's the > abstraction that this is calling to. So that's what we're going with. [1]: https://github.com/ipfs/go-ipfs/issues/1158 [2]: https://github.com/ipfs/go-ipfs/pull/1136#issuecomment-96854906 [3]: https://github.com/ipfs/go-ipfs/pull/1136#discussion_r29377283 --- shell/shell.go | 8 ++++++++ shell/unixfs/add.go | 28 ++++++++++++++++++++++++++++ shell/unixfs/unixfs.go | 10 ++++++++++ 3 files changed, 46 insertions(+) create mode 100644 shell/shell.go create mode 100644 shell/unixfs/add.go create mode 100644 shell/unixfs/unixfs.go diff --git a/shell/shell.go b/shell/shell.go new file mode 100644 index 00000000000..199fa8cb642 --- /dev/null +++ b/shell/shell.go @@ -0,0 +1,8 @@ +/* +Package shell implements a high-level interface for common IPFS activity. + +These wrappers around the low-level core interface make it easy to +accomplish common tasks with default settings. For steps where the +defaults aren't appropriate, you can use the core package directly. +*/ +package shell diff --git a/shell/unixfs/add.go b/shell/unixfs/add.go new file mode 100644 index 00000000000..8338fe4ca7b --- /dev/null +++ b/shell/unixfs/add.go @@ -0,0 +1,28 @@ +package unixfs + +import ( + "io" + + core "github.com/ipfs/go-ipfs/core" + importer "github.com/ipfs/go-ipfs/importer" + chunk "github.com/ipfs/go-ipfs/importer/chunk" + dag "github.com/ipfs/go-ipfs/merkledag" +) + +// Add builds a merkledag from a reader, pinning all objects to the +// local datastore. Returns the root node. +func AddFromReader(node *core.IpfsNode, reader io.Reader) (*dag.Node, error) { + fileNode, err := importer.BuildDagFromReader( + reader, + node.DAG, + node.Pinning.GetManual(), + chunk.DefaultSplitter, + ) + if err != nil { + return nil, err + } + if err := node.Pinning.Flush(); err != nil { + return nil, err + } + return fileNode, nil +} diff --git a/shell/unixfs/unixfs.go b/shell/unixfs/unixfs.go new file mode 100644 index 00000000000..fb798aaf099 --- /dev/null +++ b/shell/unixfs/unixfs.go @@ -0,0 +1,10 @@ +/* +Package unixfs is a high-level interface for filesystem nodes. + +Simple wrappers to: + +* create file and directory nodes from paths and readers, +* extract file and directory nodes to your local filesytem, and +* print file contents to writers. +*/ +package unixfs From 03458dc2a5ffe421066f4b30c92f12e2684cd8a1 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 28 Apr 2015 22:09:12 -0700 Subject: [PATCH 06/10] core/coreunix/add: Deprecate Add Folks should be using the node-returning AddFromReader defined in shell/unixfs instead, since that creates the same node but returns the dag.Node object itself, instead of the stringified key. It's cheaper to go from Node to stringified key than it is to go the other way around, and you need the Node to do anything besides printing the stringified key, so the shell/unixfs version has a better API and we want to push people in that direction. The shell/unixfs version also uses our intended high-/low-level API distinction (core/... is for low-level stuff) [1]. This commit starts the deprecation/migration using the procedure outlined here [2], with the API-release-notes directory proposed here [3] and accepted here [4]. [1]: https://github.com/ipfs/go-ipfs/issues/1158 [2]: https://github.com/ipfs/go-ipfs/issues/1159 [3]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29379579 [4]: https://github.com/ipfs/go-ipfs/pull/1136/files#r29498385 --- core/coreunix/add.go | 8 ++++++-- docs/release-notes/unreleased.md | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 docs/release-notes/unreleased.md diff --git a/core/coreunix/add.go b/core/coreunix/add.go index 69acbe320e3..fdbe9e15688 100644 --- a/core/coreunix/add.go +++ b/core/coreunix/add.go @@ -22,8 +22,12 @@ import ( var log = eventlog.Logger("coreunix") -// Add builds a merkledag from the a reader, pinning all objects to the local -// datastore. Returns a key representing the root node. +// DEPRECATED: Add builds a merkledag from the a reader, pinning all +// objects to the local datastore. Returns a key representing the root +// node. +// +// This function is deprecated and will be removed in 0.4.0. You +// should use shell/unixfs's AddFromReader instead. func Add(n *core.IpfsNode, r io.Reader) (string, error) { // TODO more attractive function signature importer.BuildDagFromReader dagNode, err := importer.BuildDagFromReader( diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md new file mode 100644 index 00000000000..17bd2ab64a1 --- /dev/null +++ b/docs/release-notes/unreleased.md @@ -0,0 +1,26 @@ +# `core/coreunix`'s `Add` → `shell/unixfs`'s `AddFromReader` + +We've added an `AddFromReader` function to `shell/unixfs`. The old +`Add` from `core/coreunix` is deprecated and will be removed in +version 0.4.0. To update your existing code, change usage like: + + keyString, err := coreunix.Add(ipfsNode, reader) + if err != nil { + return err + } + +to + + fileNode, err := unixfs.AddFromReader(ipfsNode, reader) + if err != nil { + return err + } + key, err := fileNode.Key() + if err != nil { + return err + } + keyString := key.String() + +That's a bit more verbose if all you want is the stringified key, but +returning a `dag.Node` makes it easier to perform other operations +like adding the file node to a directory node. From 6388c7090b3a58114812dab79282866ac9d5bb2f Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 29 Apr 2015 09:10:25 -0700 Subject: [PATCH 07/10] util/testutil/addcat: importer.BuildDagFromReader -> unixfs.AddFromReader The defaults used by the high-level wrapper are fine here, and we're in a test so consuming shell/... is ok. --- util/testutil/addcat/addcat.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/util/testutil/addcat/addcat.go b/util/testutil/addcat/addcat.go index b6f803c75e5..157e4275066 100644 --- a/util/testutil/addcat/addcat.go +++ b/util/testutil/addcat/addcat.go @@ -6,16 +6,11 @@ import ( core "github.com/ipfs/go-ipfs/core" coreunix "github.com/ipfs/go-ipfs/core/coreunix" - importer "github.com/ipfs/go-ipfs/importer" - chunk "github.com/ipfs/go-ipfs/importer/chunk" + unixfs "github.com/ipfs/go-ipfs/shell/unixfs" ) func AddCat(adder *core.IpfsNode, catter *core.IpfsNode, data []byte) error { - dagNode, err := importer.BuildDagFromReader( - bytes.NewBuffer(data), - adder.DAG, - adder.Pinning.GetManual(), - chunk.DefaultSplitter) + dagNode, err := unixfs.AddFromReader(adder, bytes.NewBuffer(data)) if err != nil { return err } From 20ff6a74fa70c23ad0d5914bc9e00ba346b2e574 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 28 Apr 2015 22:23:43 -0700 Subject: [PATCH 08/10] test/supernode_client/main.go: Migrate to unixfs.AddFromReader Using the procedure suggested in the changelog. --- test/supernode_client/main.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/test/supernode_client/main.go b/test/supernode_client/main.go index 11ee6ed7378..2a9320acf86 100644 --- a/test/supernode_client/main.go +++ b/test/supernode_client/main.go @@ -28,6 +28,7 @@ import ( "github.com/ipfs/go-ipfs/repo" config "github.com/ipfs/go-ipfs/repo/config" fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo" + unixfs "github.com/ipfs/go-ipfs/shell/unixfs" eventlog "github.com/ipfs/go-ipfs/thirdparty/eventlog" unit "github.com/ipfs/go-ipfs/thirdparty/unit" ds2 "github.com/ipfs/go-ipfs/util/datastore2" @@ -151,10 +152,15 @@ func runFileAddingWorker(n *core.IpfsNode) error { log.Fatal(err) } }() - k, err := coreunix.Add(n, piper) + fileNode, err := unixfs.AddFromReader(n, piper) if err != nil { log.Fatal(err) } + key, err := fileNode.Key() + if err != nil { + log.Fatal(err) + } + k := key.String() log.Println("added file", "seed", *seed, "#", i, "key", k, "size", unit.Information(sizeOfIthFile(i))) time.Sleep(1 * time.Second) } @@ -185,10 +191,15 @@ func runFileCattingWorker(ctx context.Context, n *core.IpfsNode) error { log.Fatal(err) } // add to a dummy node to discover the key - k, err := coreunix.Add(dummy, bytes.NewReader(buf.Bytes())) + fileNode, err := unixfs.AddFromReader(dummy, bytes.NewReader(buf.Bytes())) + if err != nil { + log.Fatal(err) + } + key, err := fileNode.Key() if err != nil { log.Fatal(err) } + k := key.String() e := elog.EventBegin(ctx, "cat", eventlog.LoggableF(func() map[string]interface{} { return map[string]interface{}{ "key": k, From 93df40de5acbcd906da64e053481dea47d89cf38 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 25 Apr 2015 20:51:46 -0700 Subject: [PATCH 09/10] core/corehttp/gateway_test: Replace coreunix.Add with importer.BuildDagFromReader coreunix.Add is basically a wrapper around BuildDagFromReader that returns a string-ified key instead of a DAG node. That's what we want here, so it's a few more lines to inline the key extraction (mostly due to Go's explicit error handling [1], which makes method chaining not idomatic [2]). But I think a bit of local error-checking is a reasonable price for a more concise API, where there's only one function for adding file nodes from a reader. [1]: https://golang.org/doc/faq#exceptions [2]: http://stackoverflow.com/a/27300387 --- core/corehttp/gateway_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/core/corehttp/gateway_test.go b/core/corehttp/gateway_test.go index 818338c1c8a..ab822a95916 100644 --- a/core/corehttp/gateway_test.go +++ b/core/corehttp/gateway_test.go @@ -10,7 +10,8 @@ import ( context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" core "github.com/ipfs/go-ipfs/core" - coreunix "github.com/ipfs/go-ipfs/core/coreunix" + importer "github.com/ipfs/go-ipfs/importer" + chunk "github.com/ipfs/go-ipfs/importer/chunk" namesys "github.com/ipfs/go-ipfs/namesys" ci "github.com/ipfs/go-ipfs/p2p/crypto" path "github.com/ipfs/go-ipfs/path" @@ -60,10 +61,19 @@ func TestGatewayGet(t *testing.T) { t.Skip("not sure whats going on here") ns := mockNamesys{} n := newNodeWithMockNamesys(t, ns) - k, err := coreunix.Add(n, strings.NewReader("fnord")) + dagNode, err := importer.BuildDagFromReader( + strings.NewReader("fnord"), + n.DAG, + n.Pinning.GetManual(), + chunk.DefaultSplitter) if err != nil { t.Fatal(err) } + key, err := dagNode.Key() + if err != nil { + t.Fatal(err) + } + k := key.String() ns["example.com"] = path.FromString("/ipfs/" + k) h, err := makeHandler(n, From c8f7b39963cdf67e231146ce0dfd22f7d7299029 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 30 Apr 2015 09:37:59 -0700 Subject: [PATCH 10/10] core/corehttp/gateway_test: Externalize test to use shell/unixfs' AddFromReader We don't want to create core -> shell -> core import cycles, so we need to move this test out of the corehttp package before we can safely use AddFromReader. Moving the tests into their own _test package means we need to explicitly import the corehttp package, which in turn means we need to *export* makeHandler (now MakeHandler). Pulling in the shell code also increases the surface area for bugs breaking the test, which was previously only testing the low-level code. Not that I expect many bugs in shell/unixfs's AddFromReader, but it's still nice to rule out a whole API when tracking down issues ;). Personally, it seems simpler to stick to keep using importer.BuildDagFromReader directly from this test, but Juan prefers the external approach taken by this commit [1]. [1]: https://github.com/ipfs/go-ipfs/pull/1136#issuecomment-97580638 --- core/corehttp/corehttp.go | 6 +++--- core/corehttp/gateway_test.go | 18 +++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/core/corehttp/corehttp.go b/core/corehttp/corehttp.go index e8852baaf59..75feb930e26 100644 --- a/core/corehttp/corehttp.go +++ b/core/corehttp/corehttp.go @@ -23,9 +23,9 @@ var log = eventlog.Logger("core/server") // initially passed in if not. type ServeOption func(*core.IpfsNode, *http.ServeMux) (*http.ServeMux, error) -// makeHandler turns a list of ServeOptions into a http.Handler that implements +// MakeHandler turns a list of ServeOptions into a http.Handler that implements // all of the given options, in order. -func makeHandler(n *core.IpfsNode, options ...ServeOption) (http.Handler, error) { +func MakeHandler(n *core.IpfsNode, options ...ServeOption) (http.Handler, error) { topMux := http.NewServeMux() mux := topMux for _, option := range options { @@ -49,7 +49,7 @@ func ListenAndServe(n *core.IpfsNode, listeningMultiAddr string, options ...Serv if err != nil { return err } - handler, err := makeHandler(n, options...) + handler, err := MakeHandler(n, options...) if err != nil { return err } diff --git a/core/corehttp/gateway_test.go b/core/corehttp/gateway_test.go index ab822a95916..6890d4ede34 100644 --- a/core/corehttp/gateway_test.go +++ b/core/corehttp/gateway_test.go @@ -1,4 +1,4 @@ -package corehttp +package corehttp_test import ( "errors" @@ -10,13 +10,13 @@ import ( context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context" core "github.com/ipfs/go-ipfs/core" - importer "github.com/ipfs/go-ipfs/importer" - chunk "github.com/ipfs/go-ipfs/importer/chunk" + corehttp "github.com/ipfs/go-ipfs/core/corehttp" namesys "github.com/ipfs/go-ipfs/namesys" ci "github.com/ipfs/go-ipfs/p2p/crypto" path "github.com/ipfs/go-ipfs/path" repo "github.com/ipfs/go-ipfs/repo" config "github.com/ipfs/go-ipfs/repo/config" + unixfs "github.com/ipfs/go-ipfs/shell/unixfs" testutil "github.com/ipfs/go-ipfs/util/testutil" ) @@ -61,11 +61,7 @@ func TestGatewayGet(t *testing.T) { t.Skip("not sure whats going on here") ns := mockNamesys{} n := newNodeWithMockNamesys(t, ns) - dagNode, err := importer.BuildDagFromReader( - strings.NewReader("fnord"), - n.DAG, - n.Pinning.GetManual(), - chunk.DefaultSplitter) + dagNode, err := unixfs.AddFromReader(n, strings.NewReader("fnord")) if err != nil { t.Fatal(err) } @@ -76,9 +72,9 @@ func TestGatewayGet(t *testing.T) { k := key.String() ns["example.com"] = path.FromString("/ipfs/" + k) - h, err := makeHandler(n, - IPNSHostnameOption(), - GatewayOption(false), + h, err := corehttp.MakeHandler(n, + corehttp.IPNSHostnameOption(), + corehttp.GatewayOption(false), ) if err != nil { t.Fatal(err)