-
Notifications
You must be signed in to change notification settings - Fork 347
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
Replace jetty with ktor. #1178
Open
bgrozev
wants to merge
16
commits into
jitsi:master
Choose a base branch
from
bgrozev:ktor
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Replace jetty with ktor. #1178
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
3d3e1fa
chore: Update jicoco to 1.1-142-gfed0320.
bgrozev 575ba42
feat: Add a ktor server, move /metrics to it.
bgrozev e78927b
feat: Move /about/version to ktor.
bgrozev c0ba297
ref: Move health to ktor.
bgrozev 2931069
wip: Port ConferenceRequest to ktor.
bgrozev 945e5c2
wip: Organize ktor exceptions.
bgrozev 37e3c18
ref: Move MoveEndpoints to ktor.
bgrozev d9a3b76
ref: Move RtcStats to ktor.
bgrozev c7dc7fa
ref: Move RestConfig to the ktor package.
bgrozev a22092a
ref: Move Debug to ktor.
bgrozev 022b65c
ref: Move Pin to ktor.
bgrozev e7dabff
ref: Move stats to ktor.
bgrozev 55690ce
ref: Remove jetty, move ktor to 8888.
bgrozev c0833e7
squash: Cleanup.
bgrozev 47439e9
chore: Remove jetty dependencies.
bgrozev 030b809
squash: Fix bugs introduced while porting.
bgrozev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,25 +15,16 @@ | |
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.jitsi.jicofo.rest.move | ||
package org.jitsi.jicofo.ktor | ||
|
||
import jakarta.servlet.http.HttpServletResponse | ||
import jakarta.ws.rs.DefaultValue | ||
import jakarta.ws.rs.GET | ||
import jakarta.ws.rs.NotFoundException | ||
import jakarta.ws.rs.Path | ||
import jakarta.ws.rs.Produces | ||
import jakarta.ws.rs.QueryParam | ||
import jakarta.ws.rs.core.MediaType | ||
import jakarta.ws.rs.core.Response | ||
import org.jitsi.config.JitsiConfig | ||
import org.jitsi.jicofo.ConferenceStore | ||
import org.jitsi.jicofo.bridge.Bridge | ||
import org.jitsi.jicofo.bridge.BridgeConfig | ||
import org.jitsi.jicofo.bridge.BridgeSelector | ||
import org.jitsi.jicofo.conference.JitsiMeetConference | ||
import org.jitsi.jicofo.rest.BadRequestExceptionWithMessage | ||
import org.jitsi.metaconfig.config | ||
import org.jitsi.jicofo.ktor.exception.BadRequest | ||
import org.jitsi.jicofo.ktor.exception.MissingParameter | ||
import org.jitsi.jicofo.ktor.exception.NotFound | ||
import org.jitsi.utils.logging2.createLogger | ||
import org.jxmpp.jid.impl.JidCreate | ||
import kotlin.math.min | ||
|
@@ -44,7 +35,6 @@ import kotlin.math.roundToInt | |
* that when re-inviting the normal bridge selection logic is used again, so it's possible that the same bridge is | ||
* selected (unless it's unhealthy/draining or overloaded and there are less loaded bridges). | ||
*/ | ||
@Path("/move-endpoints") | ||
class MoveEndpoints( | ||
val conferenceStore: ConferenceStore, | ||
val bridgeSelector: BridgeSelector | ||
|
@@ -54,28 +44,19 @@ class MoveEndpoints( | |
/** | ||
* Move a specific endpoint in a specific conference. | ||
*/ | ||
@Path("move-endpoint") | ||
@GET | ||
@Produces(MediaType.APPLICATION_JSON) | ||
fun moveEndpoint( | ||
/** | ||
* Conference JID, e.g [email protected]. This is a required parameter, but without a @DefaultValue | ||
* jetty returns a 500 and prints a stack trace. | ||
*/ | ||
@QueryParam("conference") @DefaultValue("") conferenceId: String, | ||
/** | ||
* Endpoint ID, e.g. abcdefgh. This is a required parameter, but without a @DefaultValue jetty returns a 500 | ||
* and prints a stack trace. | ||
*/ | ||
@QueryParam("endpoint") @DefaultValue("") endpointId: String, | ||
/** Conference JID, e.g [email protected]. */ | ||
conferenceId: String?, | ||
/** Endpoint ID, e.g. abcdefgh. */ | ||
endpointId: String?, | ||
/** | ||
* Optional bridge JID. If specified, the endpoint will only be moved it if is indeed connected to this bridge. | ||
*/ | ||
@QueryParam("bridge") @DefaultValue("") bridgeId: String | ||
bridgeId: String? | ||
): Result { | ||
if (conferenceId.isEmpty()) throw BadRequestExceptionWithMessage("Conference ID is missing") | ||
if (endpointId.isEmpty()) throw BadRequestExceptionWithMessage("Endpoint ID is missing") | ||
val bridge = if (bridgeId.isEmpty()) null else getBridge(bridgeId) | ||
if (conferenceId.isNullOrBlank()) throw MissingParameter("conference") | ||
if (endpointId.isNullOrBlank()) throw MissingParameter("endpoint") | ||
val bridge = if (bridgeId.isNullOrBlank()) null else getBridge(bridgeId) | ||
val conference = getConference(conferenceId) | ||
|
||
logger.info("Moving conference=$conferenceId endpoint=$endpointId bridge=$bridgeId") | ||
|
@@ -98,26 +79,20 @@ class MoveEndpoints( | |
* greedily from the list until we've selected the desired count. Note that this may need to be adjusted if it leads | ||
* to thundering horde issues (though the recentlyAddedEndpointCount correction should prevent them). | ||
*/ | ||
@Path("move-endpoints") | ||
@GET | ||
@Produces(MediaType.APPLICATION_JSON) | ||
fun moveEndpoints( | ||
/** | ||
* Bridge JID, e.g. [email protected]/jvb1. This is a required parameter, but without a | ||
* @DefaultValue jetty returns a 500 and prints a stack trace. | ||
*/ | ||
@QueryParam("bridge") @DefaultValue("") bridgeId: String, | ||
/** Bridge JID, e.g. [email protected]/jvb1. */ | ||
bridgeId: String?, | ||
/** | ||
* Optional conference JID, e.g [email protected]. If specified only endpoints from this conference | ||
* will be moved. | ||
*/ | ||
@QueryParam("conference") @DefaultValue("") conferenceId: String, | ||
conferenceId: String?, | ||
/** Number of endpoints to move. */ | ||
@QueryParam("endpoints") @DefaultValue("1") numEndpoints: Int | ||
numEndpoints: Int | ||
): Result { | ||
if (bridgeId.isEmpty()) throw BadRequestExceptionWithMessage("Bridge JID is missing") | ||
if (bridgeId.isNullOrBlank()) throw MissingParameter("bridge") | ||
val bridge = getBridge(bridgeId) | ||
val conference = if (conferenceId.isEmpty()) null else getConference(conferenceId) | ||
val conference = if (conferenceId.isNullOrBlank()) null else getConference(conferenceId) | ||
val bridgeConferences = if (conference == null) { | ||
bridge.getConferences() | ||
} else { | ||
|
@@ -136,19 +111,13 @@ class MoveEndpoints( | |
* that this may need to be adjusted if it leads to thundering horde issues (though the recentlyAddedEndpointCount | ||
* correction should prevent them). | ||
*/ | ||
@Path("move-fraction") | ||
@GET | ||
@Produces(MediaType.APPLICATION_JSON) | ||
fun moveFraction( | ||
/** | ||
* Bridge JID, e.g. [email protected]/jvb1. This is a required parameter, but without a | ||
* @DefaultValue jetty returns a 500 and prints a stack trace. | ||
*/ | ||
@QueryParam("bridge") @DefaultValue("") bridgeId: String, | ||
/** The fraction of endpoints to move. Defaults to 10% */ | ||
@QueryParam("fraction") @DefaultValue("0.1") fraction: Double | ||
/** Bridge JID, e.g. [email protected]/jvb1. */ | ||
bridgeId: String?, | ||
/** The fraction of endpoints to move. */ | ||
fraction: Double | ||
): Result { | ||
if (bridgeId.isEmpty()) throw BadRequestExceptionWithMessage("Bridge JID is missing") | ||
if (bridgeId.isNullOrBlank()) throw MissingParameter("bridge") | ||
val bridge = getBridge(bridgeId) | ||
val bridgeConferences = bridge.getConferences() | ||
val totalEndpoints = bridgeConferences.sumOf { it.second } | ||
|
@@ -175,27 +144,26 @@ class MoveEndpoints( | |
val bridgeJid = try { | ||
JidCreate.from(bridge) | ||
} catch (e: Exception) { | ||
throw BadRequestExceptionWithMessage("Invalid bridge ID") | ||
throw BadRequest("Invalid bridge ID") | ||
} | ||
|
||
bridgeSelector.get(bridgeJid)?.let { return it } | ||
|
||
val bridgeFullJid = try { | ||
JidCreate.from("${BridgeConfig.config.breweryJid}/$bridge") | ||
} catch (e: Exception) { | ||
throw BadRequestExceptionWithMessage("Invalid bridge ID") | ||
throw BadRequest("Invalid bridge ID") | ||
} | ||
return bridgeSelector.get(bridgeFullJid) ?: throw NotFoundExceptionWithMessage("Bridge not found") | ||
return bridgeSelector.get(bridgeFullJid) ?: throw NotFound("Bridge not found") | ||
} | ||
|
||
private fun getConference(conferenceId: String): JitsiMeetConference { | ||
val conferenceJid = try { | ||
JidCreate.entityBareFrom(conferenceId) | ||
} catch (e: Exception) { | ||
throw BadRequestExceptionWithMessage("Invalid conference ID") | ||
throw BadRequest("Invalid conference ID") | ||
} | ||
return conferenceStore.getConference(conferenceJid) | ||
?: throw NotFoundExceptionWithMessage("Conference not found") | ||
return conferenceStore.getConference(conferenceJid) ?: throw NotFound("Conference not found") | ||
} | ||
|
||
private fun Bridge.getConferences() = conferenceStore.getAllConferences().mapNotNull { conference -> | ||
|
@@ -208,14 +176,6 @@ data class Result( | |
val conferences: Int | ||
) | ||
|
||
class MoveEndpointsConfig { | ||
companion object { | ||
val enabled: Boolean by config { | ||
"jicofo.rest.move-endpoints.enabled".from(JitsiConfig.newConfig) | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Select endpoints to move, e.g. with a map m={a: 1, b: 3, c: 3}: | ||
* select(m, 1) should return {a: 1} | ||
|
@@ -238,11 +198,4 @@ private fun <T> List<Pair<T, Int>>.select(n: Int): Map<T, Int> { | |
put(it.first, m) | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* The [NotFoundException(String message)] constructor doesn't actually include the message in the response. | ||
*/ | ||
class NotFoundExceptionWithMessage(message: String?) : NotFoundException( | ||
Response.status(HttpServletResponse.SC_NOT_FOUND, message).build() | ||
) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "move-fraction"?