There are only two hard things in Computer Science: cache invalidation and naming things.
From skimming the code:
ActivitySequencer is adding sequence numbers and checks them
As for the direct connection, TCP is used, so package loss is handled low level and retransmissions are tried.
For IBB Connections depends on the error type, if network drops, than again its TCP Handled and again retransmissions are tried.
If the Server drops single messages, than
ActivitySequencer should handle this and simply drops the user.
@Flowdalic Uff, its been a while. I think the smack stuff just got lost in the chaos. :sweat_smile: But we are still very much interested in updating smack. We have been working hard on updating our outdated libraries along with our build infrastructure and smack is one of the big ones, especially since it provides a central functionality of the plugin and is in a somewhat messy state with all of the patch stuff we added. So we really appreciate your efforts.
I had a short look at your branch but didn't really get far. (But I am also not that well-versed in the network part of Saros, so that doesn't really mean that much :wink:) As @srossbach already mentioned, formatting the code with our formatter (google java format 1.6) reduces the PR size considerably (from +3300 -4800 LOC to +1000 LOC - 2700 LOC, which is still a lot, but more manageable).
The patch seems like a WIP (judging from the compiler errors). Are you still interested in working on the migration? If so, what are your general plans? Maybe we can work out some sort of cooperation to make the migration easier. :smile: Do you have a general estimate on how much work still is necessary to complete the migration?
Extending User Permissions
My job is to come up with an extension of the User Permissions mechanism. I've seen there is already a somewhat flexible Permission implemented in the User Class:
My question: What would you like specifically to be extended? Are there use cases for currently missing functionality? The only problem i could find until now is the missing Permission for Users to upload documents into the Session. From my point of view it would be sufficient to add a specifier to the enum Permissions and an instance method like hasShareDocumentPermission()..?
Hi, @stefaus, @Drakulix and me already talked a bit about your topic (Adding a permission model that will improve the server workflow). I'll post a summary of our talk below. @Memo2109 I think you should concentrate on the first step mentioned in this summary. @srossbach what do you think?
The current server just accepts all requests of users to join the session.
Therefore it is necessary to think about a permission model.
Using different permission models for server sessions and common session (without a server) is unnecessary. Therefore, it is necessary to create a permission model that allows to implement the current user permissions (the permissions of the host user and other users).
@Memo2109 Hey Mehmet, normaly please create a (draft) pull request to get feedback for your code.
I think instead of creating a separate
UserPrivilege class, using a
EnumSet would be a simpler approach. What you have to keep in mind is also concurrency. Using a
HashMap will not work here (neither
EnumSet without additional handling) and instead of a separate method to check Privileges, this should stick to a single get method like your
About the Privileges, a User is created inside a Session, thats why handling Permissions like to
Start a Session Server here will not work.
Once saros-project/saros#752 is merged, I would like to do another round of testing for the vector time patch saros-project/saros#714. As I was not able to produce any issues in a two person setup (even with concurrent activities), a multi-user test is quite important (especially with the weird issues that occurred the last time).
@m273d15 @stefaus @srossbach What does your availability look like in next week? My initial suggestion would be Monday or Tuesday afternoon, but I don't have any set events/meetings on Monday, Tuesday, or Thursday, so I am relatively open timewise. Wednesday before 17:00 would be fine as well.
I have one question, as I make my last PR I have seen that tests startet like codacy and travis ci. My question is should I also make the codacy and travis ci tests local on my repo befor I make a PR ? If yes have you an introduction for setting up codacy and travis ci. Thank you very much.
@Memo2109 No, you don't have to add the checks to your local fork of the repository. As you already pointed out, they are always run when you create a PR onto the main repo.
I changed the description of the
Area: X issue labels to state "Issues affecting X". In my opinion, this makes the tags much more usable as they then provide a good tool for getting an overview which PRs affect which area of the code. In my opinion, the current restrictions of having them be "specific to the area" makes sense for issues but not for PRs, as they often bundle multiple commit dealing with different parts of an issue.
Anyone have any issues with this change?