Details

Reviewers

atul
ginsu
ashoat

Summary

This is the final step of https://linear.app/comm/issue/ENG-2072/maintain-the-spoiler-state-upon-opening-textmessagetooltipmodal, where messages will retain their 'revealed' state when tapped to reveal the tooltip. With this diff, spoilers for native should now be fully functional, with two remaining tasks to just handle notifications and message previews.

Depends on D5539

Test Plan

Tested the states being retained both visually by revealing specific spoilers and then tapping a message, and also by logging the newly created objects to track the expected behavior.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

rohan created this revision.Nov 7 2022, 4:28 AM

Herald added subscribers: • abosh, tomek, ashoat. · View Herald TranscriptNov 7 2022, 4:28 AM

rohan edited the summary of this revision. (Show Details)Nov 7 2022, 4:29 AM

rohan edited the test plan for this revision. (Show Details)

rohan attached a referenced file: F225076: Spoilers.mp4. (Show Details)

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 7 2022, 4:36 AM

Harbormaster failed remote builds in B13193: Diff 18099!

Bypassing Android CI since it currently has ongoing problems

In D5541#164089, @rohan wrote:

Bypassing Android CI since it currently has ongoing problems

(When you get a second can you run git pull --rebase && arc diff to retrigger Android CI with latest changes)

Rebase for CI

Harbormaster failed remote builds in B13228: Diff 18152!Nov 7 2022, 9:45 AM

Harbormaster completed remote builds in B13228: Diff 18152.Nov 7 2022, 10:02 AM

Same feedback in D5539 about using React.useMemo instead of React.useState for holding onto style

native/markdown/markdown-spoiler.react.js
30–35	Since the style is an "output" of the state we should use a `React.useMemo` instead of `React.useState` that changes in response to changes in state. That keeps things declarative and we don't have to imperatively call `setStyleBasedOnState` anytime the style should change.

This revision now requires changes to proceed.Nov 8 2022, 7:11 AM

rohan mentioned this in D5539: [native] Maintain the revealed state of spoilers.Nov 9 2022, 10:22 AM

Rebase, resolve conflicts, and use React.useMemo for spoiler style

Harbormaster completed remote builds in B13338: Diff 18290.Nov 9 2022, 11:29 AM

rohan added a child revision: D5597: [IGNORE] GestureTouchableOpacity Spoiler changes.Nov 10 2022, 9:21 AM

Logic makes sense, just one question inline

native/markdown/markdown-context.js
12 ↗	(On Diff #18290)	I am guessing this new line was an accident

Two things to be addressed before land-able

Clean up spoilerIdentifier naming/type/handling
Stop using styleBasedOnSpoilerState in subsequent logic... styles should be an "output" not an "input"

native/chat/inner-text-message.react.js
105 ↗	(On Diff #18290)	Let's rename this `messageContextValue` just to be a bit more specific
105–107 ↗	(On Diff #18290)	Can we use `React.useMemo` here to avoid unnecessarily re-computing the `contextValue` object?
native/markdown/markdown-context.js
12 ↗	(On Diff #18290)	Yeah let's remove this newline to keep `git` history clean
native/markdown/markdown-spoiler.react.js
26 ↗	(On Diff #18290)	Can we instead call this something like `spoilerIdentifier` so it's more descriptive? What are we trying to achieve with the `casted` prefix? Should we be typing `identifier` as a `number`? I'm guessing it's typed as `string \| number \| void` because that's what `key` is typed as? If that's the case, is it fine for `castedIdentifier` to equal `identifier` if `parseInt` fails/is falsey? Or would that break things? I'd assume using `void` as an object key would lead to some broken behavior.
39 ↗	(On Diff #18290)	We don't want to be using styles in our logic like this. Can we use the same `castedIdentifier && spoilerRevealed?.[messageID]?.[castedIdentifier]` conditional?

This revision now requires changes to proceed.Nov 14 2022, 2:17 PM

rohan added inline comments.Nov 15 2022, 5:48 AM

native/markdown/markdown-spoiler.react.js
26 ↗	(On Diff #18290)	Sure, rename will come in the revision. As for the `casted` prefix, it's just a way to identify that it's not the original `identifier` typed as `string \| number \| void` - instead, it's now a `number` instead of a string. Though a better naming convention would probably be `parsedSpoilerIdentifier` since we're calling `parseInt` instead of casting it. To your other point, `state.key` is typed as `string \| number \| void` so it has to have the same type (`identifier`), though I agree - I think I better check would be: const { text, spoilerIdentifier } = props; const parsedSpoilerIdentifier = spoilerIdentifier ? parseInt(spoilerIdentifier) : -1;

Address the comments from @atul

Harbormaster completed remote builds in B13433: Diff 18427.Nov 15 2022, 6:04 AM

Rebase on previous diff in stack (should be successful)

Harbormaster completed remote builds in B13440: Diff 18438.Nov 15 2022, 10:19 AM

👍🏽

This revision is now accepted and ready to land.Nov 15 2022, 3:39 PM

Rebase

Harbormaster completed remote builds in B13451: Diff 18450.Nov 15 2022, 6:35 PM

Rebase

Harbormaster completed remote builds in B13468: Diff 18467.Nov 16 2022, 4:48 AM

rohan added a child revision: D5688: Allow for tooltip to be hidden when revealing spoiler while maintaining styles.Nov 18 2022, 3:06 PM

rohan removed a child revision: D5597: [IGNORE] GestureTouchableOpacity Spoiler changes.

After adding MessageContext.Provider to InnerTextMessage, there is no reason that it needs to stay in TextMessage, which renders InnerTextMessage inside of itself anyways. Let's make sure we don't have two components in the same hierarchy setting MessageContext

Rebase

Harbormaster completed remote builds in B13646: Diff 18695.Nov 21 2022, 1:01 PM

Update to use messageKey and remove extra MessageContext from
TextMessage

Harbormaster completed remote builds in B13652: Diff 18701.Nov 21 2022, 1:53 PM

rohan mentioned this in D5515: [native] Update the existing Context API for Markdown to encompass AppNavigator.Nov 21 2022, 3:48 PM

Let's abandon this diff... see my comment in D5515

This revision now requires changes to proceed.Nov 22 2022, 6:11 AM

rohan mentioned this in D5688: Allow for tooltip to be hidden when revealing spoiler while maintaining styles.Nov 22 2022, 12:54 PM

rohan removed a child revision: D5688: Allow for tooltip to be hidden when revealing spoiler while maintaining styles.

Abandoning since this can easily just be done in D5515, as pointed out by @ashoat. I'll include it there when I'm able to put out a revision to that diff

Diff 18152

View Options

native/chat/inner-text-message.react.js

#pragma once		#pragma once

#include "BlobGetClientReactor.h"		#include "BlobGetClientReactor.h"
		#include "DatabaseEntitiesTools.h"
#include "DatabaseManager.h"		#include "DatabaseManager.h"
#include "ServerWriteReactorBase.h"		#include "ServerWriteReactorBase.h"
#include "ServiceBlobClient.h"		#include "ServiceBlobClient.h"

#include "../_generated/backup.grpc.pb.h"		#include "../_generated/backup.grpc.pb.h"
#include "../_generated/backup.pb.h"		#include "../_generated/backup.pb.h"

#include <folly/MPMCQueue.h>		#include <folly/MPMCQueue.h>

#include <iostream>		#include <iostream>
#include <memory>		#include <memory>
#include <string>		#include <string>
		#include <vector>

namespace comm {		namespace comm {
namespace network {		namespace network {
namespace reactor {		namespace reactor {

class PullBackupReactor : public ServerWriteReactorBase<		class PullBackupReactor : public ServerWriteReactorBase<
backup::PullBackupRequest,		backup::PullBackupRequest,
backup::PullBackupResponse> {		backup::PullBackupResponse> {

enum class State {		enum class State {
COMPACTION = 1,		COMPACTION = 1,
LOGS = 2,		LOGS = 2,
};		};

std::shared_ptr<database::BackupItem> backupItem;		std::shared_ptr<database::BackupItem> backupItem;
std::shared_ptr<reactor::BlobGetClientReactor> getReactor;		std::shared_ptr<reactor::BlobGetClientReactor> getReactor;
std::mutex reactorStateMutex;		std::mutex reactorStateMutex;
std::shared_ptr<folly::MPMCQueue<std::string>> dataChunks;		std::shared_ptr<folly::MPMCQueue<std::string>> dataChunks;
ServiceBlobClient blobClient;		ServiceBlobClient blobClient;
State state = State::COMPACTION;		State state = State::COMPACTION;
		std::vector<std::shared_ptr<database::LogItem>> logs;
		size_t currentLogIndex = 0;
		std::shared_ptr<database::LogItem> currentLog;

void initializeGetReactor(const std::string &holder);		void initializeGetReactor(const std::string &holder);

public:		public:
PullBackupReactor(const backup::PullBackupRequest *request);		PullBackupReactor(const backup::PullBackupRequest *request);

void initialize() override;		void initialize() override;

Show All 34 Lines	void PullBackupReactor::initialize() {
this->backupItem = database::DatabaseManager::getInstance().findBackupItem(		this->backupItem = database::DatabaseManager::getInstance().findBackupItem(
this->request.userid(), this->request.backupid());		this->request.userid(), this->request.backupid());
if (this->backupItem == nullptr) {		if (this->backupItem == nullptr) {
throw std::runtime_error(		throw std::runtime_error(
"no backup found for provided parameters: user id [" +		"no backup found for provided parameters: user id [" +
this->request.userid() + "], backup id [" + this->request.backupid() +		this->request.userid() + "], backup id [" + this->request.backupid() +
"]");		"]");
}		}
// TODO get logs		this->logs = database::DatabaseManager::getInstance().findLogItemsForBackup(
		this->request.backupid());
		tomekUnsubmitted Not Done Inline Actions I remember asking about it somewhere and you've probably answered, but what is the plan for a scenario where someone appends a log after we started reading but before we finish? tomek: I remember asking about it somewhere and you've probably answered, but what is the plan for a…
		karolAuthorUnsubmitted Done Inline Actions I honestly don't remember a discussion like this. What plan could be there? We're going to read the current state of the logs. It feels a bit weird to see a scenario containing two people. Backups are individual, right? karol: I honestly don't remember a discussion like this. What plan could be there? We're going to…
		tomekUnsubmitted Not Done Inline Actions Ok, so let's be more precise. By someone, I meant two different clients, and in that case the same user could be logged in on both. There are three ways of handling this: We can raise an error - we really don't want that We can deliver a backup with a state from the begging of the operation. This is what you suggested. We can deliver a backup with a state from the end of the operation. I think that option 2 is easier, but at the same time we provide something which we know is outdated. Option 3 can be implemented e.g. by checking if there are new entries when we finished sending logs. Option 2 has a big issue when we restore the state from a backup - we might miss some updates that were made after we started the process. We can work-around it by e.g. having `currentAsOf` field and fetch new logs from the client after the backup is restored. So overall, both options are ok, but we should choose one and have it in our minds while implementing the rest of the system. @karol-bisztyga @ashoat tomek: Ok, so let's be more precise. By someone, I meant two different clients, and in that case the…
		ashoatUnsubmitted Not Done Inline Actions The diff was landed without replying to this question! ashoat: The diff was landed without replying to this question!
}		}

std::unique_ptr<grpc::Status>		std::unique_ptr<grpc::Status>
PullBackupReactor::writeResponse(backup::PullBackupResponse *response) {		PullBackupReactor::writeResponse(backup::PullBackupResponse *response) {
// we make sure that the blob client's state is flushed to the main memory		// we make sure that the blob client's state is flushed to the main memory
// as there may be multiple threads from the pool taking over here		// as there may be multiple threads from the pool taking over here
const std::lock_guard<std::mutex> lock(this->reactorStateMutex);		const std::lock_guard<std::mutex> lock(this->reactorStateMutex);
switch (this->state) {		if (this->state == State::COMPACTION) {
case State::COMPACTION: {
this->initializeGetReactor(this->backupItem->getCompactionHolder());		this->initializeGetReactor(this->backupItem->getCompactionHolder());
std::string dataChunk;		std::string dataChunk;
this->dataChunks->blockingRead(dataChunk);		this->dataChunks->blockingRead(dataChunk);
if (dataChunk.empty()) {		if (!dataChunk.empty()) {
// TODO try to immediately start writing logs instead of wasting a cycle		response->set_compactionchunk(dataChunk);
// sending nothing
this->state = State::LOGS;
return nullptr;		return nullptr;
}		}
response->set_compactionchunk(dataChunk);		if (!this->dataChunks->isEmpty()) {
		throw std::runtime_error(
		"dangling data discovered after reading compaction");
		}
		this->getReactor = nullptr;
		this->state = State::LOGS;
		}
		if (this->state == State::LOGS) {
		// TODO make sure logs are received in correct order regardless their size
		if (this->logs.empty()) {
		return std::make_unique<grpc::Status>(grpc::Status::OK);
		}
		if (this->currentLogIndex == this->logs.size()) {
		return std::make_unique<grpc::Status>(grpc::Status::OK);
		}
		if (this->currentLogIndex > this->logs.size()) {
		throw std::runtime_error("log index out of bound");
		}
		if (this->currentLog == nullptr) {
		this->currentLog = this->logs.at(this->currentLogIndex);
		if (this->currentLog->getPersistedInBlob()) {
		this->initializeGetReactor(this->currentLog->getValue());
		} else {
		response->set_logchunk(this->currentLog->getValue());
		++this->currentLogIndex;
		this->currentLog = nullptr;
return nullptr;		return nullptr;
}		}
case State::LOGS: {
throw std::runtime_error("unimplemented");
}		}
		std::string dataChunk;
		this->dataChunks->blockingRead(dataChunk);
		if (dataChunk.empty()) {
		++this->currentLogIndex;
		this->currentLog = nullptr;
		} else {
		response->set_logchunk(dataChunk);
		}
		return nullptr;
}		}
throw std::runtime_error("unhandled state");		throw std::runtime_error("unhandled state");
}		}

} // namespace reactor		} // namespace reactor
} // namespace network		} // namespace network
} // namespace comm		} // namespace comm

View Options

native/markdown/markdown-context.js

#pragma once		#pragma once

#include "BlobGetClientReactor.h"		#include "BlobGetClientReactor.h"
		#include "DatabaseEntitiesTools.h"
#include "DatabaseManager.h"		#include "DatabaseManager.h"
#include "ServerWriteReactorBase.h"		#include "ServerWriteReactorBase.h"
#include "ServiceBlobClient.h"		#include "ServiceBlobClient.h"

#include "../_generated/backup.grpc.pb.h"		#include "../_generated/backup.grpc.pb.h"
#include "../_generated/backup.pb.h"		#include "../_generated/backup.pb.h"

#include <folly/MPMCQueue.h>		#include <folly/MPMCQueue.h>

#include <iostream>		#include <iostream>
#include <memory>		#include <memory>
#include <string>		#include <string>
		#include <vector>

namespace comm {		namespace comm {
namespace network {		namespace network {
namespace reactor {		namespace reactor {

class PullBackupReactor : public ServerWriteReactorBase<		class PullBackupReactor : public ServerWriteReactorBase<
backup::PullBackupRequest,		backup::PullBackupRequest,
backup::PullBackupResponse> {		backup::PullBackupResponse> {

enum class State {		enum class State {
COMPACTION = 1,		COMPACTION = 1,
LOGS = 2,		LOGS = 2,
};		};

std::shared_ptr<database::BackupItem> backupItem;		std::shared_ptr<database::BackupItem> backupItem;
std::shared_ptr<reactor::BlobGetClientReactor> getReactor;		std::shared_ptr<reactor::BlobGetClientReactor> getReactor;
std::mutex reactorStateMutex;		std::mutex reactorStateMutex;
std::shared_ptr<folly::MPMCQueue<std::string>> dataChunks;		std::shared_ptr<folly::MPMCQueue<std::string>> dataChunks;
ServiceBlobClient blobClient;		ServiceBlobClient blobClient;
State state = State::COMPACTION;		State state = State::COMPACTION;
		std::vector<std::shared_ptr<database::LogItem>> logs;
		size_t currentLogIndex = 0;
		std::shared_ptr<database::LogItem> currentLog;

void initializeGetReactor(const std::string &holder);		void initializeGetReactor(const std::string &holder);

public:		public:
PullBackupReactor(const backup::PullBackupRequest *request);		PullBackupReactor(const backup::PullBackupRequest *request);

void initialize() override;		void initialize() override;

Show All 34 Lines	void PullBackupReactor::initialize() {
this->backupItem = database::DatabaseManager::getInstance().findBackupItem(		this->backupItem = database::DatabaseManager::getInstance().findBackupItem(
this->request.userid(), this->request.backupid());		this->request.userid(), this->request.backupid());
if (this->backupItem == nullptr) {		if (this->backupItem == nullptr) {
throw std::runtime_error(		throw std::runtime_error(
"no backup found for provided parameters: user id [" +		"no backup found for provided parameters: user id [" +
this->request.userid() + "], backup id [" + this->request.backupid() +		this->request.userid() + "], backup id [" + this->request.backupid() +
"]");		"]");
}		}
// TODO get logs		this->logs = database::DatabaseManager::getInstance().findLogItemsForBackup(
		this->request.backupid());
		tomekUnsubmitted Not Done Inline Actions I remember asking about it somewhere and you've probably answered, but what is the plan for a scenario where someone appends a log after we started reading but before we finish? tomek: I remember asking about it somewhere and you've probably answered, but what is the plan for a…
		karolAuthorUnsubmitted Done Inline Actions I honestly don't remember a discussion like this. What plan could be there? We're going to read the current state of the logs. It feels a bit weird to see a scenario containing two people. Backups are individual, right? karol: I honestly don't remember a discussion like this. What plan could be there? We're going to…
		tomekUnsubmitted Not Done Inline Actions Ok, so let's be more precise. By someone, I meant two different clients, and in that case the same user could be logged in on both. There are three ways of handling this: We can raise an error - we really don't want that We can deliver a backup with a state from the begging of the operation. This is what you suggested. We can deliver a backup with a state from the end of the operation. I think that option 2 is easier, but at the same time we provide something which we know is outdated. Option 3 can be implemented e.g. by checking if there are new entries when we finished sending logs. Option 2 has a big issue when we restore the state from a backup - we might miss some updates that were made after we started the process. We can work-around it by e.g. having `currentAsOf` field and fetch new logs from the client after the backup is restored. So overall, both options are ok, but we should choose one and have it in our minds while implementing the rest of the system. @karol-bisztyga @ashoat tomek: Ok, so let's be more precise. By someone, I meant two different clients, and in that case the…
		ashoatUnsubmitted Not Done Inline Actions The diff was landed without replying to this question! ashoat: The diff was landed without replying to this question!
}		}

std::unique_ptr<grpc::Status>		std::unique_ptr<grpc::Status>
PullBackupReactor::writeResponse(backup::PullBackupResponse *response) {		PullBackupReactor::writeResponse(backup::PullBackupResponse *response) {
// we make sure that the blob client's state is flushed to the main memory		// we make sure that the blob client's state is flushed to the main memory
// as there may be multiple threads from the pool taking over here		// as there may be multiple threads from the pool taking over here
const std::lock_guard<std::mutex> lock(this->reactorStateMutex);		const std::lock_guard<std::mutex> lock(this->reactorStateMutex);
switch (this->state) {		if (this->state == State::COMPACTION) {
case State::COMPACTION: {
this->initializeGetReactor(this->backupItem->getCompactionHolder());		this->initializeGetReactor(this->backupItem->getCompactionHolder());
std::string dataChunk;		std::string dataChunk;
this->dataChunks->blockingRead(dataChunk);		this->dataChunks->blockingRead(dataChunk);
if (dataChunk.empty()) {		if (!dataChunk.empty()) {
// TODO try to immediately start writing logs instead of wasting a cycle		response->set_compactionchunk(dataChunk);
// sending nothing
this->state = State::LOGS;
return nullptr;		return nullptr;
}		}
response->set_compactionchunk(dataChunk);		if (!this->dataChunks->isEmpty()) {
		throw std::runtime_error(
		"dangling data discovered after reading compaction");
		}
		this->getReactor = nullptr;
		this->state = State::LOGS;
		}
		if (this->state == State::LOGS) {
		// TODO make sure logs are received in correct order regardless their size
		if (this->logs.empty()) {
		return std::make_unique<grpc::Status>(grpc::Status::OK);
		}
		if (this->currentLogIndex == this->logs.size()) {
		return std::make_unique<grpc::Status>(grpc::Status::OK);
		}
		if (this->currentLogIndex > this->logs.size()) {
		throw std::runtime_error("log index out of bound");
		}
		if (this->currentLog == nullptr) {
		this->currentLog = this->logs.at(this->currentLogIndex);
		if (this->currentLog->getPersistedInBlob()) {
		this->initializeGetReactor(this->currentLog->getValue());
		} else {
		response->set_logchunk(this->currentLog->getValue());
		++this->currentLogIndex;
		this->currentLog = nullptr;
return nullptr;		return nullptr;
}		}
case State::LOGS: {
throw std::runtime_error("unimplemented");
}		}
		std::string dataChunk;
		this->dataChunks->blockingRead(dataChunk);
		if (dataChunk.empty()) {
		++this->currentLogIndex;
		this->currentLog = nullptr;
		} else {
		response->set_logchunk(dataChunk);
		}
		return nullptr;
}		}
throw std::runtime_error("unhandled state");		throw std::runtime_error("unhandled state");
}		}

} // namespace reactor		} // namespace reactor
} // namespace network		} // namespace network
} // namespace comm		} // namespace comm

View Options

native/markdown/markdown-spoiler.react.js

#pragma once		#pragma once

#include "BlobGetClientReactor.h"		#include "BlobGetClientReactor.h"
		#include "DatabaseEntitiesTools.h"
#include "DatabaseManager.h"		#include "DatabaseManager.h"
#include "ServerWriteReactorBase.h"		#include "ServerWriteReactorBase.h"
#include "ServiceBlobClient.h"		#include "ServiceBlobClient.h"

#include "../_generated/backup.grpc.pb.h"		#include "../_generated/backup.grpc.pb.h"
#include "../_generated/backup.pb.h"		#include "../_generated/backup.pb.h"

#include <folly/MPMCQueue.h>		#include <folly/MPMCQueue.h>

#include <iostream>		#include <iostream>
#include <memory>		#include <memory>
#include <string>		#include <string>
		#include <vector>

namespace comm {		namespace comm {
namespace network {		namespace network {
namespace reactor {		namespace reactor {

class PullBackupReactor : public ServerWriteReactorBase<		class PullBackupReactor : public ServerWriteReactorBase<
backup::PullBackupRequest,		backup::PullBackupRequest,
backup::PullBackupResponse> {		backup::PullBackupResponse> {

enum class State {		enum class State {
COMPACTION = 1,		COMPACTION = 1,
LOGS = 2,		LOGS = 2,
};		};

std::shared_ptr<database::BackupItem> backupItem;		std::shared_ptr<database::BackupItem> backupItem;
std::shared_ptr<reactor::BlobGetClientReactor> getReactor;		std::shared_ptr<reactor::BlobGetClientReactor> getReactor;
std::mutex reactorStateMutex;		std::mutex reactorStateMutex;
std::shared_ptr<folly::MPMCQueue<std::string>> dataChunks;		std::shared_ptr<folly::MPMCQueue<std::string>> dataChunks;
ServiceBlobClient blobClient;		ServiceBlobClient blobClient;
State state = State::COMPACTION;		State state = State::COMPACTION;
		std::vector<std::shared_ptr<database::LogItem>> logs;
		size_t currentLogIndex = 0;
		std::shared_ptr<database::LogItem> currentLog;

void initializeGetReactor(const std::string &holder);		void initializeGetReactor(const std::string &holder);

public:		public:
PullBackupReactor(const backup::PullBackupRequest *request);		PullBackupReactor(const backup::PullBackupRequest *request);

void initialize() override;		void initialize() override;

Show All 34 Lines	void PullBackupReactor::initialize() {
this->backupItem = database::DatabaseManager::getInstance().findBackupItem(		this->backupItem = database::DatabaseManager::getInstance().findBackupItem(
this->request.userid(), this->request.backupid());		this->request.userid(), this->request.backupid());
if (this->backupItem == nullptr) {		if (this->backupItem == nullptr) {
throw std::runtime_error(		throw std::runtime_error(
"no backup found for provided parameters: user id [" +		"no backup found for provided parameters: user id [" +
this->request.userid() + "], backup id [" + this->request.backupid() +		this->request.userid() + "], backup id [" + this->request.backupid() +
"]");		"]");
}		}
// TODO get logs		this->logs = database::DatabaseManager::getInstance().findLogItemsForBackup(
		this->request.backupid());
		tomekUnsubmitted Not Done Inline Actions I remember asking about it somewhere and you've probably answered, but what is the plan for a scenario where someone appends a log after we started reading but before we finish? tomek: I remember asking about it somewhere and you've probably answered, but what is the plan for a…
		karolAuthorUnsubmitted Done Inline Actions I honestly don't remember a discussion like this. What plan could be there? We're going to read the current state of the logs. It feels a bit weird to see a scenario containing two people. Backups are individual, right? karol: I honestly don't remember a discussion like this. What plan could be there? We're going to…
		tomekUnsubmitted Not Done Inline Actions Ok, so let's be more precise. By someone, I meant two different clients, and in that case the same user could be logged in on both. There are three ways of handling this: We can raise an error - we really don't want that We can deliver a backup with a state from the begging of the operation. This is what you suggested. We can deliver a backup with a state from the end of the operation. I think that option 2 is easier, but at the same time we provide something which we know is outdated. Option 3 can be implemented e.g. by checking if there are new entries when we finished sending logs. Option 2 has a big issue when we restore the state from a backup - we might miss some updates that were made after we started the process. We can work-around it by e.g. having `currentAsOf` field and fetch new logs from the client after the backup is restored. So overall, both options are ok, but we should choose one and have it in our minds while implementing the rest of the system. @karol-bisztyga @ashoat tomek: Ok, so let's be more precise. By someone, I meant two different clients, and in that case the…
		ashoatUnsubmitted Not Done Inline Actions The diff was landed without replying to this question! ashoat: The diff was landed without replying to this question!
}		}

std::unique_ptr<grpc::Status>		std::unique_ptr<grpc::Status>
PullBackupReactor::writeResponse(backup::PullBackupResponse *response) {		PullBackupReactor::writeResponse(backup::PullBackupResponse *response) {
// we make sure that the blob client's state is flushed to the main memory		// we make sure that the blob client's state is flushed to the main memory
// as there may be multiple threads from the pool taking over here		// as there may be multiple threads from the pool taking over here
const std::lock_guard<std::mutex> lock(this->reactorStateMutex);		const std::lock_guard<std::mutex> lock(this->reactorStateMutex);
switch (this->state) {		if (this->state == State::COMPACTION) {
case State::COMPACTION: {
this->initializeGetReactor(this->backupItem->getCompactionHolder());		this->initializeGetReactor(this->backupItem->getCompactionHolder());
std::string dataChunk;		std::string dataChunk;
this->dataChunks->blockingRead(dataChunk);		this->dataChunks->blockingRead(dataChunk);
if (dataChunk.empty()) {		if (!dataChunk.empty()) {
// TODO try to immediately start writing logs instead of wasting a cycle		response->set_compactionchunk(dataChunk);
// sending nothing
this->state = State::LOGS;
return nullptr;		return nullptr;
}		}
response->set_compactionchunk(dataChunk);		if (!this->dataChunks->isEmpty()) {
		throw std::runtime_error(
		"dangling data discovered after reading compaction");
		}
		this->getReactor = nullptr;
		this->state = State::LOGS;
		}
		if (this->state == State::LOGS) {
		// TODO make sure logs are received in correct order regardless their size
		if (this->logs.empty()) {
		return std::make_unique<grpc::Status>(grpc::Status::OK);
		}
		if (this->currentLogIndex == this->logs.size()) {
		return std::make_unique<grpc::Status>(grpc::Status::OK);
		}
		if (this->currentLogIndex > this->logs.size()) {
		throw std::runtime_error("log index out of bound");
		}
		if (this->currentLog == nullptr) {
		this->currentLog = this->logs.at(this->currentLogIndex);
		if (this->currentLog->getPersistedInBlob()) {
		this->initializeGetReactor(this->currentLog->getValue());
		} else {
		response->set_logchunk(this->currentLog->getValue());
		++this->currentLogIndex;
		this->currentLog = nullptr;
return nullptr;		return nullptr;
}		}
case State::LOGS: {
throw std::runtime_error("unimplemented");
}		}
		std::string dataChunk;
		this->dataChunks->blockingRead(dataChunk);
		if (dataChunk.empty()) {
		++this->currentLogIndex;
		this->currentLog = nullptr;
		} else {
		response->set_logchunk(dataChunk);
		}
		return nullptr;
}		}
throw std::runtime_error("unhandled state");		throw std::runtime_error("unhandled state");
}		}

} // namespace reactor		} // namespace reactor
} // namespace network		} // namespace network
} // namespace comm		} // namespace comm

[native] Maintain spoiler reveal state on message tap
AbandonedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 18152

native/chat/inner-text-message.react.js

native/markdown/markdown-context.js

native/markdown/markdown-spoiler.react.js

[native] Maintain spoiler reveal state on message tapAbandonedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 18152

native/chat/inner-text-message.react.js

native/markdown/markdown-context.js

native/markdown/markdown-spoiler.react.js

[native] Maintain spoiler reveal state on message tap
AbandonedPublic
Actions

Revision Contents
Changeset List