Added Cloudwatch Event

[fabianfett]

No description provided.

[tomerd]

is this a fixed list that then corresponds to the Detail type? maybe there is a way to express this in a more type safe way?

[fabianfett]

Okay, found the list of all detail options that are available, right now:

https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/EventTypes.html#schedule_event_type

Quite a list, I'd say. So I guess the most correct way would be to implement an

enum Payload {
  case .scheduled
  case .codepipeline(CodePipelineStateChange)
  case .ec2stateChange(EC2StateChange)
  
}

detailType and detail could be completely removed thanks to the enum, different Cloudwatch events could be handled by one lambda. 🎉

But this would mean that we need to support as many detail types as possible. Extending a defined enum with more cases is sadly not possible. That's why everything we don't define upfront, can not be used by the user. But I don't know if we want to support such a massive list of potential subtypes just from the beginning (keeping in mind semver).

Neither Go, nor Java implements the complete show.

Sadly Codable doesn't support some kind of AnyCodable as Go does. That's why I think the current solutions is the best we can do so far, because it's by far the easiest to extend for custom use cases?

wdyt?

[tomerd]

interesting dilemma, agreed we need something that can scale or iow we implement the top ~5 common ones and the users can extend to more exotic ones. here is one way:

public enum Cloudwatch {
    /// CloudWatch.Event is the outer structure of an event sent via CloudWatch Events.
    ///
    /// **NOTE**: For examples of events that come via CloudWatch Events, see https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/EventTypes.html
    public struct Event: Decodable {
        public let id: String
        public let source: String
        public let accountId: String
        public let time: Date
        public let region: AWSRegion
        public let resources: [String]

        public let payload: Payload

        enum CodingKeys: String, CodingKey {
            case id
            case detailType = "detail-type"
            case source
            case accountId = "account"
            case time
            case region
            case resources
            case detail
        }
        
        public init(from decoder: Decoder) throws {
            let container = try decoder.container(keyedBy: CodingKeys.self)
            
            self.id = try container.decode(String.self, forKey: .id)
            self.source = try container.decode(String.self, forKey: .source)
            self.accountId = try container.decode(String.self, forKey: .accountId)
            let time = try container.decode(ISO8601Coding.self, forKey: .time)
            self.time = time.wrappedValue
            self.region = try container.decode(AWSRegion.self, forKey: .region)
            self.resources = try container.decode([String].self, forKey: .resources)
            
            let detailType = try container.decode(String.self, forKey: .detailType)
            let detailsDecoder = try container.superDecoder(forKey: .detail)
            let detailsFactory: (Decoder) throws -> Decodable
            switch detailType {
            case Payload.type.scheduled.rawValue:
                detailsFactory = Empty.init
            case Payload.type.codepipeline.rawValue:
                detailsFactory = CodePipelineStateChange.init
            case Payload.type.ec2stateChange.rawValue:
                detailsFactory = EC2StateChange.init
            default:
                guard let factory = Payload.registry[detailType] else {
                    throw UnknownPayload()
                }
                detailsFactory = factory
            }
            self.payload = Payload(label: detailType, body: try detailsFactory(detailsDecoder))
        }
    }

    public struct Payload {
        enum type: String {
            case scheduled = "Scheduled Event"
            case codepipeline
            case ec2stateChange
        }
        
        public let label: String
        public let body: Decodable
        
        public init(label: String, body: Decodable) {
            self.label = label
            self.body = body
        }
        
        // FIXME: make thread safe
        static var registry = [String: (Decoder) throws -> Decodable]()
        public static func register<T: Decodable>(label: String, type: T.Type) {
            registry[label] = type.init
        }
    }
   
    public struct Empty: Decodable {}
    
    public struct CodePipelineStateChange: Decodable {
        let foo: String
    }
    
    public struct EC2StateChange: Decodable {
        let bar: String
    }
    
    struct UnknownPayload: Error {}
}

[tomerd]

this looks a bit funny since its got no actual members, maybe a generic empty one? or have details be optional? or some other better idea?

[tomerd]

do we need a unit test for this?

[fabianfett]

Those are included in the test-events. So I don't think so? If you want me to I'm happy to provide extra tests though.

[tomerd]

would nice to have a small unit test for each PW

[fabianfett]

tests added.

[tomerd]

so I spoke with the XCTest team and they told me that XCTest has improved the way they handle errors so that throwing tests are no longer considered "harmful". their current recommendation is to make the test throwable and drop the XCTAssertNoThrow and do not use try/catch(XCTFail)

[tomerd]

oh hmmm, they only fixed in on Darwin, looking to get the fix ported to Linux too

[fabianfett]

@tomerd What do you think about going with an enum here for the Details?

[tomerd]

works for me

[tomerd]

btw if we are worried about breaking compatibility we could use a struct with static vars/func (eg: https://github.com/swift-server/async-http-client/blob/master/Sources/AsyncHTTPClient/HTTPClient.swift#L723) so we can add more "cases" without major version bump

[fabianfett]

@tomerd do we need to be thread safe here? In normal usage this should only be used during cold start. I think a comment should be sufficient. wdyt?

[tomerd]

I think the question is if we can trust the users to not do crazy things, life taught me to not trust :D but you may have a different experience. if we decided to guard, a read/write lock could be the right thing since its going to be vastly read

[tomerd]

copy/pasta?

[fabianfett]

If we use a generic approach here, we don't need the factory register stuff further down. Let's use the generic type as the factory! We need to decide which detail payload we get at compile time anyway. But what we are currently going for additionally is checking whether detailType matches the Detail.Type.

But for this I would do the following:

protocol CloudwatchDetail: Decodable {
  static var name: String
}

public enum Cloudwatch {
     /// CloudWatch.Event is the outer structure of an event sent via CloudWatch Events.
     ///
     /// **NOTE**: For examples of events that come via CloudWatch Events, see
     /// https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/EventTypes.html
     /// https://docs.aws.amazon.com/eventbridge/latest/userguide/event-types.html
     public struct Event<Detail: CloudwatchDetail>: Decodable {

     }
}

[fabianfett]

@tomerd

okay, we are back to the generic approach, which I think is for the best 😉. I like it. I went further and used the generic approach to remove the registry. I figured it doesn’t make much sense to have a runtime registry if we have a compile time registry anyway (thanks to the generic type). Further this way we don't need synchronize the registry, which saves us from linking Dispatch or NIOConcurrencyHelpers here! 🥳

I created a new protocol:

public protocol CloudwatchDetail: Decodable {
   static var name: String { get }
}

I use the new protocol to constrain the generic type

public struct Event<Detail: CloudwatchDetail>: Decodable {}

I check within the init(from decoder: ) if the types align

public init(from decoder: Decoder) throws {
  let container = try decoder.container(keyedBy: CodingKeys.self)
  self.id = try container.decode(String.self, forKey: .id)
  self.source = try container.decode(String.self, forKey: .source)
  self.accountId = try container.decode(String.self, forKey: .accountId)
  self.time = (try container.decode(ISO8601Coding.self, forKey: .time)).wrappedValue
  self.region = try container.decode(AWSRegion.self, forKey: .region)
  self.resources = try container.decode([String].self, forKey: .resources)
  
  let detailType = try container.decode(String.self, forKey: .detailType)
  guard detailType == Detail.name else { // <--- type check
      throw PayloadTypeMismatch(name: detailType, type: Detail.self)
  }
  self.detail = try container.decode(Detail.self, forKey: .detail)
}

I commited the changes. Revert them if you don’t like them, but I think this makes the solution easier to use, as well as to extend (implement protocol). I especially like the typealiases, which hide the generics.

[tomerd]

remove now?

[tomerd]

make this case insensitive?

[tomerd]

I like this allot. two tiny comments and I think this is ready to go