Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Spec refining: Properties in links #2

Closed
nicola opened this issue Jul 25, 2016 · 7 comments
Closed

Spec refining: Properties in links #2

nicola opened this issue Jul 25, 2016 · 7 comments
Labels
status/deferred Conscious decision to pause or backlog

Comments

@nicola
Copy link
Member

nicola commented Jul 25, 2016

The current spec defines that links are represented using the keyword property /. However, it doesn't specify what are the constraints for the link object.

// Valid IPLD
hash1
{
  hereIsALink: {'/': HASH},
  size: 500
}
// IPLD with property in a link
hash2
{
  hereIsALink: {
    '/': HASH,
    'size': 500
  }
}

Should we support properties in links or should we not support that?

If we decide to support this, the property size cannot be addressed with an IPLD pointer, however, this property can still be accessed by loading the object hash2 and manually/programmatically traverse the object to get the size value.

Reason to support

  • the difference between ipld data model and json is just reserving the keyword /
  • developers can hide whatever they want

Reason not to support it

  • hidden elements make IPLD a little bit more complex (hidden properties cannot be addressed)
  • makes IPLD not transparent

Idea
A great idea would be to properties that are not specified by our spec, so that we can reuse this space for future reserved keywords


cc @jbenet, @mildred, @dignifiedquire

@dignifiedquire
Copy link
Member

I prefer the simplest viable solution which in my opinion is disallowing any other siblings to / in the link object.

@nicola
Copy link
Member Author

nicola commented Jul 26, 2016

Yes, that is my opinion too. In this way we can reserve that space for links specific things (say one day we want to specify some new keywords, we can use this space for that)

@nicola
Copy link
Member Author

nicola commented Jul 29, 2016

Also, what we have to define is if this is a MUST NOT, or a SHOULD NOT

@nicola nicola mentioned this issue Aug 4, 2016
2 tasks
@jbenet
Copy link
Contributor

jbenet commented Aug 6, 2016

We already bikeshedded this a ton over the last months, it was the source of months of disagreement and arguments. In the end the decision NOT to allow this was critical in cleaning up pathing.

The way to treat it in the spec is (rewording it for clarity) one of two options:

1. Completely disallow it, treat it as an error. (MUST NOT)

  • Link objects MUST NOT contain other properties.
  • If they do, the object is not treated as a link and resolution through it is disallowed. Addressing its data is disallowed. Attempts to resolve or address properties MUST return an error.

2. Remove it from the model, but work with objects that happen to have it. (SHOULD NOT)

  • Link objects SHOULD NOT contain any other properties. But if there is an object with other things, the formal IPLD model does not recognize the other properties, and addressing / resolution ignore them.
  • In case an object "has extra data" tools MUST NOT expose this other data items to pathing or addressing.

Notes

  • This is really only a problem because JSON does not let us define "types" and we hacked it with {/: <link> }. this is entirely a JSON side-effect and DOES NOT really show up in CBOR, YML, and other more expressive formats.
  • Allowing (2) will complicate implementations of IPLD in various formats. Forcing (1) lets us define special types in CBOR and other serialized formats that are very compact, and DO NOT allow properties.
  • Allowing (2) will be the source of much confusion in users and tools that decide to go against the spec (or fail to realize it) and let users address those properties.
  • Forcing (1) is "simple" and (2) is "complex" in the Rich Hickey sense of the word.
  • I think we should do (1). It's going to be a bit annoying at first (forcing us to throw errors) but much more valuable long term by increasing simplicity.

@dignifiedquire
Copy link
Member

👍 for (1) for the reasons you mentioned and from the perspective of implementation. It's straightforward to enforce this and throw appropriate errors instead of trying to figure out how to handle additional props.

@nicola
Copy link
Member Author

nicola commented Aug 7, 2016

🎉 Perfect, we are all aligned.
(as I mentioned this can also be great in case we want to reserve new link object features)

@daviddias daviddias mentioned this issue Feb 13, 2017
15 tasks
@daviddias daviddias added the status/deferred Conscious decision to pause or backlog label Mar 19, 2018
@rvagg
Copy link
Member

rvagg commented Aug 14, 2019

Closing due to staleness as per team agreement to clean up the issue tracker a bit (ipld/team-mgmt#28). This doesn't mean this issue is off the table entirely, it's just not on the current active stack but may be revisited in the near future. If you feel there is something pertinent here, please speak up, reopen, or open a new issue. [/boilerplate]

@rvagg rvagg closed this as completed Aug 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/deferred Conscious decision to pause or backlog
Projects
None yet
Development

No branches or pull requests

5 participants