gs1 / EPCIS

Draft files being shared for EPCIS 2.0 development
Other
22 stars 7 forks source link

bugs caused by new JSONLD context (#275) #307

Closed VladimirAlexiev closed 11 months ago

VladimirAlexiev commented 3 years ago

@jmcanterafonseca-iota I'll report here bugs caused by PR #275 (new JSONLD context) that was accepted yesterday. I hope you can fix them very quickly: this is higher prio than completing the JSON schema task because it blocks downstream tasks (Turtle and Diagrams). cc @CraigRe @mgh128 @mkotoff

Please test your changes:

Bug 1 (fixed)

Tried "make", got error on the first file:

jsonld format -q ../JSON/Example_9.6.1-with-comment.jsonld | cat prefixes.ttl - | riot -syntax ttl -formatted ttl > Example_9.6.1-with-comment.ttl
09:19:15 ERROR riot :: [line: 23, col: 95] Triples not terminated by DOT
make: *** [Makefile:12: Example_9.6.1-with-comment.ttl] Error 1 

Tried just the first step:

jsonld format -q ../JSON/Example_9.6.1-with-comment.jsonld 

# these are triples
_:b0 <http://purl.org/dc/terms/created> "2005-07-11T11:30:47.0Z" .
_:b0 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://ns.gs1.org/epcis/EPCISDocument> .
_:b0 <http://www.w3.org/2002/07/owl#versionInfo> "2.0" .
_:b0 <https://ns.gs1.org/epcis/epcisBody> _:b1 .
_:b1 <https://ns.gs1.org/epcis/eventList> _:b2 .
_:b1 <https://ns.gs1.org/epcis/eventList> _:b5 .

# QUADS start here
_:b3 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <https://ns.gs1.org/epcis/ObjectEvent> _:b2 .
_:b3 <https://ns.gs1.org/epcis/action> "OBSERVE" _:b2 .

Noticed the result first has triples but then QUADS. EPCIS JSON doesn't use any named graphs, so where did this came from?

We have epcis:eventList _:b2, so we should have _:b2 rdf:type epcis:ObjectEvent. Instead, the subject is a new blank node _:b3 whereas _:b2 is relegated to the named graph position (blank-node named graph).

Bug2 (fixed)

Example_9.6.4-TransformationEvent.ttl has

"eventID": "ni:///sha-256;...?ver=CBV2.0"

This results in a blank node with a wrong property and wrong value:

[ rdf:type epcis:TransactionEvent;
  epcis:eventID "ni:///sha-256;e65c3a997e77f34b58306da7a82ab0fc91c7820013287700f0b50345e5795b97?ver=CBV2.0" ;
  ...
]

That should be resolved to the node's URI (not separate property, and URI not string):

<ni:///sha-256;e65c3a997e77f34b58306da7a82ab0fc91c7820013287700f0b50345e5795b97?ver=CBV2.0>
  rdf:type epcis:TransactionEvent;
  ...

Change this in the context

"eventID": {
      "@id": "epcis:eventID"
    },

To this:

"eventID": "@id"

Bug 3 (canceled)

"isA":"EPCISMasterDataDocument" is resolved to a relative URL:

jsonld format -q ../JSON/Example_9.8.1-MasterData.jsonld
_:b0 <http://www.w3.org/1999/02/22-rdf-syntax-ns#type> <file://C:/EPCISMasterDataDocument> 

The reason is that @vocab was removed, so the context now has only "isA": "@type".

One way to resolve is to add a term shortcut as exists for event types:

 "ObjectEvent": "epcis:ObjectEvent",
 "EPCISMasterDataDocument": "epcis:EPCISMasterDataDocument"

But this also applies to other nodes that are allowed to have isA:

grep -hrF isA  *.jsonld */*.jsonld|sort|uniq

9-Dec-2021: MasterData now is not transmitted in JSON, canceled

Bug 4 (fixed)

Move out EPCIS-JSON-Schema.json, EPCIS-JSON-Schema-single-event.json from JSON to JSON-Schema (I guess that's done in the PR that's still pending)

Bug 5 (fixed)

Wrong prop names. All prop names start with lowercase:

    "sensorReport": {
      "@id": "epcis:SensorReport",
    "sensorMetadata": {
      "@id": "epcis:SensorMetadata",

Bug 5a (fixed)

The terms sensorReport, sensorMetadata are defined twice, both cases carrying nested context: once in sensorElementList and secondly at a higher level.

Don't know whether this will work, but it's certainly confusing (I think prev bug is caused by this confusion). Merge the two definitions.

Bug 5b (fixed)

type is wrong here because the rdf:type of a node is denoted isA:

    "sensorReport": {
      "@context": {
        "type": "@type",

Bug 6 (new) duplication

Unnecessary duplication:

Bug 7 (fixed)

This is wrong:

    "bizTransactionList": {
      "@id": "epcis:bizTransaction",

Should resolve to epcis:bizTransactionList

Bug 8 (new) leak

The BTT enumeration is outside of the bizTransactionType field (it is one level higher):

      "@context": [
        {
          "@protected": true,
          "@version": 1.1,
          "cbv": "https://ns.gs1.org/cbv/",
          "bol": "cbv:BTT-bol",
          "cert": "cbv:BTT-cert",
          ...
        },
        {
          "type": {
            "@id": "epcis:bizTransactionType",
            "@type": "@vocab"
          }

The problem is that these bare word definitions will leak into any custom fields in the enclosing field bizTransactionList. Try https://tinyurl.com/73e7c8wj, which shows 3 problems:

{"@context": [
  "https://gs1.github.io/EPCIS/epcis-context.jsonld",
  {"ex": "https://example.org/ns/"},
  {"ex2": "https://ns.gs1.org/cbv/"},
  {"ex:type": {"@type": "@vocab", "@context": {"@vocab": "ex:"}}}],
"@graph": {
  "https://example.org/event/1": {
    "bizTransactionList": {
      "ex:type": "bol",      // LEAK
      "type":    "ex2:foo",  // INJECT
      "type":    "foo"       // RANDOM
}}}}

Bug 8a (new) leak

Same problem as 8:

    "errorDeclaration": {
      "@context": [
        {
          "@protected": true,
          "@version": 1.1,
          "cbv": "https://ns.gs1.org/cbv/",
          "did_not_occur": "cbv:ER-did_not_occur",
          "incorrect_data": "cbv:ER-incorrect_data"
        },
        {
          "@protected": true,
          "epcis": "https://ns.gs1.org/epcis/",
          "reason": {
            "@id": "epcis:reason",
            "@type": "@vocab"
          },

Bug 8b (new) leak

Same problem as 8 (appears twice):

      "@context": [
        {
          "@protected": true,
          "@version": 1.1,
          "cbv": "https://ns.gs1.org/cbv/",
          "owning_party": "cbv:SDT-owning_party",
          "possessing_party": "cbv:SDT-possessing_party",
          "location": "cbv:SDT-location"
        },
        {
          "type": {
            "@id": "epcis:sourceOrDestinationType",
            "@type": "@vocab"
          }

Bug 9 (fixed)

We don't use xsd:anyURI but real URLs, see #206. So these:

          "bizTransaction": {
            "@id": "epcis:bizTransaction",
            "@type": "xsd:anyURI"
          },
        "uriValue": {
          "@id": "epcis:uriValue",
          "@type": "xsd:anyURI"
        },

Should be changed to "@type":"@id"

Bug 9a (new) @id alias

This is wrong, in particular "@id": "@id" doesn't mean anything.

"bizTransaction": {
            "@id": "@id",
            "@type": "@id"
          },

Change to

"bizTransaction": "@id"

Bug 10 (fixed)

These terms resolve to wrong prop names (copy-paste error)

        "chemicalSubstance": {
          "@id": "epcis:microorganism",
          "@type": "@id"
        },
        "value": {
          "@id": "epcis:microorganism",
          "@type": "xsd:double"
        },

Bug 11 (fixed)

This doesn't cast eg Temperature into gs1:MT-Temperature but should. I think you need to add "@vocab":"gs1:MT-" but please check.

        "sensorReport": {
          "@context": {
            "@protected": true,
            "type": {
              "@id": "epcis:measurementType",
              "@type": "@vocab"
            }
          }
        },

Bug 12 (fixed)

This doesn't cast eg X to cbv:Comp-X. You also need to declare it to be @id:

        "component": "epcis:component",

Bug 12a (fixed)

epcis-context-simple.jsonld: component now resolves to cbv:Comp- not cbv:COMP- (WithSensorData/SensorDataExample10.ttl)

Bug 13 (fixed)

This lacks a datatype:

    "creationDate": "dcterms:created",

Improvement 14 (fixed)

As per latest decision, these two terms should be mapped to the same RDF prop (allowing reuse of SourceOrDestination nodes).

Change

"source": {
            "@id": "epcis:source",
"destination": {
            "@id": "epcis:destination",

To:

"source": {
            "@id": "epcis:sourceOrDestination",
"destination": {
            "@id": "epcis:sourceOrDestination",

Bug 15 (fixed)

correctiveEventIDs produces string not URI

Bug 16 (canceled)

Example_9.8.1-MasterData.ttl and Example_9.8.1-MasterData-complying-with-schema.ttl produce no useful data:

[ rdf:type           epcis:EPCISDocument ;
  dcterms:created    "2005-07-11T11:30:47.0Z" ;
  owl:versionInfo    "2.0" ;
  epcis:epcisBody    []  ;
  epcis:epcisHeader  [] 
] .

I think the reason is that you removed the @vocab declaration, so vocabularyList and deeper are not resolved.

9-Dec-2021: master data is not carried in JSON

Bug 17 (fixed)

prop epcis:PersistentDisposition is wrong (should start with lowerCase)

Bug 18 (fixed)

Wrong prop (copy-paste error):

    "readPoint": {
      "@id": "epcis:bizLocation",
      "@type": "@id"
    },

As a result, epcis:readPoint is missing in all examples

Bug 19 (fixed)

Wrong prop, should be epcis:bizTransactionList:

    "bizTransactionList": {
      "@id": "epcis:bizTransaction",

Bug 20 (fixed)

This in sensorReport is wrong, should be epcis:measurementType

              "type": {
                "@id": "epcis:type",
                "@type": "@vocab"
              },

Bug 21 (fixed) enumerate barewords

Bug 22 (fixed) missing bizStep and disposition values

@jmcanterafonseca-iota @CraigRe @RalphTro @mgh128 This is clear proof that the "enumerate all barewords" approach is not only unnecessary, but also brittle and error-prone.

Bug 23 (canceled) Missing Class

9-Dec-2021: isA removed from example

Enhancement 24 (new) SBDH

Example_9.6.1-ObjectEvent-with-pseudo-SBDH-headers.ttl is misisng the following fields because they are not in context. Maybe they are unofficial/experimental?

+  epcis:instanceIdentifier  "1234567890" ;
+  epcis:receiver            "urn:epc:id:sgln:5012345.00001.0" ;
+  epcis:sender              "urn:epc:id:sgln:0353579.00001.0"

Bug 25 (fixed) gs1 namespace

ALARM_CONDITION is mapped to <https://ns.gs1.org/voc/ALARM_CONDITION> but the correct namespace doesn't have "ns"

Bug 26 (new) cbvmda namespace

cbvmda: is not defined so it results in URLs like <cbvmda:countryOfExport> (WithFullCombinationOfFields/object_event_all_possible_fields). As decided in discussion about the openepcis tool, we should define that namespace.

mgh128 commented 2 years ago

Regarding bug 6, I've done some testing. It appears that the namespace mappings for epcis and cbv could indeed by declared once at top-level within the standard context resource for EPCIS/CBV 2.0 and that these would cascade throughout the rest of the context resource. Similarly, it appears that "@protected": true and "@version": 1.1 only need to be declared at the root level. However I'd ask @jmcanterafonseca-iota to confirm this.

Regarding bug 8, I agree that the enumerated values for cbv:SDT could be moved into the nested context for type within sourceList and destinationList. I re-read your concern about their leakage into custom fields / values, so we should fix this.

I agree that we still need to fix bug 9a.

Regarding bug 26a, we need to declare the mapping for cbvmda even if it expands to a URN

I'll try to work on these tomorrow morning.

jmcanterafonseca-iota commented 2 years ago

to be honest, here, I am not in favour of changing anything at this stage that it is not a real and tangible bug, avoiding changes that are just a matter of taste or subjective. The presence of the declaration of @protected and cbv namespaces in all @context subresoruces is due to the fact they are declared in the individual contexts that are later merged, so I would keep them as they are.

On Tue, May 3, 2022 at 1:23 AM Mark Harrison @.***> wrote:

Regarding bug 6, I've done some testing. It appears that the namespace mappings for epcis and cbv could indeed by declared once at top-level within the standard context resource for EPCIS/CBV 2.0 and that these would cascade throughout the rest of the context resource. Similarly, it appears that @.": true and @.": 1.1 only need to be declared at the root level. However I'd ask @jmcanterafonseca-iota https://github.com/jmcanterafonseca-iota to confirm this.

Regarding bug 8, I agree that the enumerated values for cbv:SDT could be moved into the nested context for type within sourceList and destinationList. I re-read your concern about their leakage into custom fields / values, so we should fix this.

I agree that we still need to fix bug 9a.

Regarding bug 26a, we need to declare the mapping for cbvmda even if it expands to a URN

I'll try to work on these tomorrow morning.

— Reply to this email directly, view it on GitHub https://github.com/gs1/EPCIS/issues/307#issuecomment-1115461653, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQEZJLTE3ZH2CX67SCBDKHDVIBPV5ANCNFSM47FD4VMA . You are receiving this because you were mentioned.Message ID: @.***>

-- IOTA Foundation Pappelallee 78/79 10437 Berlin, Germany

Board of Directors: Dominik Schiener, Navin Ramachandran ID/Foundation No.: 3416/1234/2 (Foundation Register of Berlin)

mgh128 commented 2 years ago

Hi @jmcanterafonseca-iota , @VladimirAlexiev

At https://github.com/mgh128/EPCIS/tree/main/JSON-LD-Context and in the resulting compiled file at https://mgh128.github.io/EPCIS/epcis-context.jsonld I have attempted to remove the redundant declarations of mappings for cbv, epcis and gs1, as well as removing the apparently redundant repeated declarations of "@protected": true and "@version": 1.1 since these are already declared once in https://github.com/mgh128/EPCIS/blob/main/JSON-LD-Context/epcis-context-root.jsonld only and do appear to cascade to all nested contexts, at least from the testing I did using https://json-ld.org/playground/ when I attempted to redefine CURIE mappings for protected terms "cbv" or "epcis".

Looking at the current https://github.com/gs1/EPCIS/blob/master/epcis-context.jsonld I don't think it is leaking as noted in https://github.com/gs1/EPCIS/issues/307#issuecomment-1114245126 - it appears that for sourceList, destinationList and errorDeclaration, the mappings for enumerated bare string values were already correctly nested. It might just be that my earlier preview did not do this correctly.

My version at https://mgh128.github.io/EPCIS/ also fixes bugs 9a and 26a

@jmcanterafonseca-iota - if you think this is a helpful improvement, I'm happy to make the corresponding changes in the GS1 GitHub repository for EPCIS.

jmcanterafonseca-iota commented 2 years ago

@mgh128 happy to review a PR

mgh128 commented 2 years ago

Thanks @jmcanterafonseca-iota - here's https://github.com/gs1/EPCIS/pull/431/ for your review

VladimirAlexiev commented 1 year ago

@jmcanterafonseca-iota When can you approve #431 for merging? I don't have time to review it right now, but with the explanations of @mgh128 re what it fixes, I think we can close this (mega-)issue :-)