Feature/query operators #45

loudmouth · 2017-03-15T16:11:20Z

No description provided.

coveralls · 2017-03-20T18:35:24Z

Coverage decreased (-6.2%) to 77.531% when pulling e958a61 on feature/query-operators into be98aa7 on develop.

Remove unnecessary generic protocol abstraction

while preserving the benefits of generics

Make a protocol called ClientFieldst that users will conform to help them map fields. map identifier like, entry.sys.id Add ObjectMapper and replace Deodable, Wrap Link's in an Enum to make them stateful

Debug Link resolution for one-to-one links Debug Link resoulution for one-to-many links

Debug SyncSpace, API Error parsing, content type decoding & Include resolution Use ImmutableMappable Reimplement Decoding tests and rename then ObjectMappingTests Make query syntax read better Update ContentModel protocol to be easier to make use of

Separate code paths for query'ing entries and assets Don't restrict ContentModel to class types

coveralls · 2017-03-21T17:17:12Z

Coverage decreased (-3.4%) to 80.243% when pulling 051775a on feature/query-operators into be98aa7 on develop.

coveralls · 2017-03-22T10:33:35Z

Coverage decreased (-1.2%) to 82.478% when pulling e464379 on feature/query-operators into be98aa7 on develop.

mariobodemann

Small improvements I'd like to suggest ...

mariobodemann · 2017-03-22T14:10:56Z

Sources/Decoding.swift

-private func parseLocalizedFields(_ json: Any) throws -> (String, [String:[String:Any]]) {
-    let fields = try json => "fields" as! [String: Any]
-    let locale: String? = try? json => "sys" => "locale"
+//internal extension String {


Can we either uncomment or remove?

mariobodemann · 2017-03-22T14:46:10Z

Sources/Link.swift

+
+    public let id: String
+
+    public let linkType: String // "Entry" or "Asset" (-> Easier to resolve with this information.


being picky ) is missing at the end of the comment.

I'll just rm this comment. It was just for me ;-)

mariobodemann · 2017-03-22T14:53:40Z

Sources/Query.swift

+        }
+    }
+
+//    internal static func validatecombinationsRecursively(operations: QueryOperation...) throws {


To comment or to delete, that is here the question ... ;)

mariobodemann · 2017-03-22T15:13:43Z

Sources/Query.swift

+extension String {
+
+    func isValidSelection() -> Bool {
+        if characters.split(separator: ".").count > 2 {


fields..foo will still pass... What about fields..foo".characters.split(separator:".", maxSplits:34, omittingEmptySubsequences:false)

mariobodemann · 2017-03-22T15:20:02Z

Sources/Sys.swift

+        createdAt       = try? map.value("createdAt")
+        updatedAt       = try? map.value("updatedAt")
+    }
+}


Do you want to map space and revision too?

mariobodemann · 2017-03-22T15:24:01Z

Tests/EntryTests.swift

+                    case .asset(let image):
+                        expect(url(image).absoluteString).to(equal("https://images.contentful.com/cfexampleapi/4gp6taAwW4CmSgumq2ekUm/9da0cd1936871b8d72343e895a00d611/Nyan_cat_250px_frame.png"))
+                    default:
+                        fail("Should not have a link of the wrong type her")


mariobodemann · 2017-03-22T15:27:01Z

Tests/LinkTests.swift

+        let entry = Entry(localizedField: ["":["":""]])
+        let link = Link.entry(entry)
+        let newLink = Link.link(from: link)
+        expect(newLink).toNot(beNil())


Can this test be more explicit? I.e. what about changing it from .toNot(beNil()) to testing if the newLink sys.id equals the linked entry id, or so?

ooops, I thought I had deleted this file. Turns out i only removed it from the xcode project index. It's not a necessary test.

mariobodemann · 2017-03-22T15:29:00Z

Tests/QueryTests.swift

+
+
+// PERSISTENCE
+//final class Cat: NSManagedObject, ContentModel {


Are those tests to be used once persistence is updated, or can we uncomment them, make them useful?

oops. i forgot to address those. I'll remove them

mariobodemann · 2017-03-22T15:31:58Z

Tests/QueryTests.swift

+
+            switch result {
+            case .success(let dogs):
+                let doge = dogs.first!


Is it wise to depend on the undefined ordering here?

i'm not sure...any better ideas?

Yes: Also request an ordering by any field ... ;)

mariobodemann · 2017-03-22T15:43:24Z

Tests/QueryTests.swift

+        } catch let error as QueryError {
+            expect(error.message).toNot(beNil())
+        } catch _ {
+            fail("Should throw a QueryError")


do you need to catch the exception here, or can you let it bubble through to the test?

I'm catching here to make sure that the error thrown is the right type of Error. So yes, I do indeed need to catch it now.

mariobodemann

🙆‍♂️

coveralls · 2017-03-23T12:51:31Z

Coverage decreased (-1.2%) to 82.497% when pulling d0614cc on feature/query-operators into be98aa7 on develop.

loudmouth force-pushed the feature/query-operators branch 2 times, most recently from ba44680 to e958a61 Compare March 20, 2017 18:02

loudmouth assigned mariobodemann Mar 21, 2017

JP Wright added 15 commits March 21, 2017 17:41

Rename ClientConfiguration file

a28fe11

Add default arg to initialSync method

9f1806b

Add a protocol mechanism to directly decode to the desired ContentModel

7ffe159

Resolve links during generic selection

ff8d098

Add tests for dates

e4f5f85

Migrate FieldType to Swift 3 enum conventions

f737f73

Create QueryOperations and new interface for Query construction

f56f977

Remove unnecessary generic protocol abstraction

Enable using same query class for Assets & Images

fcd4cab

while preserving the benefits of generics

Enable compound queries

63fb0c8

Add test for searching on reference fields

ccd30fa

Experiment with nested structures in Entry, Asset, that mirror the API

aa5f11d

Make a protocol called ClientFieldst that users will conform to help them map fields. map identifier like, entry.sys.id Add ObjectMapper and replace Deodable, Wrap Link's in an Enum to make them stateful

Fix errors and debug tests:

ac4152d

Debug Link resolution for one-to-one links Debug Link resoulution for one-to-many links

Add contentTypeId to Sys

8c5df9d

Add depth param for link resolution

1fcd4da

Debug SyncSpace, API Error parsing, content type decoding & Include resolution Use ImmutableMappable Reimplement Decoding tests and rename then ObjectMappingTests Make query syntax read better Update ContentModel protocol to be easier to make use of

Make misc query parameters variables

051775a

Separate code paths for query'ing entries and assets Don't restrict ContentModel to class types

loudmouth force-pushed the feature/query-operators branch from ee87db7 to 051775a Compare March 21, 2017 16:43

Improve test coverage

e464379

mariobodemann reviewed Mar 22, 2017

View reviewed changes

Make changes based on Mario's feedback

d0614cc

loudmouth force-pushed the feature/query-operators branch from d6c073f to d0614cc Compare March 23, 2017 12:34

mariobodemann approved these changes Mar 23, 2017

View reviewed changes

loudmouth merged commit 03913ca into develop Mar 23, 2017

loudmouth deleted the feature/query-operators branch March 23, 2017 12:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/query operators #45

Feature/query operators #45

loudmouth commented Mar 15, 2017

coveralls commented Mar 20, 2017

coveralls commented Mar 21, 2017

coveralls commented Mar 22, 2017

mariobodemann left a comment

mariobodemann Mar 22, 2017

mariobodemann Mar 22, 2017

loudmouth Mar 22, 2017

mariobodemann Mar 22, 2017

mariobodemann Mar 22, 2017

mariobodemann Mar 22, 2017

mariobodemann Mar 22, 2017

mariobodemann Mar 22, 2017

loudmouth Mar 23, 2017

mariobodemann Mar 22, 2017

loudmouth Mar 23, 2017

mariobodemann Mar 22, 2017

loudmouth Mar 23, 2017

mariobodemann Mar 23, 2017

mariobodemann Mar 22, 2017

loudmouth Mar 23, 2017

mariobodemann left a comment

coveralls commented Mar 23, 2017


		public let id: String

		public let linkType: String // "Entry" or "Asset" (-> Easier to resolve with this information.



		// PERSISTENCE
		//final class Cat: NSManagedObject, ContentModel {

Feature/query operators #45

Feature/query operators #45

Conversation

loudmouth commented Mar 15, 2017

coveralls commented Mar 20, 2017

coveralls commented Mar 21, 2017

coveralls commented Mar 22, 2017

mariobodemann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariobodemann left a comment

Choose a reason for hiding this comment

coveralls commented Mar 23, 2017