Skip to content

Commit

Permalink
Merge branch 'attribute-value-timestamp' into develop
Browse files Browse the repository at this point in the history
  • Loading branch information
JeremyCaney committed Feb 21, 2020
2 parents c7c9d33 + a21d308 commit a83b844
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 37 deletions.
8 changes: 5 additions & 3 deletions OnTopic.Data.Sql.Database/Stored Procedures/GetTopics.sql
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,18 @@ ORDER BY SortOrder
--------------------------------------------------------------------------------------------------------------------------------
SELECT Attributes.TopicID,
Attributes.AttributeKey,
Attributes.AttributeValue
Attributes.AttributeValue,
Attributes.Version
FROM AttributeIndex Attributes
JOIN #Topics AS Storage
ON Storage.TopicID = Attributes.TopicID

--------------------------------------------------------------------------------------------------------------------------------
-- SELECT AttributeXml
-- SELECT EXTENDED ATTRIBUTES
--------------------------------------------------------------------------------------------------------------------------------
SELECT Attributes.TopicID,
Attributes.AttributesXml
Attributes.AttributesXml,
Attributes.Version
FROM ExtendedAttributeIndex AS Attributes
JOIN #Topics AS Storage
ON Storage.TopicID = Attributes.TopicID
Expand Down
4 changes: 3 additions & 1 deletion OnTopic.Data.Sql.Database/Views/AttributeIndex.sql
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ WITH Attributes AS (
SELECT TopicID,
AttributeKey,
AttributeValue,
Version,
RowNumber = ROW_NUMBER() OVER (
PARTITION BY TopicID,
AttributeKey
Expand All @@ -27,6 +28,7 @@ WITH Attributes AS (
)
SELECT Attributes.TopicID,
Attributes.AttributeKey,
Attributes.AttributeValue
Attributes.AttributeValue,
Attributes.Version
FROM Attributes
WHERE RowNumber = 1
4 changes: 3 additions & 1 deletion OnTopic.Data.Sql.Database/Views/ExtendedAttributeIndex.sql
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ AS
WITH TopicExtendedAttributes AS (
SELECT TopicID,
AttributesXml,
Version,
RowNumber = ROW_NUMBER() OVER (
PARTITION BY TopicID
ORDER BY Version DESC
)
FROM [dbo].[ExtendedAttributes]
)
SELECT TopicID,
AttributesXml
AttributesXml,
Version
FROM TopicExtendedAttributes
WHERE RowNumber = 1
18 changes: 16 additions & 2 deletions OnTopic.Data.Sql/SqlTopicRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ private static void SetIndexedAttributes(SqlDataReader reader, Dictionary<int, T
var id = Int32.Parse(reader["TopicID"]?.ToString(), CultureInfo.InvariantCulture);
var name = reader["AttributeKey"]?.ToString();
var value = reader["AttributeValue"]?.ToString();
var version = DateTime.Now;

//Check field count to avoid breaking changes with the 4.0.0 release, which didn't include a "Version" column
//### TODO JJC20200221: This condition can be removed and accepted as a breaking change in v5.0.
if (reader.FieldCount > 3) {
version = DateTime.Parse(reader["Version"]?.ToString(), CultureInfo.InvariantCulture);
}

/*------------------------------------------------------------------------------------------------------------------------
| Validate conditions
Expand All @@ -141,7 +148,7 @@ private static void SetIndexedAttributes(SqlDataReader reader, Dictionary<int, T
/*------------------------------------------------------------------------------------------------------------------------
| Set attribute value
\-----------------------------------------------------------------------------------------------------------------------*/
current.Attributes.SetValue(name, value, false);
current.Attributes.SetValue(name, value, false, version);

}

Expand Down Expand Up @@ -169,6 +176,13 @@ private static void SetExtendedAttributes(SqlDataReader reader, Dictionary<int,
| Identify attributes
\-----------------------------------------------------------------------------------------------------------------------*/
var id = Int32.Parse(reader["TopicID"]?.ToString(), CultureInfo.InvariantCulture);
var version = DateTime.Now;

//Check field count to avoid breaking changes with the 4.0.0 release, which didn't include a "Version" column
//### TODO JJC20200221: This condition can be removed and accepted as a breaking change in v5.0.
if (reader.FieldCount > 2) {
version = DateTime.Parse(reader["Version"]?.ToString(), CultureInfo.InvariantCulture);
}

/*------------------------------------------------------------------------------------------------------------------------
| Validate conditions
Expand Down Expand Up @@ -214,7 +228,7 @@ private static void SetExtendedAttributes(SqlDataReader reader, Dictionary<int,
| Set attribute value
\---------------------------------------------------------------------------------------------------------------------*/
if (String.IsNullOrEmpty(value)) continue;
current.Attributes.SetValue(name, value, false);
current.Attributes.SetValue(name, value, false, version);

} while (xmlReader.Name == "attribute");

Expand Down
18 changes: 9 additions & 9 deletions OnTopic/Attributes/AttributeSetterAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ namespace OnTopic.Attributes {
\---------------------------------------------------------------------------------------------------------------------------*/
/// <summary>
/// Flags that a property should be used when setting an attribute via
/// <see cref="AttributeValueCollection.SetValue(String, String, Boolean?)"/>.
/// <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, DateTime?)"/>.
/// </summary>
/// <remarks>
/// <para>
/// When a call is made to <see cref="AttributeValueCollection.SetValue(String, String, Boolean?)"/>, the code will check
/// to see if a property with the same name as the attribute key exists, and whether that property is decorated with the
/// <see cref="AttributeSetterAttribute"/> (i.e., <code>[AttributeSetter]</code>). If it is, then the update will be
/// routed through that property. This ensures that business logic is enforced by local properties, instead of allowing
/// When a call is made to <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, DateTime?)"/>, the code
/// will check to see if a property with the same name as the attribute key exists, and whether that property is decorated
/// with the <see cref="AttributeSetterAttribute"/> (i.e., <code>[AttributeSetter]</code>). If it is, then the update will
/// be routed through that property. This ensures that business logic is enforced by local properties, instead of allowing
/// business logic to be potentially bypassed by writing directly to the <see cref="Topic.Attributes"/> collection.
/// </para>
/// <para>
Expand All @@ -33,10 +33,10 @@ namespace OnTopic.Attributes {
/// </para>
/// <para>
/// To ensure this logic, it is critical that implementers of <see cref="AttributeSetterAttribute"/> ensure that the
/// property setters call <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, Boolean)"/> overload with
/// the final parameter set to false to disable the enforcement of business logic. Otherwise, an infinite loop will occur.
/// Calling that overload tells <see cref="AttributeValueCollection"/> that the business logic has already been enforced
/// by the caller. As this is an internal overload, implementers should use the local proxy at
/// property setters call <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, Boolean, DateTime?)"/>
/// overload with the final parameter set to false to disable the enforcement of business logic. Otherwise, an infinite
/// loop will occur. Calling that overload tells <see cref="AttributeValueCollection"/> that the business logic has
/// already been enforced by the caller. As this is an internal overload, implementers should use the local proxy at
/// <see cref="Topic.SetAttributeValue(String, String, Boolean?)"/>, which ensures that final parameter is set to false.
/// </para>
/// </remarks>
Expand Down
37 changes: 27 additions & 10 deletions OnTopic/Attributes/AttributeValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ namespace OnTopic.Attributes {
/// <para>
/// This class is immutable: once it is constructed, the values cannot be changed. To change a value, callers must either
/// create a new instance of the <see cref="AttributeValue"/> class or, preferably, call the
/// <see cref="Topic.Attributes"/>'s <see cref="AttributeValueCollection.SetValue(String, String, Boolean?)"/> method.
/// <see cref="Topic.Attributes"/>'s <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, DateTime?)"/>
/// method.
/// </para>
/// </remarks>
public class AttributeValue {
Expand Down Expand Up @@ -92,12 +93,28 @@ public AttributeValue(string key, string? value, bool isDirty = true) {
/// If disabled, <see cref="AttributeValueCollection"/> will not call local properties on <see cref="Topic"/> that
/// correspond to the <paramref name="key"/> as a means of enforcing the business logic.
/// </param>
/// <param name="lastModified">
/// The <see cref="DateTime"/> value that the attribute was last modified. This is intended exclusively for use when
/// populating the topic graph from a persistent data store as a means of indicating the current version for each
/// attribute. This is used when e.g. importing values to determine if the existing value is newer than the source value.
/// </param>
/// <requires
/// description="The key must be specified for the key/value pair." exception="T:System.ArgumentNullException">
/// !String.IsNullOrWhiteSpace(key)
/// </requires>
internal AttributeValue(string key, string? value, bool isDirty, bool enforceBusinessLogic) : this(key, value, isDirty) {
EnforceBusinessLogic = enforceBusinessLogic;
internal AttributeValue(
string key,
string? value,
bool isDirty,
bool enforceBusinessLogic,
DateTime? lastModified = null
): this(
key,
value,
isDirty
) {
EnforceBusinessLogic = enforceBusinessLogic;
LastModified = lastModified?? DateTime.Now;
}

/*==========================================================================================================================
Expand Down Expand Up @@ -147,12 +164,12 @@ internal AttributeValue(string key, string? value, bool isDirty, bool enforceBus
/// <see cref="AttributeValueCollection"/>.
/// </summary>
/// <remarks>
/// By default, when a user attempts to update an attribute's value by calling
/// <see cref="AttributeValueCollection.SetValue(String, String, Boolean?)"/>, or when an <see cref="AttributeValue"/> is
/// added to the <see cref="AttributeValueCollection"/>, the <see cref="AttributeValueCollection"/> will automatically
/// attempt to call any corresponding setters on <see cref="Topic"/> (or a derived instance) to ensure that the business
/// logic is enforced. To avoid an infinite loop, however, this is disabled when properties on <see cref="Topic"/> call
/// <see cref="Topic.SetAttributeValue(String, String, Boolean?)"/>. When that happens, the
/// By default, when a user attempts to update an attribute's value by calling <see
/// cref="AttributeValueCollection.SetValue(String, String, Boolean?, DateTime?)"/>, or when an <see cref="AttributeValue"
/// /> is added to the <see cref="AttributeValueCollection"/>, the <see cref="AttributeValueCollection"/> will
/// automatically attempt to call any corresponding setters on <see cref="Topic"/> (or a derived instance) to ensure that
/// the business logic is enforced. To avoid an infinite loop, however, this is disabled when properties on <see
/// cref="Topic"/> call <see cref="Topic.SetAttributeValue(String, String, Boolean?)"/>. When that happens, the
/// <see cref="EnforceBusinessLogic"/> value is set to false to communicate to the <see cref="AttributeValueCollection"/>
/// that it should not call the local property. This value is only intended for internal use.
/// </remarks>
Expand All @@ -172,7 +189,7 @@ internal AttributeValue(string key, string? value, bool isDirty, bool enforceBus
/// <summary>
/// Read-only reference to the last DateTime the <see cref="AttributeValue"/> instance was updated.
/// </summary>
public DateTime LastModified { get; } = DateTime.Now;
public DateTime LastModified { get; internal set; } = DateTime.Now;

} //Class
} //Namespace
32 changes: 22 additions & 10 deletions OnTopic/Collections/AttributeValueCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,11 @@ public bool IsDirty(string name) {
/// overwritten to accept whatever value is submitted. This can be used, for instance, to prevent an update from being
/// persisted to the data store on <see cref="Repositories.ITopicRepository.Save(Topic, Boolean, Boolean)"/>.
/// </param>
/// <param name="version">
/// The <see cref="DateTime"/> value that the attribute was last modified. This is intended exclusively for use when
/// populating the topic graph from a persistent data store as a means of indicating the current version for each
/// attribute. This is used when e.g. importing values to determine if the existing value is newer than the source value.
/// </param>
/// <requires
/// description="The key must be specified for the AttributeValue key/value pair."
/// exception="T:System.ArgumentNullException">
Expand All @@ -219,7 +224,8 @@ public bool IsDirty(string name) {
/// exception="T:System.ArgumentException">
/// !value.Contains(" ")
/// </requires>
public void SetValue(string key, string? value, bool? isDirty = null) => SetValue(key, value, isDirty, true);
public void SetValue(string key, string? value, bool? isDirty = null, DateTime? version = null)
=> SetValue(key, value, isDirty, true, version);

/// <summary>
/// Protected helper method that either adds a new <see cref="AttributeValue"/> object or updates the value of an existing
Expand All @@ -242,6 +248,11 @@ public bool IsDirty(string name) {
/// Instructs the underlying code to call corresponding properties, if available, to ensure business logic is enforced.
/// This should be set to false if setting attributes from internal properties in order to avoid an infinite loop.
/// </param>
/// <param name="version">
/// The <see cref="DateTime"/> value that the attribute was last modified. This is intended exclusively for use when
/// populating the topic graph from a persistent data store as a means of indicating the current version for each
/// attribute. This is used when e.g. importing values to determine if the existing value is newer than the source value.
/// </param>
/// <requires
/// description="The key must be specified for the AttributeValue key/value pair."
/// exception="T:System.ArgumentNullException">
Expand All @@ -257,7 +268,7 @@ public bool IsDirty(string name) {
/// exception="T:System.ArgumentException">
/// !value.Contains(" ")
/// </requires>
internal void SetValue(string key, string? value, bool? isDirty, bool enforceBusinessLogic) {
internal void SetValue(string key, string? value, bool? isDirty, bool enforceBusinessLogic, DateTime? version = null) {

/*------------------------------------------------------------------------------------------------------------------------
| Validate input
Expand Down Expand Up @@ -292,15 +303,15 @@ internal void SetValue(string key, string? value, bool? isDirty, bool enforceBus
else if (originalAttribute.Value != value) {
markAsDirty = true;
}
var newAttribute = new AttributeValue(key, value, markAsDirty, enforceBusinessLogic);
var newAttribute = new AttributeValue(key, value, markAsDirty, enforceBusinessLogic, version);
this[IndexOf(originalAttribute)] = newAttribute;
}

/*------------------------------------------------------------------------------------------------------------------------
| Create new attribute value
\-----------------------------------------------------------------------------------------------------------------------*/
else {
Add(new AttributeValue(key, value, isDirty ?? true, enforceBusinessLogic));
Add(new AttributeValue(key, value, isDirty ?? true, enforceBusinessLogic, version));
}

}
Expand All @@ -316,8 +327,8 @@ internal void SetValue(string key, string? value, bool? isDirty, bool enforceBus
/// <para>
/// If a settable property is available corresponding to the <see cref="AttributeValue.Key"/>, the call should be routed
/// through that to ensure local business logic is enforced. This is determined by looking for the "__" prefix, which is
/// set by the <see cref="SetValue(String, String, Boolean?, Boolean)"/>'s enforceBusinessLogic parameter. To avoid an
/// infinite loop, internal setters _must_ call this overload.
/// set by the <see cref="SetValue(String, String, Boolean?, Boolean, DateTime?)"/>'s enforceBusinessLogic parameter. To
/// avoid an infinite loop, internal setters _must_ call this overload.
/// </para>
/// <para>
/// Compared to the base implementation, will throw a specific <see cref="ArgumentException"/> error if a duplicate key
Expand Down Expand Up @@ -357,8 +368,8 @@ protected override void InsertItem(int index, AttributeValue item) {
/// <remarks>
/// If a settable property is available corresponding to the <see cref="AttributeValue.Key"/>, the call should be routed
/// through that to ensure local business logic is enforced. This is determined by looking for the "__" prefix, which is
/// set by the <see cref="SetValue(String, String, Boolean?, Boolean)"/>'s enforceBusinessLogic parameter. To avoid an
/// infinite loop, internal setters _must_ call this overload.
/// set by the <see cref="SetValue(String, String, Boolean?, Boolean, DateTime?)"/>'s enforceBusinessLogic parameter. To
/// avoid an infinite loop, internal setters _must_ call this overload.
/// </remarks>
/// <param name="index">The location that the <see cref="AttributeValue"/> should be set.</param>
/// <param name="item">The <see cref="AttributeValue"/> object which is being inserted.</param>
Expand All @@ -379,8 +390,8 @@ protected override void SetItem(int index, AttributeValue item) {
/// <remarks>
/// If a settable property is available corresponding to the <see cref="AttributeValue.Key"/>, the call should be routed
/// through that to ensure local business logic is enforced. This is determined by looking for the "__" prefix, which is
/// set by the <see cref="SetValue(String, String, Boolean?, Boolean)"/>'s enforceBusinessLogic parameter. To avoid an
/// infinite loop, internal setters _must_ call this overload.
/// set by the <see cref="SetValue(String, String, Boolean?, Boolean, DateTime?)"/>'s enforceBusinessLogic parameter. To
/// avoid an infinite loop, internal setters _must_ call this overload.
/// </remarks>
/// <param name="originalAttribute">The <see cref="AttributeValue"/> object which is being inserted.</param>
/// <param name="settableAttribute">
Expand All @@ -403,6 +414,7 @@ private bool EnforceBusinessLogic(AttributeValue originalAttribute, out Attribut
}
_typeCache.SetPropertyValue(_associatedTopic, originalAttribute.Key, originalAttribute.Value);
this[originalAttribute.Key].IsDirty = originalAttribute.IsDirty;
this[originalAttribute.Key].LastModified = originalAttribute.LastModified;
_setCounter = 0;
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion OnTopic/Topic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ public Topic? DerivedTopic {
/// called by the <see cref="AttributeValueCollection"/>. This is intended to enforce local business logic, and prevent
/// callers from introducing invalid data.To prevent a redirect loop, however, local properties need to inform the
/// <see cref="AttributeValueCollection"/> that the business logic has already been enforced. To do that, they must either
/// call <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, Boolean)"/> with the
/// call <see cref="AttributeValueCollection.SetValue(String, String, Boolean?, Boolean, DateTime?)"/> with the
/// <c>enforceBusinessLogic</c> flag set to <c>false</c>, or, if they're in a separate assembly, call this overload.
/// </remarks>
/// <param name="key">The string identifier for the AttributeValue.</param>
Expand Down

0 comments on commit a83b844

Please # to comment.