Impressions from the improvements to Telegram Android
At this stage, expanded functionality for hiding the actions of blocked users has been implemented. The general idea is to hide the user as much as possible from any possible manifestations and activities of users blocked by him – ideally, to emulate their absence in nature. In the next stages I plan to do something with stories.
Now Kilogram (that’s what I called my modification of the official Telegram client) can: for users on the blocked list –
“Nickname is typing…” in the top of the window is not highlighted
their messages are not shown
their reactions to messages are not shown
Unread mentions icons do not appear
Unread reaction icons do not appear
Messages are not shown in search results (global and local in chat)
You can switch to Kilogram mode and back to the original Telegram mode through the main menu.
To achieve this result, it was necessary to change the Telegram code, which means the main task as a doctor is not to do harm :).
To do this, you have to immerse yourself and understand the structure and details of the project, trying to recreate in your head the author’s intention – those agreements and conventions, taking into account which the project was implemented, but which in this case are not explicitly stated anywhere – neither in the comments (of which there are almost none), nor in the technical documentation (which simply does not exist). Therefore, the only source of information is the source code itself, of which there turned out to be a lot.
There were classes with 30-40 thousand lines, so even Github refuses to show the contents of the code file due to that he is too big. And if methods with tens of thousands of lines can be collapsed/expanded using plus signs through code editors, then with branches of conditional statements thousands of lines long it was already more difficult, because It’s not very convenient to constantly scroll up and down in search of differences, keep numerous contexts in your head and count indents, because inside the outer branches of the conditions there are nested inner ones, also hundreds and thousands of lines long. Moreover, in many cases, the code of both branches of conditions contains quite extensive (up to 80-100%) common parts:
if (currentType >= 0) {
cacheByChatsController.setKeepMedia(currentType, keepMedia);
if (callback != null) {
callback.onKeepMediaChange(currentType, keepMedia);
}
} else {
if (callback != null) {
callback.onKeepMediaChange(currentType, keepMedia);
}
}
if (topicId == 0) {
state = getMessagesStorage().getDatabase().executeFast("REPLACE INTO reaction_mentions VALUES(?, ?, ?)");
state.requery();
state.bindInteger(1, messageId);
state.bindInteger(2, hasUnreadReaction ? 1 : 0);
state.bindLong(3, dialogId);
state.step();
state.dispose();
} else {
state = getMessagesStorage().getDatabase().executeFast("REPLACE INTO reaction_mentions_topics VALUES(?, ?, ?, ?)");
state.requery();
state.bindInteger(1, messageId);
state.bindInteger(2, hasUnreadReaction ? 1 : 0);
state.bindLong(3, dialogId);
state.bindLong(4, topicId);
state.step();
state.dispose();
} if (progressToFullView != 1f) {
childImage.draw(canvas);
fullImage.setImageCoords(childImage.getImageX(), childImage.getImageY(), childImage.getImageWidth(), childImage.getImageHeight());
fullImage.draw(canvas);
} else {
fullImage.setImageCoords(childImage.getImageX(), childImage.getImageY(), childImage.getImageWidth(), childImage.getImageHeight());
fullImage.draw(canvas);
} if (animationInProgress) {
float startY = viewPadding + (animateFromPosition * (1f - animationProgress) + animateToPosition * animationProgress) * lineH - startOffset;
rectF.set(0, startY + linePadding, getMeasuredWidth(), startY + lineH - linePadding);
canvas.drawRoundRect(rectF, r, r, selectedPaint);
} else {
float startY = viewPadding + selectedPosition * lineH - startOffset;
rectF.set(0, startY + linePadding, getMeasuredWidth(), startY + lineH - linePadding);
canvas.drawRoundRect(rectF, r, r, selectedPaint);
}Sometimes you can even see the following options:
if (i == 0) {
particlePaints[i].setStrokeWidth(AndroidUtilities.dp(1.4f));
particlePaints[i].setStyle(Paint.Style.STROKE);
particlePaints[i].setStrokeCap(Paint.Cap.ROUND);
} else {
particlePaints[i].setStrokeWidth(AndroidUtilities.dp(1.2f));
particlePaints[i].setStyle(Paint.Style.STROKE);
particlePaints[i].setStrokeCap(Paint.Cap.ROUND);
}Up to complete (!) duplication of code in both branches of the condition:
if (lastOffset == Integer.MAX_VALUE) {
layoutManager.scrollToPositionWithOffset(0, 0);
} else {
layoutManager.scrollToPositionWithOffset(0, 0);
} if (drawInBackground) {
placeholderMatrix[index].postTranslate(-offset + totalTranslation - x, 0);
} else {
placeholderMatrix[index].postTranslate(-offset + totalTranslation - x, 0);
} if (service.isScreencast()) {
bottomButton.setType(VoIpSwitchLayout.Type.VIDEO, false, fast);
} else {
bottomButton.setType(VoIpSwitchLayout.Type.VIDEO, false, fast);
}Or similar designs:
if (x > inset && x < width && y > inset && y < height) {
return 0;
}
return 0;The above is only a small part of the examples, and for the sake of brevity and clarity, small fragments are presented, and in the code base such duplications occur hundreds and thousands of lines long. In general, looking at the project, one gets the impression that the developers adhered to the idea “copy-paste is our everything!” Or maybe they get paid per line of code they write. But it is normal for the same blocks of code to appear in one class file 5 or more times, sometimes formally different, but actually representing the same code. Compare for example:
if (d.last_message_date == 0) {
ArrayList<MessageObject> arrayList = new_dialogMessage.get(d.id);
if (arrayList != null) {
int maxDate = Integer.MIN_VALUE;
for (int i = 0; i < arrayList.size(); ++i) {
MessageObject msg = arrayList.get(i);
if (msg != null && msg.messageOwner != null && maxDate < msg.messageOwner.date) {
maxDate = msg.messageOwner.date;
}
}
if (maxDate > Integer.MIN_VALUE) {
d.last_message_date = maxDate;
}
}
} if (d.last_message_date == 0) {
ArrayList<MessageObject> messages = new_dialogMessage.get(d.id);
if (messages != null) {
int maxDate = Integer.MIN_VALUE;
for (int i = 0; i < messages.size(); ++i) {
MessageObject msg = messages.get(i);
if (msg != null && msg.messageOwner != null && msg.messageOwner.date > maxDate) {
maxDate = msg.messageOwner.date;
}
}
if (maxDate > Integer.MIN_VALUE) {
d.last_message_date = maxDate;
}
}
}And this code appears 5 times in the class file.
But enough about the copy-paste. In general, the Android Studio code analyzer highlights a lot of similar areas and even gives advice on how to change them, but my goal was not to tidy up the source code, but only to make my own workable changes. Although sometimes it was necessary to refactor heavily – for example, the last example of code that was encountered 5 times had to be modified, increasing the volume of each section by two times, and I reluctantly moved it into a separate private method.
Some funny things:
public boolean loadingBlockedPeers = false;
public LongSparseIntArray blockePeers = new LongSparseIntArray();And of course, blockePeers are used in all classes. It is clear that this is a typo and can be easily corrected. I even tried to fix it, but then I thought – if after the correction I search for the blockedPeers project, then I will come across that Boolean variable above, and much more. And so, with the typo, we have a unique name for the class field, which will not be mixed with anything else when searching 🙂
However, there were not only stylistic errors that were easily detected by the code analyzer, but such, for example:
if (!missingData && !updates.entities.isEmpty()) {
for (int a = 0; a < updates.entities.size(); a++) {
TLRPC.MessageEntity entity = updates.entities.get(a);
if (entity instanceof TLRPC.TL_messageEntityMentionName) {
long uid = ((TLRPC.TL_messageEntityMentionName) entity).user_id;
TLRPC.User entityUser = getUser(uid);
if (entityUser == null || entityUser.min) {
entityUser = getMessagesStorage().getUserSync(uid);
if (entityUser != null && entityUser.min) {
entityUser = null;
}
if (entityUser == null) {
missingData = true;
break;
}
putUser(user, true);
}
}
}
}In the code above, we go to great lengths to get the entityUser local variable to have a non-empty value. This local variable is defined inside the conditional statement block and is used exclusively in the small section of code quoted. But having received it, we ignore it and call putUser on the user variable, which is defined above in the code in the external context, and has its own call to putUser. I strongly suspect that it should have been putUser(entityUser, true).
Another example:
if (topicId != 0) {
cursor = getMessagesStorage().getDatabase().queryFinalized(String.format(Locale.US, "SELECT message_id, state FROM reaction_mentions WHERE message_id IN (%s) AND dialog_id = %d", stringBuilder, dialogId));
} else {
cursor = getMessagesStorage().getDatabase().queryFinalized(String.format(Locale.US, "SELECT message_id, state FROM reaction_mentions_topics WHERE message_id IN (%s) AND dialog_id = %d AND topic_id = %d", stringBuilder, dialogId, topicId));
}Moreover, higher in the same file, a similar construction is called as it should. There is an error due to copy-paste. Although here you can dig even deeper. Until some time, Telegram did not have forums with topics, and with their appearance, client versions had to be improved. And instead of adding the topic_id DEFAULT 0 column to all the necessary tables in the local database, all the tables were simply duplicated. As a result, there are messages, and there are messages in topics, there are reactions-mentions, and there is the same thing, but only for topics, etc. Perhaps the developers had their own arguments in favor of such a decision. But this led to the need for checks and duplication at all points of use. As a consequence, there is a lot of code with a condition – if there is a topic identifier, then we use one table, if not, then another. Well, copy-paste errors, of course.
There were also less obvious errors, noticed only during tests of the modified code. Here's a short example:
button.setUsers(users);
if (users != null && !users.isEmpty()) {
button.count = 0;
button.counterDrawable.setCount(0, false);
}This code is executed in the context of the class responsible for rendering reactions to the message. I needed to make changes so that when the filter is activated, reactions from blocked users would not appear on the message. Telegram uses the following logic: if the total number of reactions is less than 4, then it shows them not by number, but by a list of avatars of the users who posted these reactions. And the code above just checks that if the user array is not empty, then erase the number of reactions, showing only user avatars. But it does not check that we have filled in exactly as many users as there were reactions. As a result, if a user with his avatar is not loaded into the local cache, or the server, over time, decides to clear expired reactions from the recent recent block, then we can show fewer avatars on the screen than there were actual reactions. In simple words – if there are 3 reactions on a message, then we can see only 2 or 1 avatar. This can be easily fixed – just replace the !users.isEmpty() check with users.size() == button.count.
The project uses lists everywhere – the chats themselves, chat messages, forum topics, search results, etc. All lists are implemented through the RecyclerView component, which is logical. Working with this component involves creating an adapter that constructs a View based on model data, and to update the View, its model is changed and the adapter is notified about a change in one list item, range, etc. The component itself will do the rest – redraw the View, reuse ViewHolders for new elements, etc. However, the model layer of most project lists is very non-trivial and confusing, so even the developers themselves write a special procedure for updating visible elements for each list. Which iterates through all visible child elements of the parent View and causes the adapter to notify about their changes:
private void updateVisibleRows(int mask) {
if (listView == null) {
return;
}
int count = listView.getChildCount();
for (int a = 0; a < count; a++) {
View child = listView.getChildAt(a);
if (child instanceof ManageChatUserCell) {
((ManageChatUserCell) child).update(mask);
}
}
}When I tried to use this approach to hide/show messages from blocked users in chat (while enabling/disabling Kilogram mode), some messages did not change their display. The fact is that the RecyclerView component prepares several additional Views above and below the visible list area – for smooth display when scrolling. But these elements are invisible and not included in the list of children of listView.getChildAt(a), so they do not update their view when the above method is called. And when scrolling, they appear in the scope in a non-updated form. I needed a reliable solution for the functionality I was implementing, so I chose a different path – when binding a ViewHolder, I place it in a set (adapter class field), when recycling it, I remove it from this set, and as a result, I can get all associated ViewHolders at any time. s as elements of this set. A similar method for solving the described problem is outlined in this post on Stack Overflow
We also had to modify the RecyclerView.LayoutManager code. Here is the code for one of its methods (before modifications):
// Returns the first child that is visible in the provided index range, i.e. either partially or
// fully visible depending on the arguments provided. Completely invisible children are not
// acceptable by this method, but could be returned
// using #findOnePartiallyOrCompletelyInvisibleChild
View findOneVisibleChild(int fromIndex, int toIndex, boolean completelyVisible,
boolean acceptPartiallyVisible) {
ensureLayoutState();
@ViewBoundsCheck.ViewBounds int preferredBoundsFlag = 0;
@ViewBoundsCheck.ViewBounds int acceptableBoundsFlag = 0;
if (completelyVisible) {
preferredBoundsFlag = (ViewBoundsCheck.FLAG_CVS_GT_PVS | ViewBoundsCheck.FLAG_CVS_EQ_PVS
| ViewBoundsCheck.FLAG_CVE_LT_PVE | ViewBoundsCheck.FLAG_CVE_EQ_PVE);
} else {
preferredBoundsFlag = (ViewBoundsCheck.FLAG_CVS_LT_PVE
| ViewBoundsCheck.FLAG_CVE_GT_PVS);
}
if (acceptPartiallyVisible) {
acceptableBoundsFlag = (ViewBoundsCheck.FLAG_CVS_LT_PVE
| ViewBoundsCheck.FLAG_CVE_GT_PVS);
}
return (mOrientation == HORIZONTAL) ? mHorizontalBoundCheck
.findOneViewWithinBoundFlags(fromIndex, toIndex, preferredBoundsFlag,
acceptableBoundsFlag) : mVerticalBoundCheck
.findOneViewWithinBoundFlags(fromIndex, toIndex, preferredBoundsFlag,
acceptableBoundsFlag);
}
The essence of the method is explained in the comments: it returns the first fully or partially (depending on the arguments passed) visible element of the list. However, zero-height elements, according to the logic of this method, are fully visible, but not partially visible. This led to funny artifacts: when scrolling to the end of the list, the PageDown button did not disappear, a request was not sent to the server to mark the chat as fully read, etc. But the Telegram developers have nothing to do with this – they do not use zero-height elements, and everything works fine for them.
This is not the entire list of errors or suboptimalities I discovered in the Telegram code. But at the same time, the project works, apparently passes all the tests at the release stage, and the users are happy – and this is the main thing. We end up judging software products based on how they look and work, and often don't look at their source code.
Try Kilogram for yourself and see the source code you can follow this link.
I am also ready to discuss suggestions and wishes for improving the official Telegram Android client to meet custom requirements.
