From 4380679df65d3fc4f9d18147d636b95ef958b35a Mon Sep 17 00:00:00 2001 From: JustArchi Date: Wed, 29 Jun 2016 20:18:47 +0200 Subject: [PATCH] JSON code review --- ArchiSteamFarm/ArchiWebHandler.cs | 128 ++++++++++++++++++++--------- ArchiSteamFarm/JSON/Steam.cs | 132 +++++++++++++++--------------- ArchiSteamFarm/Runtime.cs | 1 - ArchiSteamFarm/Trading.cs | 4 +- 4 files changed, 161 insertions(+), 104 deletions(-) diff --git a/ArchiSteamFarm/ArchiWebHandler.cs b/ArchiSteamFarm/ArchiWebHandler.cs index ea753a64c..a4026db3c 100644 --- a/ArchiSteamFarm/ArchiWebHandler.cs +++ b/ArchiSteamFarm/ArchiWebHandler.cs @@ -484,7 +484,7 @@ namespace ArchiSteamFarm { return null; } - internal HashSet GetTradeOffers() { + internal HashSet GetActiveTradeOffers() { if (string.IsNullOrEmpty(Bot.BotConfig.SteamApiKey)) { Logging.LogNullError(nameof(Bot.BotConfig.SteamApiKey), Bot.BotName); return null; @@ -548,43 +548,103 @@ namespace ArchiSteamFarm { HashSet result = new HashSet(); foreach (KeyValue trade in response["trade_offers_received"].Children) { - Steam.TradeOffer tradeOffer = new Steam.TradeOffer { - TradeOfferID = trade["tradeofferid"].AsUnsignedLong(), - OtherSteamID3 = (uint) trade["accountid_other"].AsUnsignedLong(), - State = trade["trade_offer_state"].AsEnum() - }; + Steam.TradeOffer.ETradeOfferState state = trade["trade_offer_state"].AsEnum(); + if (state == Steam.TradeOffer.ETradeOfferState.Unknown) { + Logging.LogNullError(nameof(state)); + return null; + } - foreach (Steam.Item steamItem in trade["items_to_give"].Children.Select(item => new Steam.Item { - AppID = (uint) item["appid"].AsUnsignedLong(), - ContextID = item["contextid"].AsUnsignedLong(), - AssetID = item["assetid"].AsUnsignedLong(), - ClassID = item["classid"].AsUnsignedLong(), - InstanceID = item["instanceid"].AsUnsignedLong(), - Amount = (uint) item["amount"].AsUnsignedLong() - })) { - Tuple description; - if (descriptions.TryGetValue(steamItem.ClassID, out description)) { - steamItem.RealAppID = description.Item1; - steamItem.Type = description.Item2; + if (state != Steam.TradeOffer.ETradeOfferState.Active) { + continue; + } + + ulong tradeOfferID = trade["tradeofferid"].AsUnsignedLong(); + if (tradeOfferID == 0) { + Logging.LogNullError(nameof(tradeOfferID)); + return null; + } + + uint otherSteamID3 = (uint) trade["accountid_other"].AsUnsignedLong(); + if (otherSteamID3 == 0) { + Logging.LogNullError(nameof(otherSteamID3)); + return null; + } + + Steam.TradeOffer tradeOffer = new Steam.TradeOffer(tradeOfferID, otherSteamID3, state); + + foreach (KeyValue item in trade["items_to_give"].Children) { + uint appID = (uint) item["appid"].AsUnsignedLong(); + if (appID == 0) { + Logging.LogNullError(nameof(appID)); + return null; } + ulong contextID = item["contextid"].AsUnsignedLong(); + if (contextID == 0) { + Logging.LogNullError(nameof(contextID)); + return null; + } + + ulong classID = item["classid"].AsUnsignedLong(); + if (classID == 0) { + Logging.LogNullError(nameof(classID)); + return null; + } + + uint amount = (uint) item["amount"].AsUnsignedLong(); + if (amount == 0) { + Logging.LogNullError(nameof(amount)); + return null; + } + + uint realAppID = 0; + Steam.Item.EType type = Steam.Item.EType.Unknown; + + Tuple description; + if (descriptions.TryGetValue(classID, out description)) { + realAppID = description.Item1; + type = description.Item2; + } + + Steam.Item steamItem = new Steam.Item(appID, contextID, amount, realAppID, type); tradeOffer.ItemsToGive.Add(steamItem); } - foreach (Steam.Item steamItem in trade["items_to_receive"].Children.Select(item => new Steam.Item { - AppID = (uint) item["appid"].AsUnsignedLong(), - ContextID = item["contextid"].AsUnsignedLong(), - AssetID = item["assetid"].AsUnsignedLong(), - ClassID = item["classid"].AsUnsignedLong(), - InstanceID = item["instanceid"].AsUnsignedLong(), - Amount = (uint) item["amount"].AsUnsignedLong() - })) { - Tuple description; - if (descriptions.TryGetValue(steamItem.ClassID, out description)) { - steamItem.RealAppID = description.Item1; - steamItem.Type = description.Item2; + foreach (KeyValue item in trade["items_to_receive"].Children) { + uint appID = (uint) item["appid"].AsUnsignedLong(); + if (appID == 0) { + Logging.LogNullError(nameof(appID)); + return null; } + ulong contextID = item["contextid"].AsUnsignedLong(); + if (contextID == 0) { + Logging.LogNullError(nameof(contextID)); + return null; + } + + ulong classID = item["classid"].AsUnsignedLong(); + if (classID == 0) { + Logging.LogNullError(nameof(classID)); + return null; + } + + uint amount = (uint) item["amount"].AsUnsignedLong(); + if (amount == 0) { + Logging.LogNullError(nameof(amount)); + return null; + } + + uint realAppID = 0; + Steam.Item.EType type = Steam.Item.EType.Unknown; + + Tuple description; + if (descriptions.TryGetValue(classID, out description)) { + realAppID = description.Item1; + type = description.Item2; + } + + Steam.Item steamItem = new Steam.Item(appID, contextID, amount, realAppID, type); tradeOffer.ItemsToReceive.Add(steamItem); } @@ -803,13 +863,7 @@ namespace ArchiSteamFarm { itemID = 0; } - singleTrade.ItemsToGive.Assets.Add(new Steam.Item { - AppID = Steam.Item.SteamAppID, - ContextID = Steam.Item.SteamContextID, - Amount = item.Amount, - AssetID = item.AssetID - }); - + singleTrade.ItemsToGive.Assets.Add(new Steam.Item(Steam.Item.SteamAppID, Steam.Item.SteamContextID, item.AssetID, item.Amount)); itemID++; } diff --git a/ArchiSteamFarm/JSON/Steam.cs b/ArchiSteamFarm/JSON/Steam.cs index fec93251d..da76eb5d4 100644 --- a/ArchiSteamFarm/JSON/Steam.cs +++ b/ArchiSteamFarm/JSON/Steam.cs @@ -22,6 +22,7 @@ */ +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -33,6 +34,7 @@ using SteamKit2; namespace ArchiSteamFarm.JSON { internal static class Steam { internal sealed class Item { // REF: https://developer.valvesoftware.com/wiki/Steam_Web_API/IEconService#CEcon_Asset + // Deserialized from JSON (SteamCommunity) and constructed from code internal const ushort SteamAppID = 753; internal const byte SteamContextID = 6; @@ -50,7 +52,7 @@ namespace ArchiSteamFarm.JSON { TradingCard } - internal uint AppID { get; set; } + internal uint AppID { get; private set; } [JsonProperty(PropertyName = "appid", Required = Required.DisallowNull)] [SuppressMessage("ReSharper", "UnusedMember.Local")] @@ -73,7 +75,7 @@ namespace ArchiSteamFarm.JSON { } } - internal ulong ContextID { get; set; } + internal ulong ContextID { get; private set; } [JsonProperty(PropertyName = "contextid", Required = Required.DisallowNull)] [SuppressMessage("ReSharper", "UnusedMember.Local")] @@ -96,7 +98,7 @@ namespace ArchiSteamFarm.JSON { } } - internal ulong AssetID { get; set; } + internal ulong AssetID { get; private set; } [JsonProperty(PropertyName = "assetid", Required = Required.DisallowNull)] private string AssetIDString { @@ -125,7 +127,7 @@ namespace ArchiSteamFarm.JSON { set { AssetIDString = value; } } - internal ulong ClassID { get; set; } + internal ulong ClassID { get; private set; } [JsonProperty(PropertyName = "classid", Required = Required.DisallowNull)] [SuppressMessage("ReSharper", "UnusedMember.Local")] @@ -148,30 +150,7 @@ namespace ArchiSteamFarm.JSON { } } - internal ulong InstanceID { private get; set; } - - [JsonProperty(PropertyName = "instanceid", Required = Required.DisallowNull)] - [SuppressMessage("ReSharper", "UnusedMember.Local")] - private string InstanceIDString { - get { - return InstanceID.ToString(); - } - - set { - if (string.IsNullOrEmpty(value)) { - return; - } - - ulong result; - if (!ulong.TryParse(value, out result)) { - return; - } - - InstanceID = result; - } - } - - internal uint Amount { get; set; } + internal uint Amount { get; private set; } [JsonProperty(PropertyName = "amount", Required = Required.Always)] [SuppressMessage("ReSharper", "UnusedMember.Local")] @@ -196,9 +175,37 @@ namespace ArchiSteamFarm.JSON { internal uint RealAppID { get; set; } internal EType Type { get; set; } + + internal Item(uint appID, ulong contextID, ulong assetID, uint amount) { + if ((appID == 0) || (contextID == 0) || (assetID == 0) || (amount == 0)) { + throw new ArgumentNullException(nameof(appID) + " || " + nameof(contextID) + " || " + nameof(assetID) + " || " + nameof(amount)); + } + + AppID = appID; + ContextID = contextID; + AssetID = assetID; + Amount = amount; + } + + internal Item(uint appID, ulong contextID, uint amount, uint realAppID, EType type) { + if ((appID == 0) || (contextID == 0) || (amount == 0)) { + throw new ArgumentNullException(nameof(appID) + " || " + nameof(contextID) + " || " + nameof(amount)); + } + + AppID = appID; + ContextID = contextID; + Amount = amount; + RealAppID = realAppID; + Type = type; + } + + [SuppressMessage("ReSharper", "UnusedMember.Local")] + private Item() { } } internal sealed class TradeOffer { // REF: https://developer.valvesoftware.com/wiki/Steam_Web_API/IEconService#CEcon_TradeOffer + // Constructed from code + [SuppressMessage("ReSharper", "UnusedMember.Global")] internal enum ETradeOfferState : byte { Unknown, @@ -215,43 +222,40 @@ namespace ArchiSteamFarm.JSON { OnHold } - internal ulong TradeOfferID { get; set; } + internal readonly ulong TradeOfferID; - [JsonProperty(PropertyName = "tradeofferid", Required = Required.Always)] - [SuppressMessage("ReSharper", "UnusedMember.Local")] - private string TradeOfferIDString { + internal readonly ETradeOfferState State; + internal readonly HashSet ItemsToGive = new HashSet(); + internal readonly HashSet ItemsToReceive = new HashSet(); + + private readonly uint OtherSteamID3; + + private ulong _OtherSteamID64; + internal ulong OtherSteamID64 { get { - return TradeOfferID.ToString(); - } - - set { - if (string.IsNullOrEmpty(value)) { - return; + if (_OtherSteamID64 != 0) { + return _OtherSteamID64; } - ulong result; - if (!ulong.TryParse(value, out result)) { - return; + if (OtherSteamID3 == 0) { + Logging.LogNullError(nameof(OtherSteamID3)); + return 0; } - TradeOfferID = result; + _OtherSteamID64 = new SteamID(OtherSteamID3, EUniverse.Public, EAccountType.Individual); + return _OtherSteamID64; } } - [JsonProperty(PropertyName = "accountid_other", Required = Required.Always)] - internal uint OtherSteamID3 { private get; set; } + internal TradeOffer(ulong tradeOfferID, uint otherSteamID3, ETradeOfferState state) { + if ((tradeOfferID == 0) || (otherSteamID3 == 0) || (state == ETradeOfferState.Unknown)) { + throw new ArgumentNullException(nameof(tradeOfferID) + " || " + nameof(otherSteamID3) + " || " + nameof(state)); + } - [JsonProperty(PropertyName = "trade_offer_state", Required = Required.Always)] - internal ETradeOfferState State { get; set; } - - [JsonProperty(PropertyName = "items_to_give", Required = Required.Always)] - internal HashSet ItemsToGive { get; } = new HashSet(); - - [JsonProperty(PropertyName = "items_to_receive", Required = Required.Always)] - internal HashSet ItemsToReceive { get; } = new HashSet(); - - // Extra - internal ulong OtherSteamID64 => OtherSteamID3 == 0 ? 0 : new SteamID(OtherSteamID3, EUniverse.Public, EAccountType.Individual); + TradeOfferID = tradeOfferID; + OtherSteamID3 = otherSteamID3; + State = state; + } internal bool IsSteamCardsOnlyTradeForUs() => ItemsToGive.All(item => (item.AppID == Item.SteamAppID) && (item.ContextID == Item.SteamContextID) && (item.Type == Item.EType.TradingCard)); @@ -313,34 +317,32 @@ namespace ArchiSteamFarm.JSON { [SuppressMessage("ReSharper", "UnusedMember.Global")] internal sealed class TradeOfferRequest { + // Constructed from code internal sealed class ItemList { [JsonProperty(PropertyName = "assets", Required = Required.Always)] - internal HashSet Assets { get; } = new HashSet(); + internal readonly HashSet Assets = new HashSet(); } - [JsonProperty(PropertyName = "newversion", Required = Required.Always)] - internal bool NewVersion { get; } = true; - - [JsonProperty(PropertyName = "version", Required = Required.Always)] - internal byte Version { get; } = 2; - [JsonProperty(PropertyName = "me", Required = Required.Always)] - internal ItemList ItemsToGive { get; } = new ItemList(); + internal readonly ItemList ItemsToGive = new ItemList(); [JsonProperty(PropertyName = "them", Required = Required.Always)] - internal ItemList ItemsToReceive { get; } = new ItemList(); + internal readonly ItemList ItemsToReceive = new ItemList(); } [SuppressMessage("ReSharper", "ClassNeverInstantiated.Global")] [SuppressMessage("ReSharper", "UnusedAutoPropertyAccessor.Local")] internal sealed class ConfirmationResponse { + // Deserialized from JSON [JsonProperty(PropertyName = "success", Required = Required.Always)] internal bool Success { get; private set; } } + [SuppressMessage("ReSharper", "ClassCannotBeInstantiated")] [SuppressMessage("ReSharper", "ClassNeverInstantiated.Global")] [SuppressMessage("ReSharper", "UnusedAutoPropertyAccessor.Local")] internal sealed class ConfirmationDetails { + // Deserialized from JSON internal enum EType : byte { Unknown, Trade, @@ -495,6 +497,8 @@ namespace ArchiSteamFarm.JSON { return _HtmlDocument; } } + + private ConfirmationDetails() { } } } } diff --git a/ArchiSteamFarm/Runtime.cs b/ArchiSteamFarm/Runtime.cs index 97b95ee42..2435dd4f9 100644 --- a/ArchiSteamFarm/Runtime.cs +++ b/ArchiSteamFarm/Runtime.cs @@ -23,7 +23,6 @@ */ using System; -using System.IO; using System.Reflection; namespace ArchiSteamFarm { diff --git a/ArchiSteamFarm/Trading.cs b/ArchiSteamFarm/Trading.cs index 6b02427a3..77b4ea80e 100644 --- a/ArchiSteamFarm/Trading.cs +++ b/ArchiSteamFarm/Trading.cs @@ -95,12 +95,12 @@ namespace ArchiSteamFarm { return; } - HashSet tradeOffers = Bot.ArchiWebHandler.GetTradeOffers(); + HashSet tradeOffers = Bot.ArchiWebHandler.GetActiveTradeOffers(); if ((tradeOffers == null) || (tradeOffers.Count == 0)) { return; } - if (tradeOffers.RemoveWhere(tradeoffer => (tradeoffer.State != Steam.TradeOffer.ETradeOfferState.Active) || IgnoredTrades.Contains(tradeoffer.TradeOfferID)) > 0) { + if (tradeOffers.RemoveWhere(tradeoffer => IgnoredTrades.Contains(tradeoffer.TradeOfferID)) > 0) { tradeOffers.TrimExcess(); if (tradeOffers.Count == 0) { return;