search

ProvideQ / toolbox-server

Resources for the server that hosts the toolbox (Backend)
https://provideq.kit.edu
MIT License
1 stars 1 forks source link

Refactor controllers to avoid boilerplate code #39

Closed schweikart closed 11 months ago

schweikart commented 1 year ago

This pull request replaces our controllers with a generic router implementation that discovers and routes new meta solvers (problem types) automatically.

schweikart commented 1 year ago

Feature Model tests are currently failing because the feature model anomaly solver currently does not use the same route structure as the other solvers. We can solve this problem by separating all feature model anomalies into different problem types (implemented in #40 ) or by combining the feature model anomaly routes into a single route and adding an AnomalyType parameter to FeatureModelAnomalyRequest. We can discuss this at our meeting tomorrow

Elscrux commented 1 year ago

Cross Origin is still a problem here, and we'll have to sort that out for the new webflux framework.

schweikart commented 11 months ago

Looks good so far! The functional approach really shines here. I think we could move the documentation part to its own method in each class, similarly to the handle method. That would separate this part a bit, reduce nesting and the method name would also help to document what this is for (I first didn't really get what this is about until you told me).

That seems like a good idea! Can you address this in your PR for API documentation?

Elscrux commented 11 months ago

Cross Origin is still a problem here, and we'll have to sort that out for the new webflux framework.

Tested again with the latest versions of the web (merged PR) and toolbox (anomaly refactor), this should have been fixed. Edit: Oh I see you enabled CORS again.

Elscrux commented 11 months ago

Looks good so far! The functional approach really shines here. I think we could move the documentation part to its own method in each class, similarly to the handle method. That would separate this part a bit, reduce nesting and the method name would also help to document what this is for (I first didn't really get what this is about until you told me).

That seems like a good idea! Can you address this in your PR for API documentation?

Yeah I can do that as part of the API documentation.

schweikart commented 11 months ago

This all looks fine to me now, but I'll wait for your SolutionHandle removal (if you want to do that, and do it in this PR)

I'll do that in another PR but please merge #40 before merging this one :)