perwendel / spark

A simple expressive web framework for java. Spark has a kotlin DSL https://github.com/perwendel/spark-kotlin
Apache License 2.0
9.63k stars 1.56k forks source link

Add methods for removing routes at runtime #1189

Closed dustinkredmond closed 3 years ago

dustinkredmond commented 3 years ago

Currently spark doesn't provide for a way to remove routes after they have already been registered. Several users on both StackOverflow and other forums asked how to access this functionality. My patch exposes two static methods in spark.Spark, one for removing a route based on the URL path, and another for removing a route based on the URL path and HttpMethod.

Removing routes at runtime could be useful if the developer wants to, for example, change the logic of a route at runtime, then redeploy. I personally use a forked and patched version of Spark with the ability to remove routes in maintaining a web-based API management system that lets users define logic through a web interface.

perwendel commented 3 years ago

Will review this soon! Thanks for contributing

dustinkredmond commented 3 years ago

@perwendel Good day, have you had time to review? I feel as though this commit would really improve Spark, especially for folks trying to use it in a more dynamic sort of way.

perwendel commented 3 years ago

@dustinkredmond Hey, I have had a quick look and the functionality is simple and something I think would improve Spark. I just want to think about the naming of the methods to get the best possible notation and match with other Spark functions.

perwendel commented 3 years ago

@dustinkredmond I thought about it quickly now. Perhaps unmap() would be an alternative to removeRoute()?

dustinkredmond commented 3 years ago

@perwendel I like unmap() as well. That should be pretty straightforward and I doubt many people will be left guessing as to what the methods do. Unless you have any further ideas, I'll go ahead and put in a commit with the changes.

dustinkredmond commented 3 years ago

@perwendel I added a commit to change the method names to unmap(). I also updated some of the javadoc to better describe the behaviour. Please let me know if there's anything else that you would like.

perwendel commented 3 years ago

@dustinkredmond Merged. I added a test as well. Thanks for contributing!!!