-
Notifications
You must be signed in to change notification settings - Fork 318
Add LogRecord table update callbacks #2159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,9 +69,9 @@ | |
import net.sourceforge.ganttproject.storage.InputXlog; | ||
import net.sourceforge.ganttproject.storage.ProjectDatabaseException; | ||
import net.sourceforge.ganttproject.storage.ServerCommitResponse; | ||
import net.sourceforge.ganttproject.storage.XlogRecord; | ||
import net.sourceforge.ganttproject.task.CustomColumnsStorage; | ||
import net.sourceforge.ganttproject.task.Task; | ||
import net.sourceforge.ganttproject.task.event.TaskListenerAdapter; | ||
import net.sourceforge.ganttproject.undo.GPUndoListener; | ||
import org.apache.commons.lang3.tuple.ImmutablePair; | ||
import org.jetbrains.annotations.NotNull; | ||
|
@@ -82,15 +82,14 @@ | |
import java.awt.*; | ||
import java.awt.event.*; | ||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.HashMap; | ||
import java.util.*; | ||
import java.util.List; | ||
import java.util.concurrent.atomic.AtomicReference; | ||
import java.util.function.Consumer; | ||
|
||
import static biz.ganttproject.storage.cloud.GPCloudHttpImplKt.getWebSocket; | ||
import static biz.ganttproject.storage.cloud.GPCloudHttpImplKt.isColloboqueLocalTest; | ||
import static net.sourceforge.ganttproject.storage.XlogKt.EMPTY_LOG_BASE_TXN_ID; | ||
|
||
/** | ||
* Main frame of the project | ||
|
@@ -167,7 +166,7 @@ private static class TxnCommitInfo { | |
/** If `oldTxnId` is currently being hold, sets the txn ID to `newTxnId` and moves the local ID ahead by `committedNum`. */ | ||
void update(String oldTxnId, String newTxnId, int committedNum) { | ||
myTxnId.updateAndGet(oldValue -> { | ||
if (oldValue.left.equals(oldTxnId)) { | ||
if (Objects.equals(oldValue.left, oldTxnId)) { | ||
return new ImmutablePair<>(newTxnId, oldValue.right + committedNum); | ||
} else { | ||
return oldValue; | ||
|
@@ -178,9 +177,13 @@ void update(String oldTxnId, String newTxnId, int committedNum) { | |
ImmutablePair<String, Integer> get() { | ||
return myTxnId.get(); | ||
} | ||
|
||
void reset() { | ||
myTxnId.set(new ImmutablePair<>(null, 0)); | ||
} | ||
} | ||
|
||
private final TxnCommitInfo myBaseTxnCommitInfo = new TxnCommitInfo("", 0); | ||
private final TxnCommitInfo myBaseTxnCommitInfo = new TxnCommitInfo(null, 0); | ||
|
||
|
||
public GanttProject(boolean isOnlyViewer) { | ||
|
@@ -208,10 +211,6 @@ public GanttProject(boolean isOnlyViewer) { | |
getWebSocket().register(null); | ||
getWebSocket().onCommitResponseReceived(this::fireXlogReceived); | ||
getWebSocket().onBaseTxnIdReceived(this::onBaseTxnIdReceived); | ||
var taskListenerAdapter = new TaskListenerAdapter(); | ||
// TODO: add listeners sensibly. | ||
taskListenerAdapter.setTaskAddedHandler(event -> this.sendProjectStateLogs()); | ||
getTaskManager().addTaskListener(taskListenerAdapter); | ||
} | ||
|
||
area = new GanttGraphicArea(this, getTaskManager(), getZoomManager(), getUndoManager(), | ||
|
@@ -956,33 +955,73 @@ public void refresh() { | |
super.repaint(); | ||
} | ||
|
||
// TODO: Accumulate changes instead of sending it every time. | ||
private interface TxnSendListener { | ||
void onSendCompleted(); | ||
} | ||
|
||
private final AtomicReference<TxnSendListener> txnSendingListener = new AtomicReference<>(); | ||
|
||
private Unit sendProjectStateLogs() { | ||
gpLogger.debug("Sending project state logs"); | ||
if (txnSendingListener.get() != null) return Unit.INSTANCE; | ||
var baseTxnCommitInfo = myBaseTxnCommitInfo.get(); | ||
if (baseTxnCommitInfo.left == null) { | ||
// Connection with the server was not established. | ||
return Unit.INSTANCE; | ||
} | ||
List<XlogRecord> transactions; | ||
try { | ||
var baseTxnCommitInfo = myBaseTxnCommitInfo.get(); | ||
var txns = myProjectDatabase.fetchTransactions(baseTxnCommitInfo.right + 1, 1); | ||
if (!txns.isEmpty()) { | ||
transactions = myProjectDatabase.fetchTransactions(baseTxnCommitInfo.right + 1, 1); | ||
} catch (ProjectDatabaseException e) { | ||
gpLogger.error("Failed to send logs", new Object[]{}, ImmutableMap.of(), e); | ||
return Unit.INSTANCE; | ||
} | ||
if (!transactions.isEmpty()) { | ||
var listener = new TxnSendListener() { | ||
@Override | ||
public void onSendCompleted() { | ||
txnSendingListener.compareAndSet(this, null); | ||
} | ||
}; | ||
if (txnSendingListener.compareAndSet(null, listener)) { | ||
getWebSocket().sendLogs(new InputXlog( | ||
baseTxnCommitInfo.left, | ||
"userId", | ||
"refid", | ||
txns | ||
transactions | ||
)); | ||
} else { | ||
// Logs were sent by another thread, no action required. | ||
} | ||
} catch (ProjectDatabaseException e) { | ||
gpLogger.error("Failed to send logs", new Object[]{}, ImmutableMap.of(), e); | ||
} | ||
return Unit.INSTANCE; | ||
} | ||
|
||
@Override | ||
protected Unit onProjectLogUpdate() { | ||
if (isColloboqueLocalTest()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like that we have more and more these checks. I thought that we would have just a few differences between the "local test" and "prod" modes (url, authentication), but now it seems that we will just not do anything in prod. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove this check? And maybe the super call too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, in this case we shall check "if we are working with an online document, in any sense", not "is it a local online document setup". We probably need to modify the local dev server so that it could respond to project read and write operations, just like a regular GP Cloud server. In this case the check may look like |
||
super.onProjectLogUpdate(); | ||
sendProjectStateLogs(); | ||
} | ||
return Unit.INSTANCE; | ||
} | ||
|
||
private Unit fireXlogReceived(ServerCommitResponse response) { | ||
myBaseTxnCommitInfo.update(response.getBaseTxnId(), response.getNewBaseTxnId(), 1); | ||
txnSendingListener.get().onSendCompleted(); | ||
sendProjectStateLogs(); | ||
return Unit.INSTANCE; | ||
} | ||
|
||
// TODO: sync logs with the server. | ||
private Unit onBaseTxnIdReceived(String baseTxnId) { | ||
myBaseTxnCommitInfo.update("", baseTxnId, 0); | ||
// Websocket is [re-]started. Previous messages are discarded. | ||
txnSendingListener.set(null); | ||
if (EMPTY_LOG_BASE_TXN_ID.equals(baseTxnId)) { | ||
myBaseTxnCommitInfo.reset(); | ||
myBaseTxnCommitInfo.update(null, baseTxnId, 0); | ||
sendProjectStateLogs(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove it? Sending logs doesn't make too much sense if we just have reset the state. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In such a case, we'll send all the logs from the beginning, one by one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Send the logs when an undoable edit happens. But I don't think that a user has any chance to make an undoable edit immediately at the moment when we reset the log. |
||
} | ||
return Unit.INSTANCE; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.