Engineering Objects

[Michael Petritsch]

Hi,

I had a look at the EngineeringObjectIT and would like to start a
discussion about the current behavior. In order to better understand
how the EOObject actually works I have enhanced the tests and added 4
more tests to document the current behavior. (you can see the
corresponding code at [1])

I've added 2 Tests for inserts, 2 Tests for deletes. I've also added a
shared field to all Models used in the tests (meaning that every Model
of the linked class now has the field "shared" and it should always be
the same for all of them). While doing so, I noticed the following
behavior:

1. Inserting linked models: the EOObject linking only works if you
manually insert the SourceModels and then afterwards the EOModel. If
you just insert an EOModel or SourceModel the EngSB does not
automatically create the linked Models. This behavior is covered in
the testcases "testInsertEOModel_doesNotInsertLinkedModels" and
"testInsertSourceModel_doesNotInsertLinkedModels".

2. Deleting linked models: deleting an EOModel or SourceModel does not
delete the linked Models. Deleting has to be done manually for each
Domain. This behavior is covered in the testcases
"testDeleteEOModel_doesNotDeleteLinkedModels" and
"testDeleteSourceModel_doesNotDeleteLinkedModels".

3. Updating linked models: works generally, however after adding the
shared field, I've found one case where updating does not work: if the
shared field of a SourceModelA is changed and committed then the
shared field of SourceModelB is not updated. This can be seen in the
test testIfSourceUpdateWorksCorrectly_shouldUpdateEngineeringObject.
In the test testIfEOUpdateWorksCorrectly_shouldUpdateSourceModel where
the shared field is updated by changing it in the EOModel the
propagation is working correctly.

Ok, so much for the current behavior, next I will give my opinion to
these 3 points:

1. couldn't this be changed so that the user only has to insert 1
model and the linked ones are created automatically? however, I
believe this change would require some effort since it would require
to either predefine a naming schema for edbIds or change the
@OpenEngSBForeignKey annotation and add the possibility to set a
naming schema there.

2. shouldn't this be changed so that deletes are propagated to all
linked models? Since update propagation already works I believe this
shouldn't require much effort to implement.

3. the failed update is a bug imo.

What do you think about these points?

I also did some refactoring to improve the readability of the tests.
Unfortunately I was limited by the circumstance that OpenEngSBModels
do only work if they are used in @Test annotated methods. Using
OpenEngSBModels in other methods always causes a "not a model" error
for some reason. Do you have an idea how to circumvent this?

Btw, is the formatter at [2] the most current one? (there are some
formatting differences between the original code and my code)

br,
Michael

[1] http://github.com/mipe/openengsb/commit/a7b6281d2e788ae266722c121c189353a8a8e121
[2] http://dev.openengsb.org/resources/eclipse/formatter.xml

[Felix Mayerhuber]

Hi,

long mail with long answers ^^:

Am 2013-07-16 01:38, schrieb Michael Petritsch:
> 1. Inserting linked models: the EOObject linking only works if you
> manually insert the SourceModels and then afterwards the EOModel. If
> you just insert an EOModel or SourceModel the EngSB does not
> automatically create the linked Models. This behavior is covered in
> the testcases "testInsertEOModel_doesNotInsertLinkedModels" and
> "testInsertSourceModel_doesNotInsertLinkedModels".

It should also work if you insert the EOModel and the SourceModels
separately in no specific order. I'm quite sure that I have tested that.

> 2. Deleting linked models: deleting an EOModel or SourceModel does not
> delete the linked Models. Deleting has to be done manually for each
> Domain. This behavior is covered in the testcases
> "testDeleteEOModel_doesNotDeleteLinkedModels" and
> "testDeleteSourceModel_doesNotDeleteLinkedModels".

Deleting of linked models was never implemented since I got never an
answer from our researches about that topic. The problem is, that simply
deleting all linked models may have some unwanted side effects (at least
I suppose that this is the case, triggering deletes to models which are
not "hard-linked" to each other sounds quite dangerous to me).

> 3. Updating linked models: works generally, however after adding the
> shared field, I've found one case where updating does not work: if the
> shared field of a SourceModelA is changed and committed then the
> shared field of SourceModelB is not updated. This can be seen in the
> test testIfSourceUpdateWorksCorrectly_shouldUpdateEngineeringObject.
> In the test testIfEOUpdateWorksCorrectly_shouldUpdateSourceModel where
> the shared field is updated by changing it in the EOModel the
> propagation is working correctly.

I will have a look at this test tomorrow. I'm 100% positive that such a
scenario should work, since I have written test cases for such a
scenario too. Maybe the error is located somewhere else. Will have a
look at it.

> Ok, so much for the current behavior, next I will give my opinion to
> these 3 points:
>
> 1. couldn't this be changed so that the user only has to insert 1
> model and the linked ones are created automatically? however, I
> believe this change would require some effort since it would require
> to either predefine a naming schema for edbIds or change the
> @OpenEngSBForeignKey annotation and add the possibility to set a
> naming schema there.

You mean that you create an Engineering Object and the Source Objects
for this Engineering Object are created if they aren't existing yet? I
mean, it should be no big deal to automatically create source model
objects out of such Engineering Object instances (as long as the values
for the foreign keys are not null). This should make no problem as long
as there are no semantical checks (which are planned ^^), since objects
with only null values normally violate some constraints.

> 2. shouldn't this be changed so that deletes are propagated to all
> linked models? Since update propagation already works I believe this
> shouldn't require much effort to implement.

As already pointed out before, this need to be clarified with the
researchers, since a deletion is from a semantical point of view the
most complex operation. Suppose we activate this automated deletion
propagation. How far should this deletion be propagated? If the objects
are part of only a simple connection (one engineering object to n source
model objects, with no other connections to each involved model), this
is only a minor problem. It gets very problematic when the involved
models are themselves source models or engineering objects of other
constructs. I hope the problem is somewhat clear with these few words : ).

> 3. the failed update is a bug imo.

I will have a look at it.

> I also did some refactoring to improve the readability of the tests.
> Unfortunately I was limited by the circumstance that OpenEngSBModels
> do only work if they are used in @Test annotated methods. Using
> OpenEngSBModels in other methods always causes a "not a model" error
> for some reason. Do you have an idea how to circumvent this?

The problems source is rather simple if you know how the model
enhancement works: Like described in the documentation, the model
enhancement at normal run-time is done via the the weaving hook feature
of OSGi. It allows to modify classes (to be exact, their byte code)
before they get first used. Since in normal tests no OSGi system is
started, the byte code manipulation needs to be done in a different way:
through a Java agent. The agent is created by the weaver-service bundle
and all bundles which use tests that needs enhanced models, are
configured to use this agent for the test initialization (this behavior
is defined in the pom.xml files of the corresponding bundles). If you
start tests from an IDE, which are not integration tests, you need to
add the Java agent as JVM parameter.

> Btw, is the formatter at [2] the most current one? (there are some
> formatting differences between the original code and my code)

this formatter is the most current one AFAIK:
https://github.com/openengsb/openengsb/blob/master/etc/eclipse/formatter.xml

Kind regards,
Felix

[Felix Mayerhuber]

Hi,

sry, wrong link for the checkstyle file. The correct path is:
https://github.com/openengsb/openengsb-root/blob/master/etc/checkstyle/checkstyle.xml

Kind regards,
Felix

Am 2013-07-16 01:38, schrieb Michael Petritsch:

[Felix Mayerhuber]

Hi,

the error has been solved and the bugfix was merged into the current master.

Kind regards,
Felix