Skip to content

Commit

Permalink
#19 exclude data from stale included tabs which are not in current scope
Browse files Browse the repository at this point in the history
  • Loading branch information
teosarca committed Mar 2, 2017
1 parent 3e7acdb commit 486d406
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import de.metas.logging.LogManager;
import de.metas.ui.web.exceptions.InvalidDocumentVersionException;
import de.metas.ui.web.window.datatypes.DocumentPath;
import de.metas.ui.web.window.model.DocumentChangesCollector;
import de.metas.ui.web.window.model.IDocumentChangesCollector;
import de.metas.ui.web.window.model.NullDocumentChangesCollector;
Expand Down Expand Up @@ -87,6 +88,11 @@ public static <T> T callInNewExecution(final String name, final Callable<T> call
.name(name)
.execute(callable);
}

public static IAutoCloseable setScope(final DocumentPath documentPath)
{
return getCurrentDocumentChangesCollectorOrNull().setScope(documentPath);
}

public static IDocumentChangesCollector getCurrentDocumentChangesCollector()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import de.metas.ui.web.window.WindowConstants;
import de.metas.ui.web.window.datatypes.DataTypes;
import de.metas.ui.web.window.datatypes.DocumentId;
import de.metas.ui.web.window.datatypes.DocumentPath;
import de.metas.ui.web.window.descriptor.DetailId;
import de.metas.ui.web.window.descriptor.DocumentFieldWidgetType;
Expand Down Expand Up @@ -47,22 +48,21 @@
public final class DocumentChanges
{
private final DocumentPath documentPath;
private boolean inScope;

private final Map<String, DocumentFieldChange> fieldChangesByName = new LinkedHashMap<>();
private DocumentValidStatus documentValidStatus = null;
private DocumentSaveStatus documentSaveStatus = null;

private final Set<DetailId> staleDetailIds = new HashSet<>();

/* package */ DocumentChanges(final DocumentPath documentPath)
/* package */ DocumentChanges(final DocumentPath documentPath, final boolean inScope)
{
super();

Preconditions.checkNotNull(documentPath, "documentPath");
this.documentPath = documentPath;
}

public DocumentPath getDocumentPath()
{
return documentPath;
this.inScope = inScope;
}

@Override
Expand All @@ -71,11 +71,28 @@ public String toString()
return MoreObjects.toStringHelper(this)
.omitNullValues()
.add("documentPath", documentPath)
.add("fields", fieldChangesByName.isEmpty() ? null : fieldChangesByName)
.add("inScope", inScope)
.add("fields.count", fieldChangesByName.isEmpty() ? null : fieldChangesByName.size())
.add("staleDetailIds", staleDetailIds.isEmpty() ? null : staleDetailIds)
.toString();
}

public DocumentPath getDocumentPath()
{
return documentPath;
}

public boolean isInScope()
{
return inScope;
}

public DocumentChanges setInScope()
{
inScope = true;
return this;
}

public Set<String> getFieldNames()
{
return ImmutableSet.copyOf(fieldChangesByName.keySet());
Expand All @@ -89,13 +106,19 @@ public boolean isEmpty()
&& staleDetailIds.isEmpty();
}

private DocumentFieldChange fieldChangesOf(final IDocumentFieldView documentField)
private final void assertMatchingDocumentPath(final DocumentPath documentPath, final Object entity)
{
// Make sure the field is about same document path
if (!documentPath.equals(documentField.getDocumentPath()))
if (!this.documentPath.equals(documentPath))
{
throw new IllegalArgumentException("Field " + documentField + " does not have expected path: " + documentPath);
throw new IllegalArgumentException("Document path not matching for " + entity + ", expected " + this.documentPath + " but it was " + documentPath);
}
}

private DocumentFieldChange fieldChangesOf(final IDocumentFieldView documentField)
{
// Make sure the field is about same document path
assertMatchingDocumentPath(documentField.getDocumentPath(), documentField);

return fieldChangesByName.computeIfAbsent(documentField.getFieldName(), (fieldName) -> {
final DocumentFieldChange event = DocumentFieldChange.of(fieldName, documentField.isKey(), documentField.isPublicField(), documentField.isAdvancedField(), documentField.getWidgetType());
Expand Down Expand Up @@ -266,6 +289,14 @@ public DocumentSaveStatus getDocumentSaveStatus()
Check.assumeNotNull(detailId, "Parameter detailId is not null");
staleDetailIds.add(detailId);
}

/* package */void collectStaleIncludedDocument(final DetailId detailId, final DocumentId includedDocumentId)
{
Check.assumeNotNull(detailId, "Parameter detailId is not null");
staleDetailIds.add(detailId);

// TODO: collect includedDocumentId that got staled
}

public Set<DetailId> getStaleDetailIds()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@

import org.adempiere.ad.expression.api.LogicExpressionResult;
import org.adempiere.util.GuavaCollectors;
import org.adempiere.util.lang.IAutoCloseable;
import org.slf4j.Logger;

import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;

import de.metas.logging.LogManager;
import de.metas.printing.esb.base.util.Check;
import de.metas.ui.web.window.datatypes.DataTypes;
import de.metas.ui.web.window.datatypes.DocumentPath;
Expand Down Expand Up @@ -43,6 +48,9 @@ public static final DocumentChangesCollector newInstance()
return new DocumentChangesCollector();
}

private static final Logger logger = LogManager.getLogger(DocumentChangesCollector.class);

private DocumentPath scopeDocumentPath = null;
private final Map<DocumentPath, DocumentChanges> documentChangesByPath = new LinkedHashMap<>();

private DocumentChangesCollector()
Expand All @@ -58,6 +66,66 @@ public String toString()
.toString();
}

@Override
public IAutoCloseable setScope(final DocumentPath documentPath)
{
Preconditions.checkNotNull(documentPath);

final DocumentPath scopeDocumentPathPrevious = scopeDocumentPath;
scopeDocumentPath = documentPath;
logger.debug("Scope changed to {}", scopeDocumentPath);

//
// If there are changes already reported for the documentPath we are just setting in scope,
// flag those changes as in scope too
// Reason/Case: create a new included document...
if (documentPath != null)
{
final DocumentChanges documentChanges = documentChangesIfExists(documentPath);
if(documentChanges != null)
{
documentChanges.setInScope();
}
}

return new IAutoCloseable()
{
private boolean restored = false;

@Override
public void close()
{
if (restored)
{
return;
}
restored = true;

final DocumentPath scopeDocumentPathCurrent = scopeDocumentPath;
if (!Objects.equal(scopeDocumentPathCurrent, documentPath))
{
logger.warn("Unexpected current documentPath in scope: expected={}, actual={}", documentPath, scopeDocumentPathCurrent);
}

scopeDocumentPath = scopeDocumentPathPrevious;
logger.debug("Scope restored to {}", scopeDocumentPathPrevious);
}
};
}

private final boolean isInScope(final DocumentPath documentPath)
{
final DocumentPath scopeDocumentPath = this.scopeDocumentPath;

// No scope restrictions
if (scopeDocumentPath == null)
{
return true;
}

return scopeDocumentPath.equals(documentPath);
}

private DocumentChanges documentChanges(final IDocumentFieldView documentField)
{
final DocumentPath documentPath = documentField.getDocumentPath();
Expand All @@ -66,7 +134,15 @@ private DocumentChanges documentChanges(final IDocumentFieldView documentField)

private DocumentChanges documentChanges(final DocumentPath documentPath)
{
return documentChangesByPath.computeIfAbsent(documentPath, DocumentChanges::new);
final DocumentChanges documentChanges = documentChangesByPath.computeIfAbsent(documentPath, newDocumentPath -> new DocumentChanges(newDocumentPath, isInScope(newDocumentPath)));

if (documentPath.hasIncludedDocuments())
{
documentChanges(documentPath.getRootDocumentPath())
.collectStaleIncludedDocument(documentPath.getDetailId(), documentPath.getSingleRowId());
}

return documentChanges;
}

private DocumentChanges documentChangesIfExists(final DocumentPath documentPath)
Expand Down Expand Up @@ -214,26 +290,32 @@ public void collectStaleDetailId(final DocumentPath rootDocumentPath, final Deta

private boolean isStaleDocumentChanges(final DocumentChanges documentChanges)
{
// The documentPath in scope shall never be considered as stale
if(documentChanges.isInScope())
{
return false; // not stale
}

final DocumentPath documentPath = documentChanges.getDocumentPath();
if (!documentPath.isSingleIncludedDocument())
{
return false;
return false; // not stale
}

final DocumentPath rootDocumentPath = documentPath.getRootDocumentPath();
final DocumentChanges rootDocumentChanges = documentChangesIfExists(rootDocumentPath);
if (rootDocumentChanges == null)
{
return false;
return false; // not stale
}

final DetailId detailId = documentPath.getDetailId();
if (rootDocumentChanges.isDetailIdStaled(detailId))
{
return true;
return true; // stale
}

return false;
return false; // not stale
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ private <R> R forDocumentReadonly(final DocumentPath documentPath //
return null;
}

try (final IAutoCloseable readLock = rootDocumentForLocking.lockForReading())
try (final IAutoCloseable scope = Execution.setScope(documentPath);
final IAutoCloseable readLock = rootDocumentForLocking.lockForReading())
{
final Document rootDocument = rootDocumentSupplier.apply(rootDocumentKey);
if (documentPath.isRootDocument())
Expand All @@ -178,7 +179,8 @@ public <R> R forRootDocumentReadonly(final DocumentPath documentPath, final Func
{
final DocumentKey rootDocumentKey = DocumentKey.ofRootDocumentPath(documentPath.getRootDocumentPath());

try (final IAutoCloseable readLock = rootDocuments.getUnchecked(rootDocumentKey).lockForReading())
try (final IAutoCloseable scope = Execution.setScope(documentPath);
final IAutoCloseable readLock = rootDocuments.getUnchecked(rootDocumentKey).lockForReading())
{
final Document rootDocument = rootDocuments.getUnchecked(rootDocumentKey);
return rootDocumentProcessor.apply(rootDocument);
Expand All @@ -204,7 +206,10 @@ else if (documentPath.isSingleNewIncludedDocument())
document = rootDocument.getIncludedDocument(documentPath.getDetailId(), documentPath.getSingleRowId());
}

return documentProcessor.apply(document);
try (final IAutoCloseable scope = Execution.setScope(document.getDocumentPath()))
{
return documentProcessor.apply(document);
}
});
}

Expand All @@ -215,21 +220,25 @@ public <R> R forRootDocumentWritable(final DocumentPath documentPathOrNew, final
final Document lockHolder;
final boolean isNewRootDocument;
final DocumentKey rootDocumentKey;
final DocumentPath documentPath;
if (rootDocumentPathOrNew.isNewDocument())
{
final Document newRootDocument = createRootDocument(rootDocumentPathOrNew);
lockHolder = newRootDocument;
rootDocumentKey = DocumentKey.ofRootDocumentPath(newRootDocument.getDocumentPath());
documentPath = newRootDocument.getDocumentPath();
rootDocumentKey = DocumentKey.ofRootDocumentPath(documentPath);
isNewRootDocument = true;
}
else
{
documentPath = rootDocumentPathOrNew;
rootDocumentKey = DocumentKey.ofRootDocumentPath(rootDocumentPathOrNew);
lockHolder = rootDocuments.getUnchecked(rootDocumentKey);
isNewRootDocument = false;
}

try (final IAutoCloseable readLock = lockHolder.lockForWriting())
try (final IAutoCloseable scope = Execution.setScope(documentPath);
final IAutoCloseable readLock = lockHolder.lockForWriting())
{
final Document rootDocument;
if (isNewRootDocument)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.Set;

import org.adempiere.ad.expression.api.LogicExpressionResult;
import org.adempiere.util.lang.IAutoCloseable;

import de.metas.ui.web.window.WindowConstants;
import de.metas.ui.web.window.datatypes.DocumentPath;
Expand Down Expand Up @@ -33,6 +34,8 @@

public interface IDocumentChangesCollector
{
IAutoCloseable setScope(DocumentPath documentPath);

Map<DocumentPath, DocumentChanges> getDocumentChangesByPath();

void collectValueChanged(IDocumentFieldView documentField, ReasonSupplier reason);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
import java.util.Map;
import java.util.Set;

import javax.annotation.concurrent.Immutable;

import org.adempiere.ad.expression.api.LogicExpressionResult;
import org.adempiere.util.lang.IAutoCloseable;
import org.adempiere.util.lang.NullAutoCloseable;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
Expand All @@ -24,24 +28,34 @@
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public
* License along with this program. If not, see
* License along with this program. If not, see
* <http://www.gnu.org/licenses/gpl-2.0.html>.
* #L%
*/

@Immutable
public final class NullDocumentChangesCollector implements IDocumentChangesCollector
{
public static final transient NullDocumentChangesCollector instance = new NullDocumentChangesCollector();

// NOTE: no fields are allowed

private NullDocumentChangesCollector()
{
super();
}

@Override
public IAutoCloseable setScope(final DocumentPath documentPath)
{
// does nothing
return NullAutoCloseable.instance;
}

@Override
public Map<DocumentPath, DocumentChanges> getDocumentChangesByPath()
{
Expand Down

0 comments on commit 486d406

Please sign in to comment.