Skip to content

Use previously define uri for the td:name and jsonschema:propertyName #1169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

Murloc6
Copy link

@Murloc6 Murloc6 commented Jun 9, 2021

This is a temporary fix since there are ongoing issues in jsonld.js (digitalbazaar/jsonld.js#453) and in the api itself (w3c/json-ld-api#514).

The workaround here is to define properties "name" and "propertyName" because it is (for now) necessary for the "@index" property. The prefixed URI leads to an error and the full URI leads to a "@none" index.

c.f. #1161


Preview | Diff

mmccool and others added 30 commits February 2, 2021 21:40
fixes example 48 and 50
first working version of tm schema generation with string addition
@Murloc6 Murloc6 changed the title Use previously define uri for the td:name and "jsonschema:propertyName" Use previously define uri for the td:name and jsonschema:propertyName Jun 9, 2021
@RoboPhred
Copy link

RoboPhred commented Jun 9, 2021

I tried this out with one of my tds and it doesn't seem to have fixed the property indexes.
Playground link

It took a change in my generated TD, but this does seem to fix the issue.
Previously, I was using @index as the property name key in expanded form, but it looks like I should be using https://www.w3.org/2019/wot/td#name. I am not sure where I picked up using @index, but it seems using the td#name IRI is the way to go from #1077.
Playground link

However, something about this breaks the property affordance "type" key. It is now compacted as a full uri http://www.w3.org/1999/02/22-rdf-syntax-ns#type, and its value is now trying to use a value prefixed with jsonschema.
Using the current master schema, this works as expected:
Playground link with context from ddf2088

properties generated with proposed PR's context

  "properties": {
    "model::name": {
      "description": "The name of this thing.",
      "title": "Name",
      "http://www.w3.org/1999/02/22-rdf-syntax-ns#type": {
        "id": "jsonschema:StringSchema"
      },
      "jsonschema:maxLength": 255,
      "jsonschema:minLength": 0,
      "jsonschema:readOnly": false
    },
    "model::location": {
      "description": "The location of this thing.",
      "title": "Location",
      "http://www.w3.org/1999/02/22-rdf-syntax-ns#type": {
        "id": "jsonschema:StringSchema"
      },
      "jsonschema:maxLength": 255,
      "jsonschema:minLength": 0,
      "jsonschema:readOnly": false
    }
  }

Properties generated with current (Pre-PR) context:

  "properties": {
    "@none": [
      {
        "description": "The name of this thing.",
        "title": "Name",
        "type": "string",
        "jsonschema:maxLength": 255,
        "jsonschema:minLength": 0,
        "jsonschema:readOnly": false,
        "name": "model::name"
      },
      {
        "description": "The location of this thing.",
        "title": "Location",
        "type": "string",
        "jsonschema:maxLength": 255,
        "jsonschema:minLength": 0,
        "jsonschema:readOnly": false,
        "name": "model::location"
      }
    ]
  }

@RoboPhred
Copy link

Curiously, the rdf type issue doesnt affect a round trip of the example thing (compaction step), so the issue might be in my expanded form.

@RoboPhred
Copy link

RoboPhred commented Jun 12, 2021

The branch this context is based off of seems seriously out of date with the mainline context. I tried using it directly in testing and it types contentEncoding as an unsigned int, which seems to have been fixed a while ago. The branch reports This branch is 4 commits ahead, 209 commits behind w3c:main.

This also explains why I had to change my expanded thing definition. The old version this is based off of supplied an "@index": "td:name", but the new version does not specify "@index" at all, leaving the expanded form to use the "@index" key instead of "https://www.w3.org/2019/wot/td#name" to index properties.

sebastiankb and others added 6 commits June 16, 2021 17:28
Proposal: use version field in schemas
add success to addResp and refactor addResp into definitions
move examples form description to skos:examples
Fix ContentEncoding and ContentMediaType
@ethieblin
Copy link

ethieblin commented Jun 17, 2021

Hmm, I'm not a great git user and I tried a merge so that you would get the final context in this PR.
Unfortunately, the diff of the PR is unreadable now.

I pasted the new generated context in your previous Playground example (the one with "http://www.w3.org/1999/02/22-rdf-syntax-ns#type") and it seems fine after this merge :
Playground with updated context

@vcharpenay
Copy link
Contributor

This PR seems to have redundancies with #1077. I fixed the problem of indexing properties in that other PR. As I commented there, it should be merged soon-ish, because it overlaps with few other issues/PRs and leads to duplicated work...

@Murloc6
Copy link
Author

Murloc6 commented Jun 24, 2021

We fixed the problem defined here, and we rebase it on your proper PR. That's why there are redundancies.
Our fix is this specific commit : da6956a
This commit adds a named property, because the "@index" attribute does not accept the prefixed URI (digitalbazaar/jsonld.js#453).
That's the only contribution we've made on top of your PR, the rest is only a (not well ended) merge to let @RoboPhred test the final context.

@vcharpenay
Copy link
Contributor

OK, I see. What commit da6956a does, however, is to turn full URIs (e.g. https://www.w3.org/2019/wot/json-schema#propertyName) into relative URIs (propertyName), right?

The consequence is that properties are not properly resolved in RDF and it doesn't seem to solve @RoboPhred's problem of round-tripping. An example: the following compacted form

{
  "@context": {
    "properties": {
      "@id": "https://www.w3.org/2019/wot/json-schema#properties",
      "@container": "@index",
      "@index": "propertyName"
    },
    "title": "http://purl.org/dc/terms/title"
  },
  "properties": {
    "p": {
      "title": "some test property"
    }
  }
}

doesn't properly round-trip. The expanded, then re-compacted form is:

{
  "@context": {
    "properties": {
      "@id": "https://www.w3.org/2019/wot/json-schema#properties",
      "@container": "@index",
      "@index": "propertyName"
    },
    "title": "http://purl.org/dc/terms/title"
  },
  "properties": {
    "@none": {
      "title": "some test property"
    }
  }
}

(@none shouldn't be there).

@ethieblin
Copy link

ethieblin commented Jun 29, 2021

Not exactly, it does two things:

  • create an alias for a URI that will be used as an index "propertyName":"jsonschema:propertyName"
  • use that alias as an index

As precised in Murloc's comment, we use the alias as a workaround for an issue of the jsonld.js api (and in the w3c json ld api itself) where @index URIs are not parsed well.

In your example, you do not define the alias, therefore, the round tripping does not work.

Example of expansion with the alias

Re-compaction with the alias

@vcharpenay
Copy link
Contributor

Alright, my bad. I missed the part about the alias. The behavior of jsonld.js is beyond logic on that particular point...

@vcharpenay
Copy link
Contributor

Since I'm not sure what commits do indeed belong here, I implemented your fix in #1077 (see d02cf7b).

Unless there's something else I missed, I'll close this PR. Thanks!

@vcharpenay vcharpenay closed this Jun 29, 2021
@ethieblin
Copy link

Yes, thank you, there was one td:name remaining, I left you a comment on the given PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.