Skip to content

Commit

Permalink
#10711: Prefer field.variables() instead of reloading (#11557) (#11561)
Browse files Browse the repository at this point in the history
  • Loading branch information
@agomez-dotcms @oarrietadotcms
agomez-dotcms authored and oarrietadotcms committed May 12, 2017
1 parent da809ce commit 795ce67
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 51 deletions.
11 changes: 6 additions & 5 deletions dotCMS/src/integration-test/java/com/dotcms/contenttype/test/FieldFactoryImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,12 @@ public void testFieldVariables() throws Exception {


for (Field field : fields) {
List<FieldVariable> vars = FactoryLocator.getFieldFactory().loadVariables(field);
List<FieldVariable> vars = field.fieldVariables();
for(FieldVariable var : vars){
fieldApi.delete(var);
}
vars = fieldApi.loadVariables(field);

vars = FactoryLocator.getFieldFactory().loadVariables(field);

assertThat("No field vars for field " + field, vars.size() ==0);

Expand All @@ -103,9 +104,9 @@ public void testFieldVariables() throws Exception {
.build();

var = fieldApi.save(var, APILocator.systemUser());
assertThat("field var saved correctly " + var,var.equals(fieldApi.loadVariable(var.id())));
assertThat("field var saved correctly " + var, var.equals(FactoryLocator.getFieldFactory().loadVariable(var.id())));

vars = fieldApi.loadVariables(field);
vars = FactoryLocator.getFieldFactory().loadVariables(field);
assertThat("field var added for field " + field, vars.size() ==i+1);

}
Expand All @@ -115,7 +116,7 @@ public void testFieldVariables() throws Exception {

for(FieldVariable var : vars){
fieldApi.delete(var);
assertThat("field deleted correctly " + var,--mySize == fieldApi.loadVariables(field).size());
assertThat("field deleted correctly " + var,--mySize == FactoryLocator.getFieldFactory().loadVariables(field).size());
}
}
}
Expand Down
18 changes: 0 additions & 18 deletions dotCMS/src/main/java/com/dotcms/contenttype/business/FieldAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,24 +185,6 @@ default FieldAPI instance() {
*/
void delete(FieldVariable fieldVar) throws DotDataException;

/**
* Retrieves the Field Variables related to a specific Field.
*
* @param field Field to search on
* @return List of Field Variables Objects that are related to a Field
* @throws DotDataException Error occurred when performing the action.
*/
List<FieldVariable> loadVariables(Field field) throws DotDataException;

/**
* Retrieves a Field Variable given its id
*
* @param id Id of the field Variable.
* @return Field Variable related to the given id.
* @throws DotDataException Error occurred when performing the action.
*/
FieldVariable loadVariable(String id) throws DotDataException;

/**
* Returns the dataType and the number of the column of that field. e.g bool1
* Only one Host Field and Tag Field per Content Type is allowed.
Expand Down
10 changes: 0 additions & 10 deletions dotCMS/src/main/java/com/dotcms/contenttype/business/FieldAPIImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,16 +256,6 @@ public void delete(FieldVariable fieldVar) throws DotDataException {
fac.delete(fieldVar);

}

@Override
public List<FieldVariable> loadVariables(Field field) throws DotDataException {
return fac.loadVariables(field);
}

@Override
public FieldVariable loadVariable(String id) throws DotDataException {
return fac.loadVariable(id);
}



Expand Down
10 changes: 10 additions & 0 deletions dotCMS/src/main/java/com/dotcms/contenttype/business/FieldFactoryImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,16 @@ public void delete(FieldVariable fieldVar) throws DotDataException {
return null;
});

Field f = byId(fieldVar.fieldId());
ContentType t;
try {
t = APILocator.getContentTypeAPI(APILocator.systemUser()).find(f.contentTypeId());
if (t != null) {
CacheLocator.getContentTypeCache2().remove(t);
}
} catch (DotSecurityException e) {
throw new DotStateException(e);
}
}

@Override
Expand Down
13 changes: 13 additions & 0 deletions dotCMS/src/main/java/com/dotcms/contenttype/model/field/Field.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import java.io.Serializable;
import java.util.Calendar;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import org.elasticsearch.common.Nullable;
import org.immutables.value.Value;
Expand All @@ -22,6 +24,7 @@
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.JsonSubTypes.Type;
import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;
import com.google.common.collect.ImmutableMap;

@JsonTypeInfo(
use = Id.CLASS,
Expand Down Expand Up @@ -186,6 +189,16 @@ public List<FieldVariable> fieldVariables() {

}

@JsonIgnore
@Value.Lazy
public Map<String, FieldVariable> fieldVariablesMap() {
Map<String, FieldVariable> fmap = new HashMap<>();
for (FieldVariable fv : this.fieldVariables()) {
fmap.put(fv.id(), fv);
}
return ImmutableMap.copyOf(fmap);
}

private List<FieldVariable> innerFieldVariables = null;

public void constructFieldVariables(List<FieldVariable> fieldVariables) {
Expand Down
4 changes: 1 addition & 3 deletions dotCMS/src/main/java/com/dotcms/contenttype/util/ContentTypeImportExportUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ private void streamingJsonExport(File file, int run) throws DotDataException, Do

List<FieldVariable> fieldVariables=new ArrayList<FieldVariable>();
for(Field ff : fields) {
fieldVariables.addAll(
APILocator.getContentTypeFieldAPI().loadVariables(ff)
);
fieldVariables.addAll(ff.fieldVariables());
}

mapper.writeValue(jg, new ContentTypeWrapper(contentType,fields,fieldVariables));
Expand Down
24 changes: 16 additions & 8 deletions dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/FieldVariableResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public final Response getFieldVariablesByFieldId(@PathParam("typeId") final Stri
try {
Field field = fapi.find(fieldId);

List<FieldVariable> fieldVariables = fapi.loadVariables(field);
List<FieldVariable> fieldVariables = field.fieldVariables();

response = Response.ok(new ResponseEntityView(new JsonFieldVariableTransformer(fieldVariables).mapList())).build();

Expand Down Expand Up @@ -196,7 +196,7 @@ public final Response getFieldVariablesByFieldVar(@PathParam("typeId") final Str
try {
Field field = fapi.byContentTypeIdAndVar(typeId, fieldVar);

List<FieldVariable> fieldVariables = fapi.loadVariables(field);
List<FieldVariable> fieldVariables = field.fieldVariables();

response = Response.ok(new ResponseEntityView(new JsonFieldVariableTransformer(fieldVariables).mapList())).build();

Expand Down Expand Up @@ -230,7 +230,7 @@ public Response getFieldVariableByFieldId(@PathParam("typeId") final String type

Field field = fapi.find(fieldId);

FieldVariable fieldVariable = fapi.loadVariable(fieldVarId);
FieldVariable fieldVariable = getFieldVariable(field, fieldVarId);

if (!field.id().equals(fieldVariable.fieldId())) {

Expand Down Expand Up @@ -270,7 +270,7 @@ public Response getFieldVariableByFieldVar(@PathParam("typeId") final String typ

Field field = fapi.byContentTypeIdAndVar(typeId, fieldVar);

FieldVariable fieldVariable = fapi.loadVariable(fieldVarId);
FieldVariable fieldVariable = getFieldVariable(field, fieldVarId);

if (!field.id().equals(fieldVariable.fieldId())) {

Expand Down Expand Up @@ -325,7 +325,7 @@ public Response updateFieldVariableByFieldId(@PathParam("typeId") final String t

} else {

FieldVariable currentFieldVariable = fapi.loadVariable(fieldVarId);
FieldVariable currentFieldVariable = getFieldVariable(field, fieldVarId);

if (!currentFieldVariable.id().equals(fieldVariable.id())) {

Expand Down Expand Up @@ -381,7 +381,7 @@ public Response updateFieldVariableByFieldVar(@PathParam("typeId") final String

} else {

FieldVariable currentFieldVariable = fapi.loadVariable(fieldVarId);
FieldVariable currentFieldVariable = getFieldVariable(field, fieldVarId);

if (!currentFieldVariable.id().equals(fieldVariable.id())) {

Expand Down Expand Up @@ -425,7 +425,7 @@ public Response deleteFieldVariableByFieldId(@PathParam("typeId") final String t

Field field = fapi.find(fieldId);

FieldVariable fieldVariable = fapi.loadVariable(fieldVarId);
FieldVariable fieldVariable = getFieldVariable(field, fieldVarId);

if (!fieldVariable.fieldId().equals(field.id())) {

Expand Down Expand Up @@ -468,7 +468,7 @@ public Response deleteFieldVariableByFieldVar(@PathParam("typeId") final String

Field field = fapi.byContentTypeIdAndVar(typeId, fieldVar);

FieldVariable fieldVariable = fapi.loadVariable(fieldVarId);
FieldVariable fieldVariable = getFieldVariable(field, fieldVarId);

if (!fieldVariable.fieldId().equals(field.id())) {

Expand All @@ -492,4 +492,12 @@ public Response deleteFieldVariableByFieldVar(@PathParam("typeId") final String

return response;
}

private FieldVariable getFieldVariable(Field field, String fieldVarId) throws NotFoundInDbException {
FieldVariable result = field.fieldVariablesMap().get(fieldVarId);
if (result == null) {
throw new NotFoundInDbException("Field variable with id:" + fieldVarId + " not found");
}
return result;
}
}
9 changes: 4 additions & 5 deletions dotCMS/src/main/java/com/dotmarketing/cache/FieldsCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,11 @@ public static void removeField(Field field) {

public static List<FieldVariable> getFieldVariables(Field field) {
try {

return new FieldVariableTransformer(
APILocator.getContentTypeFieldAPI().loadVariables(
new LegacyFieldTransformer(field).from()
)
).oldFieldList();
} catch (DotStateException | DotDataException e) {
new LegacyFieldTransformer(field).from().fieldVariables()
).oldFieldList();
} catch (DotStateException e) {
throw new DotStateException(e);
}
}
Expand Down
5 changes: 3 additions & 2 deletions dotCMS/src/main/java/com/dotmarketing/portlets/structure/factories/FieldFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static com.dotcms.util.CollectionsUtils.set;
import com.dotmarketing.business.APILocator;
import com.dotmarketing.business.DotStateException;
import com.dotmarketing.business.FactoryLocator;
import com.dotmarketing.exception.DotDataException;
import com.dotmarketing.exception.DotHibernateException;
import com.dotmarketing.exception.DotSecurityException;
Expand Down Expand Up @@ -261,7 +262,7 @@ public static FieldVariable saveFieldVariable(FieldVariable fieldVar){
*/
public static FieldVariable getFieldVariable(String id){
try {
return new FieldVariableTransformer(fapi().loadVariable(id)).oldField();
return new FieldVariableTransformer(FactoryLocator.getFieldFactory().loadVariable(id)).oldField();
} catch (DotStateException | DotDataException e) {
Logger.error(FieldFactory.class, e.getMessage());
}
Expand Down Expand Up @@ -308,7 +309,7 @@ public static List<FieldVariable> getFieldVariablesForField (String fieldId ){
public static List<FieldVariable> getFieldVariablesForField (Field field ){
try {
com.dotcms.contenttype.model.field.Field newfield = fapi().find(field.getInode());
List<com.dotcms.contenttype.model.field.FieldVariable > fl = fapi().loadVariables(newfield);
List<com.dotcms.contenttype.model.field.FieldVariable > fl = newfield.fieldVariables();
return new FieldVariableTransformer(fl).oldFieldList();
} catch (DotStateException | DotDataException e) {
Logger.error(FieldFactory.class, e.getMessage());
Expand Down

0 comments on commit 795ce67

Please sign in to comment.