Extend logic that marks DistributionSet as complete #838
Conversation
...itory-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaDistributionSetType.java
Outdated
Show resolved
Hide resolved
|
SonarQube analysis reported 1 issue
Watch the comments in this conversation to review them. |
DistributionSets without SoftwareModules shall not be marked as complete, when no Software Module is assigned Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <Sandeep.Ravindranath@bosch-si.com>
smy4kor
force-pushed
the
bugfix_incomplete_distribution_set
branch
from
d353523
to
fde5b94
Compare
| @@ -209,8 +210,12 @@ public void setKey(final String key) { | |||
|
|
|||
| @Override | |||
| public boolean checkComplete(final DistributionSet distributionSet) { | |||
| return distributionSet.getModules().stream().map(SoftwareModule::getType).collect(Collectors.toList()) | |||
| .containsAll(getMandatoryModuleTypes()); | |||
| List<SoftwareModuleType> smTypes = distributionSet.getModules().stream().map(SoftwareModule::getType) | |||
There was a problem hiding this comment.
could be simplified:
final Set<SoftwareModuleType> mandatoryModuleTypes = getMandatoryModuleTypes();
return distributionSet.getModules().stream().map(SoftwareModule::getType).allMatch(type -> mandatoryModuleTypes.contains(type));
There was a problem hiding this comment.
This will not work in 1 specific case. When distributionSet.getModules() is empty and mandatoryModuleTypes is also empty then allMatch returns true. I just tried it.
I guess that allMatch assumes true and looks for a false. And in above, no iteration happens.
There was a problem hiding this comment.
You are right, we can avoid that by adding:
if (distributionSet.getModules().isEmpty()) {
return false;
}
before the stream call
There was a problem hiding this comment.
I asume you are referring to something like this:
if (distributionSet.getModules().isEmpty()) {
return false;
}
final Set<SoftwareModuleType> mandatoryModuleTypes = getMandatoryModuleTypes();
return distributionSet.getModules().stream().map(SoftwareModule::getType)
.allMatch(type -> mandatoryModuleTypes.contains(type));
This has a problem. It returns false even when I assign a software module to this type. In theory, this is a case where a DS type has no mandatory SM type. But it has a SM assigned. So it must be marked complete right?
There was a problem hiding this comment.
I have a question, why do we need to change the original implementation?
There was a problem hiding this comment.
Its for 2 reasons:
- An incomplete DS should not be visible in deployment view.
- If all software modules are removed from a given DS, then this DS should be marked incomplete.
There was a problem hiding this comment.
Ok, now I got it.
Maybe a good idea here is to introduce some unit tests? specially if a Model class has some logic like this
There was a problem hiding this comment.
You can maybe use the DistributionSetTypeManagementTest test, and test the different use cases where this method is called, here it's called in constructor, addModule, and removeModule
There was a problem hiding this comment.
Discussed with @stefbehl to add the test cases later as an improvement. The tests will be added in the next sprint.
schabdo
changed the title
DistributionSets without SoftwareModules shall not be marked as complete, when no Software Module is assigned Signed-off-by: Ravindranath Sandeep (INST-IOT/ESW-Imb) <Sandeep.Ravindranath@bosch-si.com>
A DistributionSet (DS) can only be assigned to a target, when it is marked as complete.
A DS is marked as complete, when all requirements of the selected DS-Type are fulfilled.
A DS-Type determines which SoftwareModules (SM) of which SM-Type can be (optional) or have to be (required) contained.
When specifying a DS-Type that contains only optional SMs, a DS is marked as complete even without any SM assigned. This shall be changed.
A DS shall be only considered as complete, when:
SMs of the required SM-Types are assigned
At least one SM is assigned, in case all SM-Types are optional