-
Notifications
You must be signed in to change notification settings - Fork 58
Implemented python properties in base.py #41
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
Changes from all commits
cc02f33
ba79763
2536b14
9107da6
0d2b50f
cc258df
ee12a18
eba7e68
6fd0e93
b4fc958
8db3123
9bf2317
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,61 +35,141 @@ | |
# TODO(slinkydeveloper) is this really needed? | ||
class EventGetterSetter(object): | ||
|
||
# ce-specversion | ||
def CloudEventVersion(self) -> str: | ||
raise Exception("not implemented") | ||
|
||
# CloudEvent attribute getters | ||
def EventType(self) -> str: | ||
raise Exception("not implemented") | ||
@property | ||
def specversion(self): | ||
return self.CloudEventVersion() | ||
|
||
def Source(self) -> str: | ||
def SetCloudEventVersion(self, specversion: str) -> object: | ||
raise Exception("not implemented") | ||
|
||
def EventID(self) -> str: | ||
raise Exception("not implemented") | ||
@specversion.setter | ||
def specversion(self, value: str): | ||
self.SetCloudEventVersion(value) | ||
Comment on lines
+42
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe having 3 methods like this is too much, don't we just need 2, getter and setter/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason why there's 4 methods (property/foo.setter and getter/setter) is the original developers made getters/setters. To make sure I didn't break any existing code, I had to make sure the old getters/setters still work which is why I didn't remove them. The benefit of the properties is now you can do something such as event.specversion which is more pythonic instead of event.CloudEventVersion() |
||
|
||
def EventTime(self) -> str: | ||
# ce-type | ||
def EventType(self) -> str: | ||
raise Exception("not implemented") | ||
|
||
def SchemaURL(self) -> str: | ||
raise Exception("not implemented") | ||
@property | ||
def type(self): | ||
return self.EventType() | ||
|
||
def Data(self) -> object: | ||
def SetEventType(self, eventType: str) -> object: | ||
raise Exception("not implemented") | ||
|
||
def Extensions(self) -> dict: | ||
raise Exception("not implemented") | ||
@type.setter | ||
def type(self, value: str): | ||
self.SetEventType(value) | ||
|
||
def ContentType(self) -> str: | ||
# ce-source | ||
def Source(self) -> str: | ||
raise Exception("not implemented") | ||
|
||
# CloudEvent attribute constructors | ||
# Each setter return an instance of its class | ||
# in order to build a pipeline of setter | ||
def SetEventType(self, eventType: str) -> object: | ||
raise Exception("not implemented") | ||
@property | ||
def source(self): | ||
return self.Source() | ||
|
||
def SetSource(self, source: str) -> object: | ||
raise Exception("not implemented") | ||
|
||
@source.setter | ||
def source(self, value: str): | ||
self.SetSource(value) | ||
|
||
# ce-id | ||
def EventID(self) -> str: | ||
raise Exception("not implemented") | ||
|
||
@property | ||
def id(self): | ||
return self.EventID() | ||
|
||
def SetEventID(self, eventID: str) -> object: | ||
raise Exception("not implemented") | ||
Comment on lines
91
to
92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't rename the original setters/getters because that would break users code whom are using these functions |
||
|
||
@id.setter | ||
def id(self, value: str): | ||
self.SetEventID(value) | ||
|
||
# ce-time | ||
def EventTime(self) -> str: | ||
raise Exception("not implemented") | ||
|
||
@property | ||
def time(self): | ||
return self.EventTime() | ||
|
||
def SetEventTime(self, eventTime: str) -> object: | ||
raise Exception("not implemented") | ||
Comment on lines
106
to
107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need these methods? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately yes. Removing them would break existing user code |
||
|
||
@time.setter | ||
def time(self, value: str): | ||
self.SetEventTime(value) | ||
|
||
# ce-schema | ||
def SchemaURL(self) -> str: | ||
raise Exception("not implemented") | ||
|
||
@property | ||
def schema(self) -> str: | ||
return self.SchemaURL() | ||
|
||
def SetSchemaURL(self, schemaURL: str) -> object: | ||
raise Exception("not implemented") | ||
|
||
@schema.setter | ||
def schema(self, value: str): | ||
self.SetSchemaURL(value) | ||
|
||
# data | ||
def Data(self) -> object: | ||
raise Exception("not implemented") | ||
|
||
@property | ||
def data(self) -> object: | ||
return self.Data() | ||
|
||
def SetData(self, data: object) -> object: | ||
raise Exception("not implemented") | ||
|
||
@data.setter | ||
def data(self, value: object): | ||
self.SetData(value) | ||
|
||
# ce-extensions | ||
def Extensions(self) -> dict: | ||
raise Exception("not implemented") | ||
|
||
@property | ||
def extensions(self) -> dict: | ||
return self.Extensions() | ||
|
||
def SetExtensions(self, extensions: dict) -> object: | ||
raise Exception("not implemented") | ||
|
||
@extensions.setter | ||
def extensions(self, value: dict): | ||
self.SetExtensions(value) | ||
|
||
# Content-Type | ||
def ContentType(self) -> str: | ||
raise Exception("not implemented") | ||
|
||
@property | ||
def content_type(self) -> str: | ||
return self.ContentType() | ||
|
||
def SetContentType(self, contentType: str) -> object: | ||
raise Exception("not implemented") | ||
|
||
@content_type.setter | ||
def content_type(self, value: str): | ||
self.SetContentType(value) | ||
|
||
|
||
class BaseEvent(EventGetterSetter): | ||
def Properties(self, with_nullable=False) -> dict: | ||
|
@@ -120,7 +200,6 @@ def Set(self, key: str, value: object): | |
attr.set(value) | ||
setattr(self, formatted_key, attr) | ||
return | ||
|
||
exts = self.Extensions() | ||
exts.update({key: value}) | ||
self.Set("extensions", exts) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
# All Rights Reserved. | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); you may | ||
# not use this file except in compliance with the License. You may obtain | ||
# a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
# License for the specific language governing permissions and limitations | ||
# under the License. | ||
|
||
import io | ||
import json | ||
import copy | ||
import pytest | ||
|
||
from uuid import uuid4 | ||
|
||
from cloudevents.sdk import converters | ||
from cloudevents.sdk import marshaller | ||
|
||
from cloudevents.sdk.converters import structured | ||
from cloudevents.sdk.event import v03, v1 | ||
|
||
|
||
from cloudevents.tests import data | ||
|
||
|
||
@pytest.mark.parametrize("event_class", [ v03.Event, v1.Event]) | ||
def test_general_binary_properties(event_class): | ||
m = marshaller.NewDefaultHTTPMarshaller() | ||
event = m.FromRequest( | ||
event_class(), | ||
{"Content-Type": "application/cloudevents+json"}, | ||
io.StringIO(json.dumps(data.json_ce[event_class])), | ||
lambda x: x.read(), | ||
) | ||
|
||
new_headers, _ = m.ToRequest(event, converters.TypeBinary, lambda x: x) | ||
assert new_headers is not None | ||
assert "ce-specversion" in new_headers | ||
|
||
# Test properties | ||
assert event is not None | ||
assert event.type == data.ce_type | ||
assert event.id == data.ce_id | ||
assert event.content_type == data.contentType | ||
assert event.source == data.source | ||
|
||
# Test setters | ||
new_type = str(uuid4()) | ||
new_id = str(uuid4()) | ||
new_content_type = str(uuid4()) | ||
new_source = str(uuid4()) | ||
|
||
event.extensions = {'test': str(uuid4)} | ||
event.type = new_type | ||
event.id = new_id | ||
event.content_type = new_content_type | ||
event.source = new_source | ||
|
||
assert event is not None | ||
assert (event.type == new_type) and (event.type == event.EventType()) | ||
assert (event.id == new_id) and (event.id == event.EventID()) | ||
assert (event.content_type == new_content_type) and (event.content_type == event.ContentType()) | ||
assert (event.source == new_source) and (event.source == event.Source()) | ||
assert event.extensions['test'] == event.Extensions()['test'] | ||
assert (event.specversion == event.CloudEventVersion()) | ||
|
||
|
||
@pytest.mark.parametrize("event_class", [v03.Event, v1.Event]) | ||
def test_general_structured_properties(event_class): | ||
copy_of_ce = copy.deepcopy(data.json_ce[event_class]) | ||
m = marshaller.NewDefaultHTTPMarshaller() | ||
http_headers = {"content-type": "application/cloudevents+json"} | ||
event = m.FromRequest( | ||
event_class(), http_headers, io.StringIO(json.dumps(data.json_ce[event_class])), lambda x: x.read() | ||
) | ||
# Test python properties | ||
assert event is not None | ||
assert event.type == data.ce_type | ||
assert event.id == data.ce_id | ||
assert event.content_type == data.contentType | ||
assert event.source == data.source | ||
|
||
new_headers, _ = m.ToRequest(event, converters.TypeStructured, lambda x: x) | ||
for key in new_headers: | ||
if key == "content-type": | ||
assert new_headers[key] == http_headers[key] | ||
continue | ||
assert key in copy_of_ce | ||
|
||
# Test setters | ||
new_type = str(uuid4()) | ||
new_id = str(uuid4()) | ||
new_content_type = str(uuid4()) | ||
new_source = str(uuid4()) | ||
|
||
event.extensions = {'test': str(uuid4)} | ||
event.type = new_type | ||
event.id = new_id | ||
event.content_type = new_content_type | ||
event.source = new_source | ||
|
||
assert event is not None | ||
assert (event.type == new_type) and (event.type == event.EventType()) | ||
assert (event.id == new_id) and (event.id == event.EventID()) | ||
assert (event.content_type == new_content_type) and (event.content_type == event.ContentType()) | ||
assert (event.source == new_source) and (event.source == event.Source()) | ||
assert event.extensions['test'] == event.Extensions()['test'] | ||
assert (event.specversion == event.CloudEventVersion()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be called
CloudEventSpecversion
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish, but unfortunately users are already using CloudEventVersion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make breaking changes for major versions of this package?