1. 21 Mar, 2019 1 commit
    • Eric Myhre's avatar
      Iterator refactor: entry-based, for map and list. · b84e99cd
      Eric Myhre authored
      We now have both MapIterator and ListIterator interfaces.
      Both return key-value (or index-value) pairs, rather than just keys.
      
      List iterators may seem a tad redundant: you just loop over the length,
      right?  Well, sure.  But there's one place a list iterator shines:
      selecting only a subset of elements.  And indeed, we'll be doing
      exactly that in the traversal/selector package; therefore, we
      definitely need list iterators.
      
      We might want keys-only iterators again in the future, but at present,
      I'm deferring that.  It's definitely true that we should have iterators
      returning values as a core feature, since they're likely to be more
      efficiently supportable than "random" access (especially when we get to
      some Advanced Layout data systems), so we'll implement those first.
      
      Additionally, note that MapIterator now returns a Node for the key.
      This is to account for that fact that when using the schema system and
      typed nodes, map keys can be more *specific* types.  Such nodes are
      still required to be kind==ReprKind_String, but string might not be
      their *preferred* native format (think: tuples with serialized to be
      delimiter-separated strings); we need to account for that.
      (MapBuilder.Insert method already takes a Node parameter for similar
      reasons: so it can take *typed* nodes.  Node.TraverseField accepting
      a plain string is the oddball out here, and should be rectified.)
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      b84e99cd
  2. 20 Mar, 2019 3 commits
  3. 19 Mar, 2019 1 commit
    • Eric Myhre's avatar
      Naming: ReprKind. · fe099392
      Eric Myhre authored
      Having a function called "Kind" return a "ReprKind" was inconsistent.
      
      Also, we want to introduce a "Kind" method on `typed.Node` in the future.
      
      No logical content to this change: you can safely refactor with sed.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      fe099392
  4. 18 Mar, 2019 1 commit
  5. 16 Mar, 2019 13 commits
    • Eric Myhre's avatar
      Merge branch 'drop-mutable-node' · adc089c2
      Eric Myhre authored
      adc089c2
    • Eric Myhre's avatar
      Add NodeBuilder to Node interface. · 025fcf8a
      Eric Myhre authored
      (... offically.  Lots of docs have probably already been stating that
      this is there.  Now it actually... is.)
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      025fcf8a
    • Eric Myhre's avatar
      Drop MutableNode interface. · 6428f14f
      Eric Myhre authored
      This has been deprecated and replaced by the NodeBuilder system
      for a good while now; time to scrape it into the dustbin completely.
      
      Tests that were primarily on the mutable node system itself also
      drop, so, this is a *very* large delete diff.
      
      A few other tests used MutableNode just incidentally, and those are
      quick fixed to use NodeBuilder.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      6428f14f
    • Eric Myhre's avatar
      Merge branch 'linking-redux' · fdbbd90f
      Eric Myhre authored
      fdbbd90f
    • Eric Myhre's avatar
      Update traversal package to new linking system. · b8550cf5
      Eric Myhre authored
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      b8550cf5
    • Eric Myhre's avatar
      Dag-json marshal and unmarshal. · 10442b3c
      Eric Myhre authored
      And fixes to all the encoding systems for checking lengths when they're
      provided by map and list start tokens.  Inconsistencies there are now
      errors.
      
      And some consistency changes across all the encoders to keep the diff
      of the dag-json system as minimal as possible.  (Dag-json needs to
      refer to the last handful of tokens sometimes when parsing a mapClose,
      so we keep their values outside of the loop body now.)
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      10442b3c
    • Eric Myhre's avatar
      Dag-cbor marshal/unmarshal. · a274b2db
      Eric Myhre authored
      We now have CIDs support!  You can create links backed by cids,
      and marshal them with dag-cbor; and you can unmarshal cbor data
      with dag-cbor and expect things with the CID link tag to be parsed
      into CIDs and exposed as IPLD Links.  Yay!
      
      (Dag-json is lagging.  The parse for those links is... more involved.
      When supported, it'll similarly have its own unmarshal and marshal
      just like the ones this diff introduces for dag-cbor.)
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      a274b2db
    • Eric Myhre's avatar
      Update Node interfaces to use Link instead of CID. · 694c6f3c
      Eric Myhre authored
      As detailed in comments a few commits ago, this is part of a big, big
      roll towards keeping linking details far enough off to one side that
      one can actually use most of the IPLD system without forming an
      explicit compile-time dependency on any linking features (until, of
      course, one uses the linking features).
      
      This is a surprisingly small diff, because... well, because most of
      the *interesting* features around linking simply weren't implemented
      yet, and at this point everything that is has already been isolated
      in the new cidlink and related encoding packages.
      "CID" was *already* just a semantic placeholder that meant "eh, link".
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      694c6f3c
    • Eric Myhre's avatar
      Finish creating cid in cidlink.LinkBuilder. · 89c40af3
      Eric Myhre authored
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      89c40af3
    • Eric Myhre's avatar
      Repose refactored into several packages. · 57832ad7
      Eric Myhre authored
      All of which now explicitly confess their cid-specificness.
      
      Some things in the 'repose' draft were *really* twisted; I'm glad that
      first draft is getting replaced before anything actually used it...
      
      For example, NodeBuilderChooser was just ridiculously misplaced abstraction.
      When doing a traversal, you have the local type information (if any) already
      in hand, and can just... pick an appropriate NodeBuilder already.  Now that
      the NodeBuilder is simply a parameter to Link.Load, everything shakes out
      much, much more clearly as a result.
      
      The cidlink package contains all concrete referns to Cids.  This implements
      the ipld.Link and ipld.LinkBuilder interfaces... but if you don't import
      the cidlink package in your program, you won't find any of the cid packages
      (nor their numerous transitive dependencies) in your dependency set.
      
      Multicodecs are now a registry which is confined in scope to the cidlink
      package.  (It's global, but I think in practice this will be fine: it's a
      plugin system, and there's no good cause for allowing variations in how
      those magic bytes of cids are interpreted.)
      
      There are now dagcbor and dagjson packages for encoding.  These explicitly
      refer to the cidlink package (and register themselves on package init).
      While these refer to cidlink, you could imagine we might also introduce
      other encoding packages which *don't*.
      
      Finally, note that the dagcbor and dagjson packages are in fact still not
      done.  This is the same logic/completeness they had before this diff...
      which does not include actual parsing of cids!  However, it's now clear
      where to introduce that and at what scope.  (It will probably require
      more duplication of unmarshalling code than desirable, but, alas, that
      might simply be the cost of doing business.  Dagjson in particular has
      topologically "interesting" things to handle that I'd be loathe to make
      a sufficiently pluggable unmarshal traversal to support; it would be
      possible, but likely a noticable slowdown to continuously check and
      then promptly disregard the interesting case.)
      
      Some work remains, but this is now pretty close to sanity.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      57832ad7
    • Eric Myhre's avatar
      Add context params to link load and build. · dbf02a67
      Eric Myhre authored
      These belong in load and build since those are at the top of the stack;
      they also (perhaps surprisingly) aren't necessary as params to the
      Loader and Storer function interfaces (since those just return readers
      and writers, and thus 'cancel' for those is 'stop using it').
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      dbf02a67
    • Eric Myhre's avatar
      Begin introduction of new linking interfaces. · 6950c5cf
      Eric Myhre authored
      This is going to be the start of a pretty hectic set of commits.
      
      1. The LinkLoader, LinkBuilder, and LinkContext types currently in
        the traversal package are hereby doomed (and will be deleted by
        the time this branch is ready to land).
      
      2. Cid itself will disappear from almost all remaining concrete uses.
        For example, traversal.TraversalProgress.LastBlock will use Link
        instead of Cid.  (You'll have to cast back to Cid if that detail
        is important to your application!)
      
      3. The 'repose' package is pretty much getting nuked.
      
      Instead: we have these new interfaces.  Link will be abstract.
      
      We'll add a linking package with a subpackage containing implementation
      with Cids.  Our encoding systems will then also live there: this makes
      sense since multicodec is definitely a detail associated with Cids.
      We'll also have dag-cbor and dag-json specific encodings in subpackages
      associated with this whole thing: those will be able to read and reify
      Link instances during their handling of serial data.  This too is
      parsimonious and correct (e.g. dag-json parsing is technically distinct
      from regular json parsing, even if it's experientially close).
      
      And if all goes according to plan, we will -- shockingly -- be able to
      use almost the entirety of the IPLD system... *without* forming an
      explicit import dependency on the Cid packages *until* we directly use
      any link loading features based on Cids.  That'll be neat.
      
      All these changes will be staged across a series of commits because
      the total diff will definitely be something fierce.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      6950c5cf
    • Eric Myhre's avatar
      Move Path to the root package. · d0edb867
      Eric Myhre authored
      Yes, this one has moved about a bunch now.  Hopefully this is the last time.
      It's true and elegant that paths really only emerge as descriptions of traversal
      progress; however, this misses a few other practicalities.
      There are other kinds of traversal (other than the traversal package, whoa!) out
      there: see the typesystem packages, which had grown a custom path implementation
      simply for error reporting messages!
      In general, we seem to want to have Path around for logging and errors, which
      will make it increasingly desirable to have available in the root package when
      we begin to clean up towards strongly typed errors.
      And we also need Path for LinkContext -- and wherever that comes to rest, it
      definitely need to not be an import cycle problem, which it *is* if Path is
      in traversal and we wanted LinkContext to be *anywhere* else.
      All of these point to moving Path back up to the root, and the errors concern
      in particular cinches it.
      
      Drop the Path.traverse method.  As has been noted in its comment for a while
      now, that method wasn't useful for much, having been replaced by features
      in the traversal package.
      
      (Also drop the tests specific to the Path.traverse method.  We should write
      more tests against the features now implemented by travesral.Focus... but at
      this point, it'd be easier to start over.  The tests we're dropping are against
      a different model of traversal (returns rather than visitors) and are also
      built against the old hacky ipldfree mutable model which is deprecated and
      soon to be dropped.)
      
      Also, a small docs fix: drop description of this Path implementation as a
      "merklepath" -- it is not.  These paths are all relative, and do not contain
      an innate understanding of hashed object identifiers at their first segment.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      d0edb867
  6. 13 Mar, 2019 5 commits
  7. 12 Mar, 2019 6 commits
  8. 08 Mar, 2019 1 commit
  9. 04 Mar, 2019 1 commit
  10. 27 Feb, 2019 2 commits
    • Eric Myhre's avatar
      Beginye another pass on docs. · d168547a
      Eric Myhre authored
      Starting with a new index, which enumerates lots of things which shall
      deserve at least one page or section.
      
      Big picture also includes links out to things which shall require their
      own (as yet unwritten) pages.
      
      Old ball-of-mud "dev.md" already being eroded into the new files.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      d168547a
    • Eric Myhre's avatar
      Merge branch 'traversals'. · aaea73ad
      Eric Myhre authored
      Incomplete, but progress.
      
      There are still many many todos in the content, but that branch
      has simply gotten long enough.  And I need to spend some time
      docuemnting (and docs have diverged on master), so it's time to
      reel it in.
      aaea73ad
  11. 21 Feb, 2019 6 commits
    • Eric Myhre's avatar
      Merge branch 'encoding' into traversals · cd841fb1
      Eric Myhre authored
      cd841fb1
    • Eric Myhre's avatar
      Wiring JSON and CBOR to multicodec in repose! Yas! · 749cf35c
      Eric Myhre authored
      Finally.
      
      This is "easy" now that we've got all the generic marshalling
      implemented over Node and unmarshalling onto NodeBuilder:
      we just glue those pieces together with the JSON and CBOR
      parsers and emitters of refmt, and we're off.
      
      These methods in the repose package further conform this to
      byte-wise reader and writer interfaces so we can squeeze them
      into Multicodec{En|De}codeTable... and thence feed them
      into ComposeLink{Loader|Builder} and USE them!
      
      And by use them, I mean not just use them, but use them transparently
      and without a fuss, even from the very high level Traverse and
      Transform methods, which let users manipulate Nodes and NodeBuilders
      with *no idea* of all the details of serialization down here.
      
      Which is awesome.
      
      (Okay, there's a few more todo entries to sort out in the link
      handling stuff.  Probably several.)
      
      But this should about wrap it up with encoding stuff!  I'm going
      to celebrate with a branch merge, and consider the main topic to
      be lifted back up to the Traverse stuff and how to integrate
      cleanly with link loaders and link builders.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      749cf35c
    • Eric Myhre's avatar
      New marshal implementation! Generic. Woo! · f150a81b
      Eric Myhre authored
      We have both generic marshal and unmarshal -- they should work for any
      current or future ipld.Node implementation, and for any encoding
      mechanism that can be bridged to via refmt tokens.
      
      Tests are also updated to use builders rather than the ancient
      "mutable node" nonsense, which removes... I think nearly the last
      incident of that stuff; maybe we can remove it entirely soon.
      
      As when we moved the unmarshal code into its generic form, most of
      this code already existed and needed minor modification.  Git even
      correctly detects it as a rename this time since the diff is so small.
      And as when we moved the unmarshal code, now we also remove the
      whole PushTokens interface; we've gotten to something better now.
      
      Finally we're getting to the point we can look at wiring these up
      together with all the multicodec glue and get link loading wizardry
      at full voltage.  Yesss.  Sooon.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      f150a81b
    • Eric Myhre's avatar
      Remove unused field in fluent.NodeBuilder. · 5f70f2e3
      Eric Myhre authored
      Totally unexported, never touched.  Baleet.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      5f70f2e3
    • Eric Myhre's avatar
      Remove Build from fluent.{Map|List}Builder view. · 0776b93b
      Eric Myhre authored
      Per comments in this diff as well as discussion in previous commit's
      message, there's no need for it to be exported anymore.
      
      In fact, the only possible way it could've been used would've been
      invalid -- calling those methods in the closure body would've caused
      the list/map builder to invalidate itself, and make the library's
      later Build call almost certainly error.  So, it's very much better
      not to have it at exported at all.
      
      This has brought us to an odd result: we could very nearly be dropping
      the fluent.{Map|List}Builder *interfaces* (though of course keeping
      their internal error-into-panic implementations) and just continuing
      to fit the existing ipld.{Map|List}Builder interfaces (but you'd be
      within your rights to ignore all error returns; they'd be redundant).
      Would this be helpful, though?  I dunno.  So let's not.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      0776b93b
    • Eric Myhre's avatar
      Significant changes to fluent.{List|Map}Builder. · f316102c
      Eric Myhre authored
      Two changes are combined here: these builders now work in tandem with
      closures (and call-chaining style is being backed away from entirely);
      and, these new closures also get NodeBuilder values supplied to them.
      
      This correctly prepares for NodeBuilder specializations to be viable
      in the future.  Although this need is not "yet" exercised,
      typed.NodeBuilder won't be able to keep its semantics properly
      without this -- It's similar to the existing comments in Unmarshal code
      which mention how we'll require specializations for typed.Node usage
      in the future around the recursion sites there.  And while this isn't
      exercised yet, getting the API correctly-shaped earlier than later
      will save a lot of refactoring fuss.
      
      I have also just plain been wanting a closure style syntax, because it
      nests nicely and more does the right things with visual alignment.
      So now we have one.  Nice.
      
      The diffs in the test code using this new style of builder should be
      a nice example of how this looks compared to the old API syntax.
      I think those diffs speak for themselves.
      
      (It absolutely would be even nicer if we had a syntax for declaring
      closures in golang that could infer the types of their arguments
      without putting such a heavy textual weight on the closure declaration
      site.  There's no way to do this in Golang currently, but I'm pretty
      certain it would be a feasible language enhancement without much
      in the way of theoretic stretches nor significant complier complexity
      nor additional user complexity.  This might be something to pursue.)
      
      ---
      
      One interesting design point to debate within the closure design here:
      should we really try to 'save' the closure from calling Build?
      (This code does.)
      
      On a first thought, perhaps not: doing the Build call in the library
      code deprives the author of the closure from being able to Recover
      errors from the final Build call if desired.
      Doubly problematically, stack traces from those Build errors would
      contain worrying less meaningful stack traces, since such errors would
      be raised from library code rather than the closure.
      
      On the second thought: both those issues seem easily addressed in
      in actual usage.  The stack trace will still capture the line of the
      `CreateFoo(fn(...` invokation, and that line should be sufficiently
      proximate to user (rather than library) code to be meaningful.
      (There's approximately no reason I can imagine to be passing any
      ListBuildingClosure functions from far away.  If we had ruby syntax,
      this stuff would all be a 'do' block at the end of line, and
      it would be syntactically unsupported to provide from a distance.)
      It's even possible to wrap exactly the `CreateFoo` call in a Recover,
      which makes it possible to get every bit as much control as if we
      made the Build call the closure's responsibility and wrapped it there.
      
      In light of those 'second' thoughts -- and also because having the
      {Map|List}BuildingClosure definitions include an 'ipld.Node' return
      type *in addition* to their already irritating length is a not
      insigificant exacerbation of usage friction -- I'm opting to keep
      the Build call in the fluent library code for now.
      
      ---
      
      (This fix comes here and now because writing additional test fixtures
      for the next-up marshalling code without these QoL improvements was
      just not any fun, and the more critical future issues jumped in front
      of my eyes at roughly the same time.)
      
      ---
      
      Also worth noting in this diff: the large REVIEW comment in the
      MapBuildingClosure docs about having a NodeBuilder for keys.
      
      I haven't added all of the notes I have on the strings-vs-Nodes
      questions around map key APIs to tracking in git, and probably should;
      it's turned out more fiddly than expected.  This comment here probably
      already gives a fair insight into both how and why and how much so.
      
      The NodeBuilder-for-keys param is included in MapBuildingClosure
      for now out of an abundance of caution.  But this is indeed worthy
      of review and might be elided in the future.
      Signed-off-by: default avatarEric Myhre <hash@exultant.us>
      f316102c