Skip to content
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

Rewrite #36

Merged
merged 41 commits into from Jan 12, 2020
Merged

Rewrite #36

merged 41 commits into from Jan 12, 2020

Conversation

Blacksmoke16
Copy link
Member

@Blacksmoke16 Blacksmoke16 commented Dec 15, 2019

CLI

  • Removed this
    • Will be reimplemented in another shard in a future PR

Config

  • Common configuration logic will now reside in https://github.com/athena-framework/config
    • Defines the base config type
      • Components that require configuration should reopen the type to add to it
        • Athena::Routing::Config and Athena::Routing::Config::Cors is an example of this
      • A module is provided to make this a bit easier
        • Includes the required modules and defines the required methods
    • Defines the ConfigurationResolver service
      • Allows a component's configuration to be resolved from within a service
        • Aids testing
        • Abstracts where/how the configuration comes from/is obtained
      • Configuration types that should be resolvable should reopen the type and add a #resolve(_type) method typed to that object

DI

Routing

  • Replaced HTTP::Handler architecture with an event dispatcher observer/mediator pattern.
  • Switch controllers back to classes again
  • Strip some of the complexity from the routing code
    • Switches the goals of Athena to start off simple as a JSON REST API framework, adding in features as needed versus to handle everything at once.
    • Removes some compile time errors that aren't worth the complexity
      • Mainly just if a route param was defined but is missing from the action arguments
    • Removes the built in param converters
  • Query parameters are now annotation based versus being included in the route definition
    • Keeps support for regex constraints
  • Change how the raw POST body is accessed
    • No longer need an action argument called body
    • Now just type an action argument as HTTP::Request
  • Provide access to the raw Response object
    • Just type an action argument as HTTP::Server::Response
  • Remove direct dependencies on CrSerializer and Crylog
    • They'll be optional dependencies utilizing optional DI services
  • Add a kemal/sinatra style DSL for defining routes
    • Should make defining routes a bit more user friendly
  • Move the routing code out from the routing/ directory
    • Renames the main file from routing.cr to athena.cr
  • Define a HEAD route for each GET route
  • Param converters are now based on a module
    • #convert is not passed the HTTP::Request object in order to be more flexible

TODO

  • Reimplement the CORS functionality
    • Possibly as a future PR since it'll require the configuration stuff
  • Finish writing specs
  • Update docs
    • Update deployment script to build docs for all required athena components

Goals include:
* Using a listener based approach
* Reduce complexity
* Better user/develop experience
Support no argument actions
Support passing request to actions
Dispatch Response, ActionArguments, and Terminate events
Remove some now unneeded code
Some minor polish here and there
Add some more DI specs
Cleanup some services
Remove some invalid requirements
Better param type conversion error handling
Support converters on query params
Only return 204 no content if the response is nil and the return type is Nil
Allows for using DI as well as generics
Use E2E tests to avoid the compiler errors with unit testing
Minor fixes to argumentResolver
Add specs
Return 204 is action's return type is nil regardless of the response value
spec/routing/compiler_spec.cr Outdated Show resolved Hide resolved
spec/routing/compiler_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
spec/routing/listeners/cors_listener_spec.cr Outdated Show resolved Hide resolved
Default macro DSL to String
Add specs for macro DSL
Switch to GH actions
Remove need for "routing/"
Rename base file routing => athena
Fix some typos
Use a more common domain name
README.md Outdated Show resolved Hide resolved
Replace ART::Types.convert_type with a class method defined directly on the type
Remove ActionArguments event
Remove ability to get/set a specific argument
Make Controller a generic arg
Some reorganization of param converters/controller code
Part 1 of documentation pass
src/argument_resolver.cr Outdated Show resolved Hide resolved
src/controller.cr Outdated Show resolved Hide resolved
src/controller.cr Outdated Show resolved Hide resolved
Use EventListenerInterface
More documentation
Fix some typos
@Blacksmoke16 Blacksmoke16 marked this pull request as ready for review January 12, 2020 02:01
@Blacksmoke16 Blacksmoke16 merged commit 8eaf7fc into master Jan 12, 2020
@Blacksmoke16 Blacksmoke16 deleted the rewrite branch January 12, 2020 03:24
src/annotations.cr Show resolved Hide resolved
src/controller.cr Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants