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

3.0 RC 1: String parameter type does not work #36

Closed
4np opened this issue Mar 1, 2018 · 3 comments
Closed

3.0 RC 1: String parameter type does not work #36

4np opened this issue Mar 1, 2018 · 3 comments
Assignees
Labels
bug Something isn't working
Projects
Milestone

Comments

@4np
Copy link

4np commented Mar 1, 2018

Using a String.parameter does not work.

router.get("cores", String.parameter, "solar", use: solarController.getSolarForCoreByDeviceID)

This seems to be related to this guard statement (also see this discussion with @0xTim):

guard current.slug == [UInt8](P.uniqueSlug.utf8) else { ... }

Steps to reproduce

Create a route that takes a string parameter:

router.get("cores", String.parameter, "solar", use: solarController.getSolarForCoreByDeviceID)

Calling the endpoint (e.g. {{ url }}/cores/somestring/solar) will lead to the following error:

[ ERROR ] RoutingError.invalidParameterType: Invalid parameter type. Expected String got [99, 111, 114, 101] (ParameterContainer.swift:69)

Setting a breakpoint on the parameter line (let deviceID = try req.parameter(String.self)) in the controller's implementation shows that this guard statement fails.

Dumping what slug and uniqueSlug are:

        print("----")
        dump("slug: \(String(bytes: current.slug, encoding: .utf8))")
        dump("unique slug: \(P.uniqueSlug.utf8)")

results in the following output when requesting {{ url }}/cores/somestring/solar:

----
- "slug: Optional(\"core\")"
- "unique slug: string"
[ ERROR ] RoutingError.invalidParameterType: Invalid parameter type. Expected String got [99, 111, 114, 101] (ParameterContainer.swift:73)

Controller:

import Foundation
import Vapor
import Fluent

final class SolarController {
    func getSolarForCoreByDeviceID(_ req: Request) throws -> Future<SolarInfo> {
//        let deviceID = try req.parameters.next(String.self)
        let deviceID = try req.parameter(String.self)
        print("device id: \(deviceID)")
        
        return Core.query(on: req).filter(\Core.deviceID == deviceID).first().flatMap(to: SolarInfo.self) { (core) -> Future<SolarInfo> in
            guard let core = core else {
                throw Abort(.notFound, reason: "No such core")
            }
            
            let solarInfo = SolarInfo(sunrise: core.sunrise, sunset: core.sunset)
            return Future<SolarInfo>(solarInfo)
        }
    }
}

struct SolarInfo: Content {
    var sunrise: Date
    var sunriseTime: String
    var sunset: Date
    var sunsetTime: String
    
    init(sunrise: Date, sunset: Date) {
        self.sunrise = sunrise
        self.sunset = sunset
        
        let timeFormatter = DateFormatter()
        timeFormatter.locale = Locale(identifier: "nl-NL")
        timeFormatter.dateStyle = .none
        timeFormatter.timeStyle = .short
        
        self.sunriseTime = timeFormatter.string(from: sunrise)
        self.sunsetTime = timeFormatter.string(from: sunset)
    }
}

Expected behavior

I expect a String.parameter to not fail when called with a string :)

Actual behavior

The request fails with:

Oops: Invalid parameter type. Expected String got [99, 111, 114, 101]

Environment

  • Vapor Framework version: 3.0 RC
  • Vapor Toolbox version: n/a
  • OS version: High Sierra
@0xTim 0xTim added the bug Something isn't working label Mar 1, 2018
@4np
Copy link
Author

4np commented Mar 1, 2018

I think this is caused by the following:

    // Core
    let coreController = CoreController()
    router.get("cores", Core.parameter, use: coreController.getByID)

    // Solar
    let solarController = SolarController()
    router.get("cores", String.parameter, "solar", use: solarController.getSolarForCoreByDeviceID)

The first route uses a Core.parameter whereas the seconds route uses a String.parameter. I would expect the Routing logic to match the requests against all of the configured routes, not the first one it encounters?

I ran into this because I wanted to move from using the Core ID to a property on the Core model (the deviceID) because the integrating party is only aware of coreIDs and I would like to refrain from customizing web hooks on a per-core basis.

@0xTim
Copy link
Member

0xTim commented Mar 1, 2018

It's a bug, I've tried it with a really simple route as well

Joannis added a commit that referenced this issue Mar 2, 2018
@tanner0101 tanner0101 added this to the 3.0.0-rc.2 milestone Mar 12, 2018
@tanner0101 tanner0101 self-assigned this Mar 12, 2018
@4np 4np changed the title 3.0 RC: String parameter type does not work 3.0 RC 1: String parameter type does not work Mar 22, 2018
@4np
Copy link
Author

4np commented Mar 22, 2018

It looks like this works in RC 2, however it looks like a new bug #39 was introduced.

@0xTim 0xTim closed this as completed Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Vapor 3
  
Awaiting triage
Development

No branches or pull requests

3 participants