SteKoe / ocl.js

The Object Constraint Language (OCL) is a language for describing rules that apply to MOF conform modelling languages like UML. The OCL is a text based language that provides constraint and object query expressions that cannot be expressed by a meta modelling language.
http://ocl.stekoe.de
Other
54 stars 10 forks source link

forAll() expression without collector always returns false #709

Open s-blu opened 1 month ago

s-blu commented 1 month ago

Greetings!

First of all, (again) a warm thank you for providing this great library.

If I use a forAll() expression an collector/variable, for example context Person inv: self.children->forAll(c | c.age <= 10), it works fine and returns true as result. However, if I formulate the same rule without identifier, meaning context Person inv: self.children->forAll(age <= 10), it does return false on the same data. You can illustrate that by enhancing the corresponding test case in forAll.spec.ts:

  it('should iterate over collected items without having a collector', () => {
    mother.children = [
      FixtureFactory.createPerson('A', 1),
      FixtureFactory.createPerson('B', 2),
      FixtureFactory.createPerson('C', 4),
      FixtureFactory.createPerson('D', 8),
      FixtureFactory.createPerson('E', 10),
    ];

    oclExpression = 'context Person inv: self.children->forAll(age <= 10)';
    expectOclRuleValidatesToTrue(oclExpression, mother);

    oclExpression = 'context Person inv: self.children->forAll(age < 10)';
    expectOclRuleValidatesToFalse(oclExpression, mother);
  });

Adjusted like that, the test fails.

I was on my road to fix another bug I encountered and would love to take care of this behavior as well. but I am a bit stuck right now.

When the forAllExpression runs, it'll pass this code:

        if (collection instanceof Array) {
            const iterators = this.getIterators();
            const body = this.getBody();

            if (!iterators || iterators.length === 0) {
                return false;

this.getIterators() is returning undefined if no collector is defined, leading to an instant exit with false due to the first if condition.

Though, I could not quite follow when the Iterators should be set - or to what - to make this syntax working or if I might adjust some completely other part of the library. Do you have any pointers for me what the wanted behaviour is and how I possibly could achieve that?

My latest idea was to go through the collection and setting the current object of the collection as variables:

if (!iterators || iterators.length === 0) {
    return !collection.some(c => {
        return body.evaluate(visitor, c) === false;
    });
}

but I'd certainly prefer to understand better what I am tinkering with.

Regards

SteKoe commented 1 month ago

Hi @s-blu,

thanks for opening this issue. I have pushed a branch that seems to fix the bug. Please try out if it works for you, as well :)

Regards, Stephan

s-blu commented 1 month ago

Hello @SteKoe, thanks alot for having a look.

fix/709 does indeed fixes context Person inv: self.children->forAll(age <= 10). However, when I integrate the fix branch into #710, following two tests still fails:

library = new Library();
library.writers = [new Writer('Joe'), new Writer('Alice'), new Writer('Ben')];
library.writers[0].books = [
    new Book('A great tale', 9.99),
    new Book('An awesome tale', 9.99),
    new Book('Biography', 15.99),
    new Book('Some light thriller', 10)
];
library.writers[1].books = [new Book('Biography', 5.99), new Book('A tale', 19.99)];
library.writers[2].books = [
    new Book('Biography', 15),
    new Book('A tale', 10),
    new Book('Cookbook for Cakes', 20),
    new Book('Cookbook for Cookies', 20),
    new Book('Cookbook for Vegetables', 25)
];
library.writers[1].books[1].awards.push('Best Newcomer');
library.writers[2].books[2].awards.push('Pulitzer Prize');

let oclExpression = 'context Library inv: self.writers->forAll(w | w.books->isUnique(price))';
expectOclRuleValidatesToFalse(oclExpression, library);

oclExpression = 'context Library inv: self.writers->forAll(w | w.books->one(title = "Biography"))';
expectOclRuleValidatesToTrue(oclExpression, library);

Without #710, they fail due to the nesting of collection expressions. With context Library inv: self.writers->forAll(w | w.books->isUnique(price)) I am not quite sure if I messed up the syntax for isUnique, still havent figured out how to use this. I tried to setup an easier example and ended up with this:

it('should work without collector', () => {
    const obj = {
        name: 'example',
        videogames: ['A', 'B', 'C', 'D', 'A'],
        books: ['R', 'S', 'T']
    }

    // this works
    let oclExpression = 'context Object inv: books->isUnique(books)';
    expectOclRuleValidatesToTrue(oclExpression, obj);

    // this fails because isUnique takes the whole object as collection and returns false (no array) 
    oclExpression = 'context Object inv: self->isUnique(books)'; 
    expectOclRuleValidatesToTrue(oclExpression, obj);

    // this fails with "Cannot read properties of undefined (reading 'getVariable')"
    oclExpression = 'context Object inv: books->isUnique()'; 
    expectOclRuleValidatesToTrue(oclExpression, obj);
})

I don't know if the failure is due to your changes or if this is an isolated problem.

For context Library inv: self.writers->forAll(w | w.books->one(title = "Biography"))' however, I'd expect this to work. Worth to mention that following test is successfull, so this doesn't seem to be a general issue with nested collection functions without collector, but maybe a problem specific to one?

let oclExpression = 'context Library inv: self.writers->forAll(w | w.books->exists(title = "Biography"))';
expectOclRuleValidatesToTrue(oclExpression, library);

In addition, using the same rule with explicit collector - context Library inv: self.writers->forAll(w | w.books->one(b | b.title = "Biography")) - works fine, too.

--- EDIT ---- Sometimes I am blind to the most obvious thing to do. If I change the last test in one.spec.ts to work without collector, it fails. So it seems to be a problem with one.

The problem does not persists for the forAll expression anymore though, so I leave it up to you if you want to take care of the problem with one as well.

    it('should return false if there is no matching property in object', () => {
        const oclExpression = 'context Object inv: self->one(a > 0) = true';
        expectOclRuleValidatesToTrue(oclExpression, [
            {a: 1},
            {c: 2},
            {b: 3}
        ]);
    });
github-actions[bot] commented 1 month ago

:tada: This issue has been resolved in version 1.4.4 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

s-blu commented 1 month ago

Thank you very much for taking care!

SteKoe commented 1 month ago

Hey @s-blu,

all good, I am happy to help and very glad to see, that this library is used and still(!) works!

I am still adding some more tests and will release some more versions asap.