-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add support and tests for capability specification via provides
#80
Conversation
eeb6ea9
to
1c1c1a3
Compare
2180cbb
to
978c919
Compare
286e75f
to
f6b5485
Compare
const disjuncts = filter(requirement.raw.data.getAll(), (disjunct) => { | ||
return shouldEvaluateType(disjunct.raw.data.type); | ||
}); | ||
// (3.2) An empty disjuction means that this particular | ||
// requirement is fulfilled, so we can carry on. | ||
// A disjunction naturally contains a list of further | ||
// requirements we need to check for. If at least one | ||
// of the members is fulfilled, we can proceed with | ||
// next requirement. | ||
if (disjuncts.length === 0 || some(disjuncts, hasMatch)) { | ||
return true; | ||
} |
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'm less confident on this combination but it should work (I may have done the conversion incorrectly, but the conversion into a single every
should be possible)
const disjuncts = filter(requirement.raw.data.getAll(), (disjunct) => { | |
return shouldEvaluateType(disjunct.raw.data.type); | |
}); | |
// (3.2) An empty disjuction means that this particular | |
// requirement is fulfilled, so we can carry on. | |
// A disjunction naturally contains a list of further | |
// requirements we need to check for. If at least one | |
// of the members is fulfilled, we can proceed with | |
// next requirement. | |
if (disjuncts.length === 0 || some(disjuncts, hasMatch)) { | |
return true; | |
} | |
// (3.2) An empty disjuction means that this particular | |
// requirement is fulfilled, so we can carry on. | |
// A disjunction naturally contains a list of further | |
// requirements we need to check for. If at least one | |
// of the members is fulfilled, we can proceed with | |
// next requirement. | |
if (every(requirement.raw.data.getAll(), (disjunct) => { | |
return !shouldEvaluateType(disjunct.raw.data.type) || !hasMatch(disjunct) | |
})) { | |
return true; | |
} |
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.
This doesn't work unfortunately, in order for the requirement to be accepted we need either:
- there are no requirements to evaluate
- if there are requirements to evaluate, there is at least one match
The closest thing would be
if (
every(requirement.raw.data.getAll(), (disjunct) => {
return (
!shouldEvaluateType(disjunct.raw.data.type) || hasMatch(disjunct)
);
})
) {
return true;
}
however this is not the same, because if shouldEvaluateType(disjunct.raw.data.type)
is always true, this is equivalent to writing
if (
every(requirement.raw.data.getAll(), (disjunct) => {
return hasMatch(disjunct);
})
) {
return true;
}
which is the opposite of what we want
I'm not sure there is an equivalent using a single some/every
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.
Yeah, I think the problem is we need every(disjuncts, !shouldEvaluateType) || some(disjuncts, shouldEvaluateType && hasMatch)
and those can't be combined so what we currently have is the best we can manage with functional. However it can be done via a for-loop with return
to short-circuit the some
case but keeping the every
case requiring an entire loop:
const disjuncts = filter(requirement.raw.data.getAll(), (disjunct) => { | |
return shouldEvaluateType(disjunct.raw.data.type); | |
}); | |
// (3.2) An empty disjuction means that this particular | |
// requirement is fulfilled, so we can carry on. | |
// A disjunction naturally contains a list of further | |
// requirements we need to check for. If at least one | |
// of the members is fulfilled, we can proceed with | |
// next requirement. | |
if (disjuncts.length === 0 || some(disjuncts, hasMatch)) { | |
return true; | |
} | |
// (3.2) An empty disjuction means that this particular | |
// requirement is fulfilled, so we can carry on. | |
// A disjunction naturally contains a list of further | |
// requirements we need to check for. If at least one | |
// of the members is fulfilled, we can proceed with | |
// next requirement. | |
let noneToEvaluate = true; | |
for (const disjunct of (requirement.raw.data.getAll()) { | |
if (shouldEvaluateType(disjunct.raw.data.type)) { | |
noneToEvaluate = false | |
if (hasMatch(disjunct)) { | |
return true; | |
} | |
} | |
} | |
return noneToEvaluate; |
Fwiw I don't think this is worth it unless it's a performance bottleneck but since I dug into how to do it I thought I'd share
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.
It does sacrifice a bit of readability, I suspect at some point we'll need to work on the efficiency of some of the operations at which point we can review. Thank you for looking into it!
5198c53
to
63db5d0
Compare
This removes some code duplication between `satisfiesChildContract` and `getNotSatisfiedRequirements` Change-type: patch
This lines up better with the original specification and how contracts are being used by the OS Change-type: minor
Using array cardinalities would result in the blueprint using the `Array.filter` function as the filter for the selector, causing the filtered set to always have size 0 Change-type: patch
63db5d0
to
a79b696
Compare
This updates the
satisfiesChildContract
andgetNotSatisfiedRequirements
functions to check for capabilities defined viaprovides
on the current context. This PR is just surfacing some functionality that was already partially there but never tested.Change-type: minor
Depends-on: #79