Refactor: Move DataStore Model schema to Internal directory #563

lawmicha · 2020-06-18T19:12:30Z

Issue #, if available:

Description of changes:

Moved the Model related classes over to Model/Internal.
Moved Model Schema related classes to Model/Internl/Schema
Since AnyModel is only used internally in DataStore only. Moved this over to AWSPluginsCore

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

palpatim · 2020-06-18T19:23:54Z

Amplify/Categories/DataStore/Model/Internal/Embedded.swift

@@ -9,6 +9,7 @@ import Foundation

 // MARK: - Embeddable

+/// Note that although this is public, it is intended for internal use and not consumed directly by host applications.


Let's have something a little more noticeable and dire:

/// <rest of comment> /// - Warning: Although this has `public` access, it is intended for internal use and should not be used directly /// by host applications. The behavior of this may change without warning.

thanks, taking this comment and applying it below the existing comment for classes. removing from extensions

Amplify/Categories/DataStore/Model/Internal/Schema/Model+Schema.swift

Amplify/Categories/DataStore/Model/Internal/Model+Subscript.swift

Amplify/Categories/DataStore/Model/Internal/Model+ModelName.swift

palpatim · 2020-06-18T19:28:21Z

Amplify/Categories/DataStore/Model/Internal/Model+Enum.swift

@@ -7,6 +7,7 @@

 import Foundation

+/// Note that although this is public, it is intended for internal use and not consumed directly by host applications.
 /// Protocol that represents a `Codable` Enum that can be persisted and easily
 /// integrate with remote APIs since it must have a raw `String` value.
 ///


below: change "simple enums (i.e. the ones that don't have arguments)" to "enums without associated values"

Amplify/Categories/DataStore/Model/Internal/Model+Codable.swift

Amplify/Categories/DataStore/Model/Internal/ModelRegistry+Syncable.swift

Amplify/Categories/DataStore/Model/Internal/Schema/ModelSchema+Attributes.swift

palpatim · 2020-06-18T19:37:51Z

AmplifyPlugins/Core/AWSPluginsCore/Model/AnyModel/Model+AnyModel.swift

@@ -5,6 +5,9 @@
 // SPDX-License-Identifier: Apache-2.0
 //

+import Amplify
+
+/// Note that although this is public, it is intended for internal use and not consumed directly by host applications.


This needs to be on the public symbol, not the extension wrapper

i think this can be removed, since this was moved to AWSPluginsCore

palpatim · 2020-06-18T19:39:27Z

AmplifyPlugins/DataStore/AWSDataStoreCategoryPlugin/Storage/SQLite/Statement+Model.swift

@@ -8,6 +8,7 @@
 import Amplify
 import Foundation
 import SQLite
+import AWSPluginsCore

 typealias ModelValues = [String: Any?]



Below: a public func (from(dictionary:)) on a non-public type doesn't make sense. Remove public from the func

palpatim · 2020-06-18T22:10:11Z

Amplify/Categories/DataStore/Model/Internal/Schema/ModelValueConverter.swift

@@ -7,7 +7,10 @@

 import Foundation

+/// Note that although this is public, it is intended for internal use and not consumed directly by host applications.


woops, removed, checked if there was another instance left

palpatim

As discussed, I don't thiknk we can make Model internal

lawmicha · 2020-06-22T23:16:43Z

while a codegenerated model looks like this in jazzy docs, none of the fields marked as internal. which is expected

Clicking through to the Model docs:

The highlighted ones here are marked as internal
Some reasoning behind this:

Made Model not internal
id is used by the developer in many methods like query(byId) and is part of the concrete Model class
I can see modelName may be used in developer code somewhere, potentially used when filtering on mutationEvents from DataStore publisher
i think everthing else should be marked as internal

palpatim

As discussed, let's try this in a sample app to ensure that moving files & types between modules don't result in any code changes required for a customer project. Assuming everything is transparent, ship it.

lawmicha · 2020-06-25T15:51:09Z

As discussed, let's try this in a sample app to ensure that moving files & types between modules don't result in any code changes required for a customer project. Assuming everything is transparent, ship it.

looks like things check out in the sample app. Following these instructions from the discord message helped set up the local podspec repo. Using ./CircleciScripts/setup_private_specs.sh 1.0.1 1.0.1-unreleased since this local branch is on 1.0.1, will replace the local podspec's with 1.0.1-unreleased for the cocoapods repo.

In sample app's Podfile, i explicitly referenced pod 'Amplify', '1.0.1-unreleased' rather than ~> since it was resolving to 1.0.2 (I also have 1.0.2 in my private local repo)

lawmicha marked this pull request as ready for review June 18, 2020 19:13

lawmicha requested a review from palpatim June 18, 2020 19:13

lawmicha self-assigned this Jun 18, 2020

lawmicha requested a review from drochetti June 18, 2020 19:17

palpatim reviewed Jun 18, 2020

View reviewed changes

lawmicha force-pushed the lawmicha/internal branch from 4ac2795 to 3223fed Compare June 18, 2020 21:10

lawmicha changed the base branch from lawmicha/customtype to master June 18, 2020 21:11

lawmicha requested a review from palpatim June 18, 2020 21:15

palpatim reviewed Jun 18, 2020

View reviewed changes

lawmicha added 6 commits June 18, 2020 15:53

Move Model schema related to Internal directory structure

dc28f27

Move AnyModel to AWSPluginsCore

7d5bbe5

PR comment and move AnyModel unit tests

9bb405d

comment update

db2fc61

adding warning comment to all public symbols in Internal directory

9030cc1

remove old comment

9258d11

lawmicha force-pushed the lawmicha/internal branch from ec71667 to 9258d11 Compare June 18, 2020 22:56

lawmicha mentioned this pull request Jun 18, 2020

Update CHANGELOG.md for 1.0.2 #553

Merged

palpatim reviewed Jun 18, 2020

View reviewed changes

move Model and ModelName out of Internal

d576f80

lawmicha requested a review from palpatim June 22, 2020 23:23

palpatim changed the base branch from master to main June 24, 2020 14:55

palpatim approved these changes Jun 25, 2020

View reviewed changes

lawmicha merged commit 468e885 into main Jun 25, 2020

lawmicha deleted the lawmicha/internal branch June 25, 2020 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: Move DataStore Model schema to Internal directory #563

Refactor: Move DataStore Model schema to Internal directory #563

lawmicha commented Jun 18, 2020

palpatim Jun 18, 2020

lawmicha Jun 18, 2020

palpatim Jun 18, 2020

palpatim Jun 18, 2020

lawmicha Jun 18, 2020

palpatim Jun 18, 2020

lawmicha Jun 18, 2020

palpatim Jun 18, 2020

lawmicha Jun 18, 2020

palpatim left a comment

lawmicha commented Jun 22, 2020 •

edited

Loading

palpatim left a comment

lawmicha commented Jun 25, 2020

		@@ -9,6 +9,7 @@ import Foundation

		// MARK: - Embeddable

		/// Note that although this is public, it is intended for internal use and not consumed directly by host applications.

		@@ -7,7 +7,10 @@

		import Foundation

		/// Note that although this is public, it is intended for internal use and not consumed directly by host applications.

Refactor: Move DataStore Model schema to Internal directory #563

Refactor: Move DataStore Model schema to Internal directory #563

Conversation

lawmicha commented Jun 18, 2020

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

palpatim left a comment

Choose a reason for hiding this comment

lawmicha commented Jun 22, 2020 • edited Loading

palpatim left a comment

Choose a reason for hiding this comment

lawmicha commented Jun 25, 2020

lawmicha commented Jun 22, 2020 •

edited

Loading