Idan Shaby has uploaded a new change for review. Change subject: Revert "restapi: optimize getUriBuilder" ......................................................................
Revert "restapi: optimize getUriBuilder" This reverts commit d83d67ff715c0707b1715e09249532ca14c7e21a. The original patch added a static Map that cached UriBuilders. This made the test "testGet" in "BackendHostNicLabelResourceTest" to fail in some cases, depends on the order JUnit runs the tests. Change-Id: I18db692bcfd0de26bb57481657b7645a01acb7d3 Signed-off-by: Idan Shaby <[email protected]> --- M backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java 2 files changed, 13 insertions(+), 54 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/65/38165/1 diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java index a48591e..49dfd1f7 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/utils/LinkHelper.java @@ -21,17 +21,13 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.Map.Entry; -import java.util.concurrent.ConcurrentHashMap; import javax.ws.rs.Path; import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; import org.apache.commons.lang.StringUtils; -import org.apache.commons.lang.builder.EqualsBuilder; -import org.apache.commons.lang.builder.HashCodeBuilder; import org.ovirt.engine.api.model.ActionableResource; import org.ovirt.engine.api.model.ActionsBuilder; import org.ovirt.engine.api.model.AffinityGroup; @@ -303,11 +299,6 @@ * A constant representing the pseudo-parent of a top-level collection */ private static final Class<? extends BaseResource> NO_PARENT = BaseResource.class; - - /** - * Cache the {@code UriBuilder}s by the associated model so we don't have to create the builders each time. - */ - private static Map<Collection, UriBuilder> builderMap = new ConcurrentHashMap<Collection, UriBuilder>(); /** * A map describing every possible collection @@ -772,8 +763,12 @@ return null; } - if (suggestedParentType != null && collections.get(suggestedParentType) != null) { - return collections.get(suggestedParentType); + if (suggestedParentType != null) { + for (Entry<Class<? extends BaseResource>, Collection> entry : collections.entrySet()) { + if (entry.getKey().equals(suggestedParentType)) { + return entry.getValue(); + } + } } for (Entry<Class<? extends BaseResource>, Collection> parentTypeEntry : collections.entrySet()) { @@ -818,6 +813,7 @@ } UriBuilder uriBuilder; + if (collection.getParentType() != NO_PARENT) { BaseResource parent = getParentModel(model, collection.getParentType()); @@ -829,19 +825,12 @@ uriBuilder = getUriBuilder(uriInfo, parent).path(path); } else { - uriBuilder = builderMap.get(collection); - if (uriBuilder == null) { - String path = getPath(collection.getCollectionType()); - uriBuilder = uriInfo != null - ? UriBuilder.fromPath(uriInfo.getBaseUri().getPath()).path(path) - : UriBuilder.fromPath(path); - builderMap.put(collection, uriBuilder.clone()); - } else { - //We need to clone so we have our own copy to work with. Cloning is much faster than building a new one - //from scratch each time. - uriBuilder = uriBuilder.clone(); - } + String path = getPath(collection.getCollectionType()); + uriBuilder = uriInfo != null + ? UriBuilder.fromPath(uriInfo.getBaseUri().getPath()).path(path) + : UriBuilder.fromPath(path); } + return uriBuilder.path(model.getId()); } @@ -1201,34 +1190,6 @@ public Class<?> getResourceType() { return resourceType; } public Class<?> getCollectionType() { return collectionType; } public Class<?> getParentType() { return parentType; } - - @Override - public int hashCode() { - HashCodeBuilder hashCodeBuilder = new HashCodeBuilder(); - hashCodeBuilder.append(resourceType); - hashCodeBuilder.append(collectionType); - hashCodeBuilder.append(parentType); - return hashCodeBuilder.toHashCode(); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj == null) { - return false; - } - if (!(obj instanceof Collection)) { - return false; - } - Collection other = (Collection)obj; - EqualsBuilder equalsBuilder = new EqualsBuilder(); - equalsBuilder.append(resourceType, other.resourceType); - equalsBuilder.append(collectionType, other.collectionType); - equalsBuilder.append(parentType, other.parentType); - return equalsBuilder.isEquals(); - } } /** diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java index 63f5263..0a8a0d6 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java @@ -7,7 +7,6 @@ import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; -import javax.ws.rs.core.UriBuilder; import org.ovirt.engine.api.model.BaseResource; import org.ovirt.engine.api.model.CreationStatus; @@ -260,10 +259,9 @@ protected R linkSubCollections(R model, Class<? extends BaseResource> suggestedParent, String... subCollectionMembersToExclude) { if (subCollections != null) { - UriBuilder uriBuilder = LinkHelper.getUriBuilder(getUriInfo(), model, suggestedParent); for (String relation : subCollections) { if(!shouldExclude(relation, subCollectionMembersToExclude)) { - addOrUpdateLink(model, relation, uriBuilder.clone().path(relation).build().toString()); + addOrUpdateLink(model, relation, LinkHelper.getUriBuilder(getUriInfo(), model, suggestedParent).path(relation).build().toString()); } else{ removeIfExist(model, relation); -- To view, visit https://gerrit.ovirt.org/38165 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I18db692bcfd0de26bb57481657b7645a01acb7d3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
