#2340 Modifications after code reviews

#2340
metasfresh · Sep 18, 2017 · eb8fb33 · eb8fb33
1 parent 9a280c3
commit eb8fb33
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 81 deletions.
diff --git a/...piere/base/src/main/java-legacy/org/adempiere/model/AbstractRelationTypeZoomProvider.java b/...piere/base/src/main/java-legacy/org/adempiere/model/AbstractRelationTypeZoomProvider.java
@@ -1,7 +1,6 @@
 package org.adempiere.model;
 
 import java.util.List;
-import java.util.Properties;
 
 import javax.annotation.Nullable;
 
@@ -53,6 +52,11 @@ public abstract class AbstractRelationTypeZoomProvider implements IZoomProvider
 	protected int adRelationTypeId;
 	protected static final Logger logger = LogManager.getLogger(RelationTypeZoomProvider.class);
 
+	/**
+	 * Each zoom provider must have a target reference
+	 */
+	protected static ZoomProviderDestination target;
+
 	public boolean isDirected()
 	{
 		return directed;
@@ -73,10 +77,6 @@ public int getAD_RelationType_ID()
 		return adRelationTypeId;
 	}
 
-	protected abstract ZoomProviderDestination getTarget();
-
-	protected abstract String getTargetTableName();
-
 	protected static void updateRecordsCountAndZoomValue(final MQuery query)
 	{
 		final String sqlCommon = " FROM " + query.getZoomTableName() + " WHERE " + query.getWhereClause(false);
@@ -94,7 +94,13 @@ protected static void updateRecordsCountAndZoomValue(final MQuery query)
 		}
 	}
 
-	public abstract <T> List<T> retrieveDestinations(final Properties ctx, final PO sourcePO, final Class<T> clazz, final String trxName);
+	public ZoomProviderDestination getTarget()
+	{
+		return target;
+	}
+
+
+	public abstract <T> List<T> retrieveDestinations( final PO sourcePO, final Class<T> clazz, final String trxName);
 
 	protected static class ZoomProviderDestination
 	{

diff --git a/...ase/src/main/java-legacy/org/adempiere/model/ReferenceTargetRelationTypeZoomProvider.java b/...ase/src/main/java-legacy/org/adempiere/model/ReferenceTargetRelationTypeZoomProvider.java
@@ -55,16 +55,16 @@
 public class ReferenceTargetRelationTypeZoomProvider extends AbstractRelationTypeZoomProvider
 {
 
-	private ZoomProviderDestination target;
+
 
 	private ReferenceTargetRelationTypeZoomProvider(final Builder builder)
 	{
 		super();
 
-		directed = builder.isDirected();
 		zoomInfoId = builder.getZoomInfoId();
 		internalName = builder.getInternalName();
 		adRelationTypeId = builder.getAD_RelationType_ID();
+
 		target = new ZoomProviderDestination(builder.getTarget_Reference_ID(), builder.getTargetTableRefInfoOrNull(), builder.getTargetRoleDisplayName());
 	}
 
@@ -75,7 +75,7 @@ public String toString()
 				.omitNullValues()
 				.add("zoomInfoId", zoomInfoId)
 				.add("internalName", internalName)
-				.add("directed", directed)
+				.add("adRelationTypeId", adRelationTypeId)
 				.toString();
 	}
 
@@ -88,11 +88,11 @@ private MQuery mkQuery(final IZoomSource zoomSource, final ZoomProviderDestinati
 		final ITableRefInfo refTable = target.getTableRefInfo();
 
 		final Properties ctx = zoomSource.getCtx();
-		
+
 		final I_AD_Column referenceTargetColumn = InterfaceWrapperHelper.create(ctx, refTable.getReferenceTargetColumnID(), I_AD_Column.class, ITrx.TRXNAME_None);
-		
+
 		Check.assumeNotNull(referenceTargetColumn, "No ReferenceTarget Column defined for the AD_Ref_Table entry " + refTable);
-		
+
 		queryWhereClause
 				.append(zoomSource.getAD_Table_ID())
 				.append(" = ")
@@ -138,21 +138,15 @@ public List<ZoomInfo> retrieveZoomInfos(IZoomSource zoomSource, int targetAD_Win
 		return ImmutableList.of(ZoomInfo.of(zoomInfoId, adWindowId, query, display));
 	}
 
-	@Override
-	protected ZoomProviderDestination getTarget()
-	{
-		return target;
-	}
+
 
 	@Override
-	protected String getTargetTableName()
-	{
-		return target.getTableName();
-	}
-
-	@Override
-	public <T> List<T> retrieveDestinations(Properties ctx, PO sourcePO, Class<T> clazz, String trxName)
+	public <T> List<T> retrieveDestinations(final PO sourcePO, final Class<T> clazz, final String trxName)
 	{
+		Check.assumeNotNull(sourcePO, "Parameter sourcePO is not null");
+
+		final Properties ctx = sourcePO.getCtx();
+
 		final IZoomSource zoomSource = POZoomSource.of(sourcePO, -1);
 
 		final MQuery query = mkQuery(zoomSource, getTarget());
@@ -174,12 +168,11 @@ public static final class Builder
 	{
 		private final transient ILookupDAO lookupDAO = Services.get(ILookupDAO.class);
 
-		private Boolean directed;
 		private String internalName;
 		private int adRelationTypeId;
 		private boolean isReferenceTarget;
 
-		private int targetReferenceId = -1;
+		private int targetReferenceId;
 		private ITranslatableString targetRoleDisplayName;
 		private ITableRefInfo targetTableRefInfo = null; // lazy
 
@@ -227,18 +220,6 @@ private String getInternalName()
 			return internalName;
 		}
 
-		public Builder setDirected(final boolean directed)
-		{
-			this.directed = directed;
-			return this;
-		}
-
-		private boolean isDirected()
-		{
-			Check.assumeNotNull(directed, "Parameter directed is not null");
-			return directed;
-		}
-
 		public Builder setTarget_Reference_ID(final int targetReferenceId)
 		{
 			this.targetReferenceId = targetReferenceId;
@@ -278,10 +259,6 @@ public Builder setIsReferenceTarget(boolean isReferenceTarget)
 			return this;
 		}
 
-		private boolean isReferenceTarget()
-		{
-			return isReferenceTarget;
-		}
 	}
 
 }
diff --git a/...ere.adempiere/base/src/main/java-legacy/org/adempiere/model/RelationTypeZoomProvider.java b/...ere.adempiere/base/src/main/java-legacy/org/adempiere/model/RelationTypeZoomProvider.java
@@ -127,26 +127,11 @@ public List<ZoomInfo> retrieveZoomInfos(final IZoomSource zoomSource, final int
 		return ImmutableList.of(ZoomInfo.of(zoomInfoId, adWindowId, query, display));
 	}
 
-	protected ZoomProviderDestination getTarget()
-	{
-		return target;
-	}
-
-	public String getTargetTableName()
-	{
-		return target.getTableName();
-	}
-
-	protected ZoomProviderDestination getSource()
+	private ZoomProviderDestination getSource()
 	{
 		return source;
 	}
 
-	public String getSourceTableName()
-	{
-		return source.getTableName();
-	}
-
 	/**
 	 * @return effective source and target pair
 	 */
@@ -247,11 +232,16 @@ private static String parseWhereClause(final IZoomSource source, final String wh
 		return whereParsed;
 	}
 
-	public <T> List<T> retrieveDestinations(final Properties ctx, final PO sourcePO, final Class<T> clazz, final String trxName)
+	public <T> List<T> retrieveDestinations( final PO sourcePO, final Class<T> clazz, final String trxName)
 	{
+		Check.assumeNotNull(sourcePO, "Parameter sourcePO is not null");
+
+		final Properties ctx = sourcePO.getCtx();
+
 		final IZoomSource zoomSource = POZoomSource.of(sourcePO, -1);
 
 		final ZoomProviderDestination target = findSourceAndTargetEffective(zoomSource).getRight();
+
 		final MQuery query = mkQuery(zoomSource, target);
 
 		return new Query(ctx, query.getZoomTableName(), query.getWhereClause(false), trxName)

diff --git a/...piere/base/src/main/java-legacy/org/adempiere/model/RelationTypeZoomProvidersFactory.java b/...piere/base/src/main/java-legacy/org/adempiere/model/RelationTypeZoomProvidersFactory.java
@@ -11,7 +11,6 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
-import java.util.Properties;
 import java.util.Set;
 
 import org.adempiere.ad.service.IADReferenceDAO;
@@ -62,6 +61,9 @@ public final class RelationTypeZoomProvidersFactory
 
 	public static final transient RelationTypeZoomProvidersFactory instance = new RelationTypeZoomProvidersFactory();
 
+	// services
+	public static final transient IADReferenceDAO adReferenceDAO = Services.get(IADReferenceDAO.class);
+
 	/**
 	 * Selection for those relation types whose AD_Reference(s) might match a given PO. Only evaluates the table and key
 	 * column of the reference's AD_Ref_Table entries. See {@link #retrieveTypes(PO, int)}.
@@ -114,7 +116,7 @@ public final class RelationTypeZoomProvidersFactory
 			+ "    AND rt." + I_AD_RelationType.COLUMNNAME_IsReferenceTarget + " = 'Y'"
 			+ "    AND ref.IsActive='Y'" //
 			+ "    AND ref.ValidationType='T'" // must have table validation
-	
+
 			+ "    AND (" // join the source AD_Reference
 			+ "      rt.AD_Reference_Source_ID is null" //
 			+ "      AND " // not directed? -> also join the target AD_Reference
@@ -152,7 +154,7 @@ public List<ReferenceTargetRelationTypeZoomProvider> retrieveReferenceZoomProvid
 
 		final POInfo poInfo = POInfo.getPOInfo(tableName);
 		final String keyColumnName = poInfo.getKeyColumnName();
-		if (keyColumnName == null)
+		if (Check.isEmpty(keyColumnName))
 		{
 			logger.error("{} does not have a single key column", tableName);
 			throw PORelationException.throwWrongKeyColumnCount(tableName, poInfo.getKeyColumnNames());
@@ -195,7 +197,8 @@ private List<RelationTypeZoomProvider> retrieveZoomProvidersBySourceTableName(fi
 
 		final POInfo poInfo = POInfo.getPOInfo(tableName);
 		final String keyColumnName = poInfo.getKeyColumnName();
-		if (keyColumnName == null)
+
+		if (Check.isEmpty(keyColumnName))
 		{
 			logger.error("{} does not have a single key column", tableName);
 			throw PORelationException.throwWrongKeyColumnCount(tableName, poInfo.getKeyColumnNames());
@@ -238,43 +241,42 @@ private List<RelationTypeZoomProvider> retrieveZoomProvidersBySourceTableName(fi
 	 */
 	private static List<ReferenceTargetRelationTypeZoomProvider> retrieveReferenceTargetZoomProviders(final ResultSet rs) throws SQLException
 	{
-		final IADReferenceDAO adReferenceDAO = Services.get(IADReferenceDAO.class);
-		final Properties ctx = Env.getCtx();
-
 		final List<ReferenceTargetRelationTypeZoomProvider> result = new ArrayList<>();
 		final Set<Integer> alreadySeen = new HashSet<>();
 
 		while (rs.next())
 		{
 			final int adRelationTypeId = rs.getInt(I_AD_RelationType.COLUMNNAME_AD_RelationType_ID);
+
 			if (!alreadySeen.add(adRelationTypeId))
 			{
 				continue;
 			}
 
-			final I_AD_RelationType relationType = InterfaceWrapperHelper.create(ctx, adRelationTypeId, I_AD_RelationType.class, ITrx.TRXNAME_None);
+			final I_AD_RelationType relationType = InterfaceWrapperHelper.create(Env.getCtx(), adRelationTypeId, I_AD_RelationType.class, ITrx.TRXNAME_None);
 
 			final ADRefListItem roleTargetItem = adReferenceDAO.retrieveListItemOrNull(X_AD_RelationType.ROLE_TARGET_AD_Reference_ID, relationType.getRole_Target());
+
 			final ITranslatableString roleTargetDisplayName = roleTargetItem == null ? null : roleTargetItem.getName();
 
 			try
 			{
 				final ReferenceTargetRelationTypeZoomProvider zoomProvider = ReferenceTargetRelationTypeZoomProvider.builder()
-						.setDirected(relationType.isDirected())
+
 						.setAD_RelationType_ID(relationType.getAD_RelationType_ID())
 						.setInternalName(relationType.getInternalName())
 						//
 						.setTarget_Reference_ID(relationType.getAD_Reference_Target_ID())
 						.setTargetRoleDisplayName(roleTargetDisplayName)
+						.setIsReferenceTarget(true)
 						//
 						.buildOrNull();
 
-				if (zoomProvider == null)
+				if (zoomProvider != null)
 				{
-					continue;
+					result.add(zoomProvider);
 				}
 
-				result.add(zoomProvider);
 			}
 			catch (Exception ex)
 			{
@@ -284,10 +286,15 @@ private static List<ReferenceTargetRelationTypeZoomProvider> retrieveReferenceTa
 		return result;
 	}
 
+	/**
+	 * Retrieve general kind of zoom providers, that have both source and target and their references are of type TableValidation
+	 * 
+	 * @param rs
+	 * @return
+	 * @throws SQLException
+	 */
 	private static List<RelationTypeZoomProvider> retrieveZoomProviders(final ResultSet rs) throws SQLException
 	{
-		final IADReferenceDAO adReferenceDAO = Services.get(IADReferenceDAO.class);
-		final Properties ctx = Env.getCtx();
 
 		final List<RelationTypeZoomProvider> result = new ArrayList<>();
 		final Set<Integer> alreadySeen = new HashSet<>();
@@ -300,7 +307,7 @@ private static List<RelationTypeZoomProvider> retrieveZoomProviders(final Result
 				continue;
 			}
 
-			final I_AD_RelationType relationType = InterfaceWrapperHelper.create(ctx, adRelationTypeId, I_AD_RelationType.class, ITrx.TRXNAME_None);
+			final I_AD_RelationType relationType = InterfaceWrapperHelper.create(Env.getCtx(), adRelationTypeId, I_AD_RelationType.class, ITrx.TRXNAME_None);
 
 			final ADRefListItem roleSourceItem = adReferenceDAO.retrieveListItemOrNull(X_AD_RelationType.ROLE_SOURCE_AD_Reference_ID, relationType.getRole_Source());
 			final ITranslatableString roleSourceDisplayName = roleSourceItem == null ? null : roleSourceItem.getName();
@@ -323,12 +330,10 @@ private static List<RelationTypeZoomProvider> retrieveZoomProviders(final Result
 						//
 
 						.buildOrNull();
-				if (zoomProvider == null)
+				if (zoomProvider != null)
 				{
-					continue;
+					result.add(zoomProvider);
 				}
-
-				result.add(zoomProvider);
 			}
 			catch (Exception ex)
 			{

diff --git a/de.metas.adempiere.adempiere/base/src/main/java/org/adempiere/ad/service/impl/LookupDAO.java b/de.metas.adempiere.adempiere/base/src/main/java/org/adempiere/ad/service/impl/LookupDAO.java
@@ -157,10 +157,10 @@ public static TableRefInfoBuilder builder()
 
 		// #2340
 		// Flag to mark a ref_table as ReferenceTarget
-		private boolean isReferenceTarget;
+		private final boolean isReferenceTarget;
 		// #2340
 		// Specify which [table_]Record_ID column is the wanter ReferenceTargetColumn; only relevant if the isReferenceTarget flag is true
-		private int referenceTargetColumnID;
+		private final int referenceTargetColumnID;
 
 
 		private TableRefInfo(final TableRefInfoBuilder builder)

diff --git a/...etas.swat/de.metas.swat.base/src/main/java/de/metas/ordercandidate/api/impl/OLCandBL.java b/...etas.swat/de.metas.swat.base/src/main/java/de/metas/ordercandidate/api/impl/OLCandBL.java
@@ -138,7 +138,7 @@ public void process(
 		final String relationTypeInternalName = mkRelationTypeInternalName(processor);
 		final List<I_C_OLCand> allCandidates = RelationTypeZoomProvidersFactory.instance
 				.getZoomProviderBySourceTableNameAndInternalName(I_C_OLCand.Table_Name, relationTypeInternalName)
-				.retrieveDestinations(ctx, InterfaceWrapperHelper.getPO(processor), I_C_OLCand.class, trxName);
+				.retrieveDestinations(InterfaceWrapperHelper.getPO(processor), I_C_OLCand.class, trxName);
 
 		if (allCandidates.isEmpty())
 		{