Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Issue/487 copied instance properties #590

Closed
wants to merge 11 commits into from
Closed

Issue/487 copied instance properties #590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c646d1d
create a new propertydescriptor and persist it if the property is par…
mburri Jan 21, 2021
3eb204c
fix typos in comment
mburri Jan 21, 2021
8d2db8e
extract method to create map of existingPropertiesByDescriptorId
mburri Jan 21, 2021
b647000
extract method to create new or merge existing property
mburri Jan 21, 2021
2e21cda
extract method to get the target property from the map of existion pr…
mburri Jan 21, 2021
c850bc2
cleanup code to get the target property from the existing properties
mburri Jan 21, 2021
36b5213
renamed methods and improved doc
mburri Jan 21, 2021
12468bf
fix intellij inspection warnings
mburri Jan 21, 2021
e7447c5
invert (by intellij) expression to avoid empty if block
mburri Jan 21, 2021
64aa72a
add test to cover case where property value is merged
mburri Jan 21, 2021
b4bcbd6
Merge branch 'master' into issue/487-copied-instance-properties
yvespp Apr 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 94 additions & 46 deletions ...java/ch/puzzle/itc/mobiliar/business/resourcegroup/control/CopyResourceDomainService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ protected CopyResourceResult doCopyResourceAndSave(CopyUnit copyUnit) throws For
protected void copyConsumedMasterRelations(CopyUnit copyUnit) throws ForeignableOwnerViolationException {
Set<ConsumedResourceRelationEntity> targetConsumedMasterRel = copyUnit.getTargetResource()
.getConsumedMasterRelations() != null ? copyUnit.getTargetResource()
.getConsumedMasterRelations() : new HashSet<ConsumedResourceRelationEntity>();
.getConsumedMasterRelations() : new HashSet<>();
Set<ConsumedResourceRelationEntity> originConsumedMasterRel = copyUnit.getOriginResource()
.getConsumedMasterRelations();
copyConsumedResourceRelationEntities(originConsumedMasterRel, targetConsumedMasterRel, copyUnit);
Expand All @@ -231,7 +231,7 @@ protected void copyConsumedMasterRelations(CopyUnit copyUnit) throws Foreignable
protected void copyConsumedSlaveRelations(CopyUnit copyUnit) throws ForeignableOwnerViolationException {
Set<ConsumedResourceRelationEntity> targetConsumedSlaveRel = copyUnit.getTargetResource()
.getConsumedSlaveRelations() != null ? copyUnit.getTargetResource()
.getConsumedSlaveRelations() : new HashSet<ConsumedResourceRelationEntity>();
.getConsumedSlaveRelations() : new HashSet<>();
Set<ConsumedResourceRelationEntity> originConsumedSlaveRel = copyUnit.getOriginResource()
.getConsumedSlaveRelations();
if (copyUnit.getMode() == CopyMode.RELEASE) {
Expand Down Expand Up @@ -306,7 +306,7 @@ private boolean shallReplaceRuntime(Set<ConsumedResourceRelationEntity> original
protected void copyProvidedMasterRelations(CopyUnit copyUnit) throws ForeignableOwnerViolationException {
Set<ProvidedResourceRelationEntity> targetProvidedResRels = copyUnit.getTargetResource()
.getProvidedMasterRelations() != null ? copyUnit.getTargetResource()
.getProvidedMasterRelations() : new HashSet<ProvidedResourceRelationEntity>();
.getProvidedMasterRelations() : new HashSet<>();
Set<ProvidedResourceRelationEntity> originProvidedResRels = copyUnit.getOriginResource()
.getProvidedMasterRelations();
if (copyUnit.getMode() == CopyMode.RELEASE || copyUnit.getMode() == CopyMode.MAIA_PREDECESSOR) {
Expand All @@ -325,7 +325,7 @@ protected void copyProvidedMasterRelations(CopyUnit copyUnit) throws Foreignable
protected void copyProvidedSlaveRelations(CopyUnit copyUnit) throws ForeignableOwnerViolationException {
Set<ProvidedResourceRelationEntity> targetProvidedSlaveRels = copyUnit.getTargetResource()
.getProvidedSlaveRelations() != null ? copyUnit.getTargetResource()
.getProvidedSlaveRelations() : new HashSet<ProvidedResourceRelationEntity>();
.getProvidedSlaveRelations() : new HashSet<>();
Set<ProvidedResourceRelationEntity> originProvidedSlaveRels = copyUnit.getOriginResource()
.getProvidedSlaveRelations();
if (copyUnit.getMode() == CopyMode.RELEASE) {
Expand Down Expand Up @@ -541,7 +541,7 @@ private void copyPropertyDescriptor(ContextDependency<?> targetContextDependency
}

protected String createDescriptorKey(PropertyDescriptorEntity desc) {
return desc.getPropertyName() + "_" + String.valueOf(desc.isTesting());
return desc.getPropertyName() + "_" + desc.isTesting();
}

protected void copyTags(PropertyDescriptorEntity origin, PropertyDescriptorEntity target) {
Expand Down Expand Up @@ -588,7 +588,7 @@ protected ContextDependency<?> copyContextDependency(ContextDependency<?> origin
/**
* <ul>
* <li>The identifier between target propertyDescriptor and origin TargetDescriptor is the propertyName (= technicalKey)</li>
* <li>If a propertyDescript of the targetResources has already a properyValue, this value will not be overwritten.
* <li>If a propertyDescriptor of the targetResources has already a propertyValue, this value will not be overwritten.
* </ul>
*
* @param origins, all properties of the origin resource for one context
Expand All @@ -599,55 +599,103 @@ protected Set<PropertyEntity> copyProperties(Set<PropertyEntity> origins,
Map<String, PropertyDescriptorEntity> targetPropDescriptorMap,
Set<PropertyEntity> targetProperties,
CopyUnit copyUnit) {
Map<Integer, PropertyEntity> existingPropertiesByDescriptorId = new HashMap<>();
if (targetProperties != null) {
for (PropertyEntity existingProperty : targetProperties) {
if (existingProperty.getDescriptor() != null && existingProperty.getDescriptor().getId() != null) {
existingPropertiesByDescriptorId.put(existingProperty.getDescriptor().getId(), existingProperty);
}
}
}

Map<Integer, PropertyEntity> existingPropertiesByDescriptorId = groupPropertiesByDescriptorId(targetProperties);

Set<PropertyEntity> targets = new HashSet<>();
if (origins != null) {
for (PropertyEntity origin : origins) {
// If a property exists on this context for the same descriptor, we define it as the
// target property...
PropertyEntity targetProperty = existingPropertiesByDescriptorId.get(origin.getDescriptor().getId());
PropertyDescriptorEntity targetDescriptor = null;
if (targetProperty == null) {
// If it can't be found, it's possible that we have copied the target descriptor.
// Let's look for it.
String key = createDescriptorKey(origin.getDescriptor());
targetDescriptor = targetPropDescriptorMap.get(key);
if (targetDescriptor != null) {
// If a property is already defined for the existing descriptor, we update this
// value...
targetProperty = existingPropertiesByDescriptorId.get(targetDescriptor.getId());
}
}
String key = createDescriptorKey(origin.getDescriptor());
PropertyDescriptorEntity targetDescriptor = targetPropDescriptorMap.get(key);

if (CopyMode.MAIA_PREDECESSOR == copyUnit.getMode() && targetDescriptor == null) {
// do not add property for null descriptor when Predecessor mode
} else {
if (targetProperty == null) {
// If no property for the found property descriptor exists, we create a new one...
PropertyEntity target = origin.getCopy(null, copyUnit);
// targetDescriptor null come for properties on ResourceTypes or relations
if (targetDescriptor != null) {
target.setDescriptor(targetDescriptor);
}
targets.add(target);
} else {
// otherwise, we merge the new value with the old property entity
targets.add(mergePropertyEntity(origin, targetProperty));
}
// do not add property for null descriptor when Predecessor mode
if (CopyMode.MAIA_PREDECESSOR != copyUnit.getMode() || targetDescriptor != null) {
targets.add(getTargetProperty(copyUnit, origin, targetDescriptor, existingPropertiesByDescriptorId));
}
}
}
return targets;
}

/**
* Creates a new {@link PropertyEntity} if the targetProperty is null or merges the new value with the {@link PropertyEntity} from origin
*
* @param copyUnit
* @param origin
* @param targetDescriptor
* @param existingPropertiesByDescriptorId
* @return
*/
private PropertyEntity getTargetProperty(CopyUnit copyUnit, PropertyEntity origin, PropertyDescriptorEntity targetDescriptor, Map<Integer, PropertyEntity> existingPropertiesByDescriptorId) {

PropertyEntity targetProperty = getTargetPropertyFromExistingProperties(existingPropertiesByDescriptorId, origin, targetDescriptor);
return targetProperty == null ?
createNewPropertyEntityFromOrigin(copyUnit, origin, targetDescriptor) :
mergePropertyEntity(origin, targetProperty);
}

/**
* Try to get the new target property from the existing properties. If it does not exist but there is a propertydescriptor
* - reuse this property
*
* @param existingPropertiesByDescriptorId
* @param origin
* @param targetDescriptor
* @return
*/
private PropertyEntity getTargetPropertyFromExistingProperties(Map<Integer, PropertyEntity> existingPropertiesByDescriptorId, PropertyEntity origin, PropertyDescriptorEntity targetDescriptor) {
PropertyEntity targetProperty = existingPropertiesByDescriptorId.get(origin.getDescriptor().getId());
if (targetProperty == null && targetDescriptor != null) {
targetProperty = existingPropertiesByDescriptorId.get(targetDescriptor.getId());
}
return targetProperty;
}

/**
* Create a copy of the given origin propertyEntity - and set the property descriptor. If there is one available (targetDescriptor) use
* this one, otherwise create and persist a new property descriptor.
* The target descriptor is set if it comes from the resource type
*
* @param copyUnit
* @param origin
* @param targetDescriptor
* @return
*/
private PropertyEntity createNewPropertyEntityFromOrigin(CopyUnit copyUnit, PropertyEntity origin, PropertyDescriptorEntity targetDescriptor) {
PropertyEntity propertyEntity = origin.getCopy(null, copyUnit);
if (targetDescriptor != null) {
propertyEntity.setDescriptor(targetDescriptor);
} else {
copyAndPersistPropertyDescriptor(copyUnit, propertyEntity);
}
return propertyEntity;
}

/**
* creates and persist a copy of the existing propertyDescriptor on the given propertyEntity
* fixes issue https://github.com/liimaorg/liima/issues/487
*
* @param copyUnit
* @param propertyEntity
*/
private void copyAndPersistPropertyDescriptor(CopyUnit copyUnit, PropertyEntity propertyEntity) {
PropertyDescriptorEntity propertyDescriptorEntity = propertyEntity.getDescriptor().getCopy(new PropertyDescriptorEntity(), copyUnit);
propertyEntity.setDescriptor(propertyDescriptorEntity);
entityManager.persist(propertyEntity.getDescriptor());
}

private Map<Integer, PropertyEntity> groupPropertiesByDescriptorId(Set<PropertyEntity> targetProperties) {
Map<Integer, PropertyEntity> existingPropertiesByDescriptorId = new HashMap<>();
if (targetProperties != null) {
for (PropertyEntity existingProperty : targetProperties) {
if (existingProperty.getDescriptor() != null && existingProperty.getDescriptor().getId() != null) {
existingPropertiesByDescriptorId.put(existingProperty.getDescriptor().getId(), existingProperty);
}
}
}
return existingPropertiesByDescriptorId;
}

/**
* Merges the value (and comment) of the original property into the target property.
*/
Expand All @@ -666,14 +714,14 @@ protected Set<TemplateDescriptorEntity> copyTemplates(Set<TemplateDescriptorEnti
Map<String, TemplateDescriptorEntity> targetTemplatesMap = new HashMap<>();
if (targets != null) {
for (TemplateDescriptorEntity t : targets) {
String key = t.getName() + String.valueOf(t.isTesting());
String key = t.getName() + t.isTesting();
targetTemplatesMap.put(key, t);
}
}

if (origins != null) {
for (TemplateDescriptorEntity origin : origins) {
String key = origin.getName() + String.valueOf(origin.isTesting());
String key = origin.getName() + origin.isTesting();
targetTemplatesMap.put(key, origin.getCopy(targetTemplatesMap.get(key), copyUnit));
}
}
Expand Down
53 changes: 50 additions & 3 deletions ...c/mobiliar/business/resourcegroup/control/CopyResourceDomainServiceParameterizedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,9 @@ public void copyProperties() throws ForeignableOwnerViolationException, AMWExcep
}

/**
* If a propertyDescript of the targetResources has already a properyValue, this value should not be
* If a propertyDescriptor of the targetResources has already a propertyValue, this value should not be
* overwritten.
*
*
* @param propDescOwner
* @throws ForeignableOwnerViolationException
*/
Expand Down Expand Up @@ -467,11 +467,58 @@ private void shouldSetTargetPropsFromOriginIfTargetDoesNotExist(CopyUnit copyUni
assertEquals(targetPropDesc, prop.getDescriptor());
}


private void shouldMergePropertyValuesIfTargetPropertyExists(CopyUnit copyUnit, ForeignableOwner propDescOwner) throws ForeignableOwnerViolationException {

// given
ContextEntity context = contextEntityBuilder.mockContextEntity("GLOBAL", null, null);

ResourceRelationContextEntity origin = resRelContextBuilder.mockResourceRelationContextEntity(context);
ResourceRelationContextEntity target = resRelContextBuilder.buildResourceRelationContextEntity(context);

String descName = "descriptorA";
String origPropValue = "origPropValue";

// Original descriptor with property value
PropertyDescriptorEntity originPropDesc = createPropDesc(1, descName, origPropValue, propDescOwner);
when(origin.getPropertyDescriptors()).thenReturn(Collections.singleton(originPropDesc));
when(origin.getProperties()).thenReturn(Collections.singleton(originPropDesc.getProperties().iterator().next()));


// Target descriptor without property value
PropertyDescriptorEntity targetPropDesc = createPropDesc(10, descName, null, propDescOwner);
target.addPropertyDescriptor(targetPropDesc);

// add a property to the target context

PropertyEntity targetProperty = new PropertyEntityBuilder().buildPropertyEntity("my-value", origin.getPropertyDescriptors().iterator().next());
targetProperty.setDescriptor(targetPropDesc);
target.addProperty(targetProperty);


// when
Map<String, PropertyDescriptorEntity> descriptorMap = copyDomainService.copyPropertyDescriptors(
origin.getPropertyDescriptors(), target.getPropertyDescriptors(), target, copyUnit);
ContextDependency<?> copy = copyDomainService.copyContextDependency(origin, target, copyUnit, descriptorMap);

// then
assertNotNull(copy);
assertTrue(copyUnit.getResult().isSuccess());
assertEquals(1, copy.getPropertyDescriptors().size());
PropertyDescriptorEntity desc = copy.getPropertyDescriptors().iterator().next();
assertNotNull(desc);
assertEquals(1, copy.getProperties().size());
PropertyEntity prop = copy.getProperties().iterator().next();
assertNotNull(prop);
assertEquals(origPropValue, prop.getValue());
assertEquals(targetPropDesc, prop.getDescriptor());
}

/**
* If a propertyDescriptor from origin does not yet exist on targetResource the descriptor and the
* propertyValue should be copied from origin. EXCEPT for {@link CopyMode#MAIA_PREDECESSOR}; if a
* propertyDescriptor AMW does not exist in target then it should not be copied!
*
*
* @param copyUnit
* @param propDescOwner
* @throws ForeignableOwnerViolationException
Expand Down