Permalink
Browse files

Only take version if it's lower than the one mapped

  • Loading branch information...
Miggets7 committed Nov 19, 2018
1 parent ba455ef commit 2e93449b523eafdb080fde55fae871694a23b6a6
@@ -25,10 +25,10 @@
import org.openremote.manager.web.ManagerWebResource;
import org.openremote.model.Constants;
import org.openremote.model.asset.*;
import org.openremote.model.query.AssetQuery;
import org.openremote.model.query.BaseAssetQuery.Select;
import org.openremote.model.attribute.*;
import org.openremote.model.http.RequestParams;
import org.openremote.model.query.AssetQuery;
import org.openremote.model.query.BaseAssetQuery.Select;
import org.openremote.model.query.filter.ParentPredicate;
import org.openremote.model.query.filter.TenantPredicate;
import org.openremote.model.security.Tenant;
@@ -45,8 +45,9 @@
import static javax.ws.rs.core.Response.Status.*;
import static org.openremote.container.Container.JSON;
import static org.openremote.model.query.AssetQuery.*;
import static org.openremote.model.attribute.AttributeEvent.Source.CLIENT;
import static org.openremote.model.query.AssetQuery.Access;
import static org.openremote.model.query.AssetQuery.Include;
import static org.openremote.model.util.TextUtil.isNullOrEmpty;
public class AssetResourceImpl extends ManagerWebResource implements AssetResource {
@@ -241,13 +242,13 @@ public void update(RequestParams requestParams, String assetId, Asset asset) {
boolean isRestrictedUser = isRestrictedUser();
// The asset that will ultimately be stored (override/ignore some values for restricted users)
Asset resultAsset = storageAsset.map(
Asset resultAsset = Asset.map(
asset,
storageAsset,
isRestrictedUser ? storageAsset.getName() : null, // TODO We could allow restricted users to update names?
isRestrictedUser ? storageAsset.getRealmId() : null, // Restricted users can not change realm
isRestrictedUser ? storageAsset.getParentId() : null, // Restricted users can not change realm
storageAsset.getType(), // The type an never change
storageAsset.getType(), // The type can never change
isRestrictedUser ? storageAsset.isAccessPublicRead() : null, // Restricted user can not change access public flag
isRestrictedUser ? storageAsset.getAttributes() : null // Restricted users need manual attribute merging (see below)
);
@@ -702,7 +702,11 @@ public static Asset map(Asset assetToMap, Asset asset,
String overrideType,
Boolean overrideAccessPublicRead,
ObjectValue overrideAttributes) {
asset.setVersion(assetToMap.getVersion());
if (asset.getVersion() < assetToMap.getVersion()) {
asset.setVersion(assetToMap.getVersion());
}
asset.setName(overrideName != null ? overrideName : assetToMap.getName());
if (overrideType != null) {
asset.setType(overrideType);

4 comments on commit 2e93449

@richturner

This comment has been minimized.

Contributor

richturner replied Nov 19, 2018

@Miggets7 I'm not sure about this change...looks like trying to avoid a DB exception when trying to merge an out of sync asset...the version check is there specifically for that reason...front end apps should ensure they have the latest version loaded before trying to merge an asset.

@Miggets7

This comment has been minimized.

Collaborator

Miggets7 replied Nov 19, 2018

Then I would recommend to return a 409 CONFLICT, to let the client now it's not up to date.

@richturner

This comment has been minimized.

Contributor

richturner replied Nov 19, 2018

Yes sounds sensible....general flow should be that front end app should subscribe to asset modifications via web socket and if asset being edited is modified outside then user must reload and redo changes otherwise potentially valid modifications are lost. But apps should cope with update failure and 409 code would fit well and allow frontend app to either reload asset and re-apply changes or be more sophisticated and show potential merge conflicts and allow user to decide how to resolve.

@Miggets7

This comment has been minimized.

Collaborator

Miggets7 replied Nov 19, 2018

Fixed in: 1ab0d7d

Please sign in to comment.