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.

Sign up for GitHub

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

Add Date Time Range type #2

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add Date Time Range type #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
680ee59
Add Date Time Range type
izavits Sep 8, 2016
283beb1
Naming and error descr improvements
izavits Sep 9, 2016
344fb85
add additional fact types
izavits Sep 22, 2016
ee6efee
Move dataTypeGen from abstract-sql-compiler here
izavits Dec 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
* Add Date Time Range type

v1.2.0

* Return null for null dates rather than the unix epoch.
Expand Down
19 changes: 18 additions & 1 deletion Type.sbvr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,21 @@ Fact type: Color has Green Component
Fact type: Color has Blue Component
Necessity: Each Color has exactly one Blue Component
Fact type: Color has Alpha Component
Necessity: Each Color has exactly one Alpha Component
Necessity: Each Color has exactly one Alpha Component

Term: Start
Concept Type: Date Time
Term: End
Concept Type: Date Time
Term: Bounds
Concept Type: Short Text
Term: Date Time Range
Concept Type: Short Text
Fact type: Date Time Range has Start
Necessity: Each Date Time Range has exactly one Start
Fact Type: Date Time Range has End
Necessity: Each Date Time Range has at most one End
Fact Type: Date Time Range has Bounds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is "Bounds"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defines the lower and upper bound. Can be: [], [), (), (]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this should be in an English form, eg Date Time Range is inclusive of beginning, Date Time Range includes beginning, something like that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be an additional Fact Type? I am currently looking at the SBVR Date Time Vocabulary but I can't find anything similar.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it should be an additional fact type

Necessity: Each Date Time Range has exactly one Bounds
Fact Type: Date Time Range is inclusive or exclusive of Start
Fact Type: Date Time Range is inclusive or exclusive of End
13 changes: 13 additions & 0 deletions src/TypeUtils.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,17 @@ do ->
callback('is not a valid date: ' + value)
else
callback(null, processedValue)

dataTypeGen: (dbType, engine, dataType, necessity, index = '', defaultValue) ->
necessity = if necessity then ' NOT NULL' else ' NULL'
defaultValue = if defaultValue then " DEFAULT #{defaultValue}"
if index != ''
index = ' ' + index
if dbType?
if _.isFunction(dbType)
return dbType(necessity, index)
defaultValue ?= ''
return dbType + defaultValue + necessity + index
else
throw new Error("Unknown data type '#{dataType}' for engine: #{engine}")
}
10 changes: 10 additions & 0 deletions src/types/Boolean.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,14 @@ do ->
callback("is not a boolean: #{JSON.stringify(originalValue)} (#{typeof originalValue})")
else
callback(null, value)

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Color.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,14 @@
callback('has an unknown component: ' + component)
return
callback(null, processedValue)

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/ConceptType.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,14 @@
Real: TypeUtils.nativeFactTypeTemplates.comparison

validate: TypeUtils.validate.integer

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
66 changes: 66 additions & 0 deletions src/types/Date Time Range.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{
types:
postgres: 'TSTZRANGE'
mysql: 'VARCHAR(255)'
websql: 'VARCHAR(255)'
odata:
name: 'Self.DateTimeRange'
complexType: '''
<ComplexType Name="DateTimeRange">
<Property Name="Start" Nullable="false" Type="Edm.DateTime"/>\
<Property Name="End" Nullable="true" Type="Edm.DateTime"/>\
<Property Name="Bounds" Nullable="false" Type="Edm.String"/>\
</ComplexType>'''

nativeProperties:
has:
Start: (from) -> ['RangeStart', from]
End: (from) -> ['RangeEnd', from]
Bounds: (from) -> ['RangeBounds', from]

fetchProcessing: (data, callback) ->
if data?
[start, end] = data.slice(1, -1).split(',')
res =
Start: start
End: end.trim()
Bounds: data[0] + data[data.length - 1]
callback(null, res)
else
callback(null, data)

validate: (value, required, callback) ->
if !_.isObject(value)
callback('is not a date time range object: ' + value)
else
# Check with hasOwnProperty since null values are allowed
if value.hasOwnProperty('Start') and value.hasOwnProperty('End') and value.hasOwnProperty('Bounds')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This forces the properties into a specific case where otherwise we would allow any case (with the .toLowerCase() below). I think it does make sense to be case insensitive, so maybe after the for loop we can do an if !start? or !end? or !bounds? to check that all the properties were set

processedValue = ''
start = undefined
end = undefined
bounds = undefined
for own component, componentValue of value
switch component.toLowerCase()
when 'start'
start = componentValue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do some validation of this?

when 'end'
end = componentValue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do some validation of this?

when 'bounds'
bounds = componentValue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to validate bounds here, because we access it using index properties below but if it's actually a number for instance then that will fail (and in general only certain values make sense)

else
callback('has an unknown component: ' + component)
processedValue = bounds[0] + start + ', ' + end + bounds[1]
callback(null, processedValue)
else
callback('is missing components: ' + value)

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Date Time.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,14 @@
callback(null, data)

validate: TypeUtils.validate.date

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue != 'CURRENT_TIMESTAMP'
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Date.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,14 @@
callback(null, data)

validate: TypeUtils.validate.date

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/File.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,14 @@
callback(null, value)
else
callback("could not be converted to binary: #{typeof value}")

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/ForeignKey.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,14 @@
Real: TypeUtils.nativeFactTypeTemplates.comparison

validate: TypeUtils.validate.integer

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Hashed.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,14 @@ do ->
.asCallback(callback)

compare: _.bind(bcrypt.compareAsync, bcrypt)

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Integer.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,14 @@
Real: TypeUtils.nativeFactTypeTemplates.comparison

validate: TypeUtils.validate.integer

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Interval.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,14 @@
name: 'Edm.Int64'

validate: TypeUtils.validate.integer

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/JSON.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,14 @@
catch e
console.error(e)
callback('cannot be turned into JSON: ' + value)

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Real.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,14 @@
callback('is not a number: ' + value)
else
callback(null, processedValue)

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Serial.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,14 @@
name: 'Edm.Int64'

validate: TypeUtils.validate.integer

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Short Text.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,14 @@
name: 'Edm.String'

validate: TypeUtils.validate.text(255)

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Text.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,14 @@
Text: TypeUtils.nativeFactTypeTemplates.equality

validate: TypeUtils.validate.text()

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
if defaultValue
@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
else
defaultValue = null
dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
10 changes: 10 additions & 0 deletions src/types/Time.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,14 @@
callback(err)
return
callback(null, value.toLocaleTimeString())

dataTypeGen: (engine, dataType, necessity, index = '', defaultValue) ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see all of the default value checking in $type.dataTypeGen functions are the same, so it can be pushed directly into TypeUtils.dataTypeGen and have this bit just be dataTypeGen: TypeUtils.dataTypeGen

Copy link
Contributor Author

@izavits izavits Dec 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have a default value equal to CURRENT_TIMESTAMP (e.g. in Date Time type) which will not get caught by the validate function. So these cases need a special treatment I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if I move everything to TypeUtils.dataTypeGen I won't be able to reference lets say the per type validate from there. (I assume, by looking how the types.js is being created)

if defaultValue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would remove the option of having a default value of 0, false, '' etc, it should be if defaultValue? to check for nullish values directly (which is ok since a default value of null is equivalent to no default)

@validate defaultValue, true, (err, value) ->
if !err
defaultValue = value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works because our "async" callback is actually synchronous, if you try to do a default value in the Hashed datatype then it will break because that is actually truly async (and so this will happen after we've already returned the datatype, making the default value be the unprocessed one). Therefore we need to turn this into an async function that accepts a callback - if you're feeling up for it it would also be great to turn it into a dual interface api that supports both promises and callbacks by doing something like

fn = (..., callback) ->
	PromiseStuff...
	.asCallback(callback)

with http://bluebirdjs.com/docs/api/ascallback.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly should be turned into an async function? The validate function you mean?

else
defaultValue = null
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the defaultValue is invalid we should actually throw an error rather than just silently ignoring it - if we've tried to use a default value it's probably required for things to work properly.

dbType = @types?[engine]
TypeUtils.dataTypeGen dbType, engine, dataType, necessity, index, defaultValue
}
22 changes: 22 additions & 0 deletions test/Date Time Range.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
helpers = require './helpers'

helpers.describe 'Date Time Range', (test) ->
start = new Date()
end = 'null'
bounds = '[)'
describe 'fetchProcessing', ->
test.fetch(bounds[0] + start + ', ' + end + bounds[1], {
Start: start.toString()
End: end
Bounds: bounds
})

describe 'validate', ->
start = new Date()
end = null
bounds = '[)'
test.validate({
Start: start
End: end
Bounds: bounds
}, true, bounds[0] + start + ', ' + end + bounds[1])