-
Notifications
You must be signed in to change notification settings - Fork 3
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
Determine link prop Iri form link value prop… #163
Conversation
* | ||
* @param linkValueProperty IRI of the link value property. | ||
*/ | ||
getLinkPropertyIriFromLinkValuePropertyIri(linkValueProperty: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tobiasschweizer As I mentioned yesterday, I believe this type of regex is not the solution. What if a user has ended the property name also with Value. eg: property: "somethingValue" then LinkValue would be "somethingValueValue". This code would trim the "Value" from both property and its linkValue. That would be actually wrong. I believe the API internally defines the connection between a property and its linkValue. In js-lib we should exploit that connection instead of applying regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is just a naming convention that a link value property ends with "Value" and the corresponding link property is the same just without "Value" at the end. @benjamingeer Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes:
The name of the link value property is constructed using a simple naming convention:
the wordValue
is appended to the name of the link property.
https://docs.knora.org/paradox/02-knora-ontologies/knora-base.html#links-between-resources
In Knora, the SmartIri
class has methods fromLinkPropToLinkValueProp
and fromLinkValuePropToLinkProp
, which just add and remove the word Value
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, on startup, Knora checks that in each ontology, there is a link value property for each link property and vice versa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the API internally defines the connection between a property and its linkValue.
I think that would actually make sense, but the relation is not defined explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that would be possible since ReadResource
contains all the necessary information. I will do that. But still the relation between the two properties in only based on the naming convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But still the relation between the two properties in only based on the naming convention.
That's right. But I think that if you have property example:hasBook
with knora-api:objectType example:Book
, and property example:hasBookValue
with knora-api:objectType knora-api:LinkValue
, you can be sure that they go together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benjamingeer I was aware of this regulation for the definition of links in the ontology. But I was under the impression that API aligns the link value property to its corresponding link property. Then one could always request the link property of each link value property. In this case, we would not need to check if the property name ends with "Value" or not. Instead, we could just check the ObjectType, if it was linkValue, we would request its corresponding link property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could find out whether a property is really a link property, or really a link value property, by checking its
knora-api:objectType
in its ontology.
done in 4e0d7d8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Internally, no connection is stored between a link property and the corresponding link value property. We just use the naming convention, and the type information in the property definitions, to convert from one to another and check for errors.
To determine whether a property is a link property or a link value property, Knora doesn't rely on whether the property's name ends in Value
. Instead, it looks at the property's object type as defined in its ontology, which says whether its object should be a resource or a link value. Then, once Knora is sure which kind of property it is, it adds or removes Value
as needed to convert to the other kind of property. It then checks that this other property exists as well, etc.
I think this PR is not the place to have a debate whether the relation between link value and link properties should be represented differently by Knora. Please create an issue in knora-api repo for that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I added a test for a non link value prop just to be sure: 3848979 |
@SepidehAlassi Is this PR ok for you? |
@SepidehAlassi Ok thanks, I will merge this PR and update #125 (branch we are using in knora-ui-ng-lib tests) as soon as the tests passed. Then you can use it in your PR. |
Merged :-) |
do not forget to rebuild knora-api-js-ib and republish it with yalc :-) |
closes #162