You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@avro.apache.org by GitBox <gi...@apache.org> on 2022/08/12 03:00:19 UTC
[GitHub] [avro] github-code-scanning[bot] commented on a diff in pull request #1823: Avro 3603 dotnet reflect refactoring 1
github-code-scanning[bot] commented on code in PR #1823:
URL: https://github.com/apache/avro/pull/1823#discussion_r944079023
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -46,13 +64,14 @@
throw new AvroException($"Type {dotnetClass.Name} is not a class");
}
- _nameClassMap.TryAdd(schema.Fullname, new DotnetClass(dotnetClass, schema, this));
+ AddClass(schema.Fullname, new DotnetClass(dotnetClass, schema, this));
Review Comment:
## Call to obsolete method
Call to obsolete method [AddClass](1).
[Show more details](https://github.com/apache/avro/security/code-scanning/2858)
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -46,13 +64,14 @@
throw new AvroException($"Type {dotnetClass.Name} is not a class");
}
- _nameClassMap.TryAdd(schema.Fullname, new DotnetClass(dotnetClass, schema, this));
+ AddClass(schema.Fullname, new DotnetClass(dotnetClass, schema, this));
Review Comment:
## Dereferenced variable may be null
Variable [schema](1) may be null here as suggested by [this](2) null check.
[Show more details](https://github.com/apache/avro/security/code-scanning/2864)
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -179,20 +190,15 @@
/// <returns></returns>
public DotnetClass GetClass(RecordSchema schema)
{
- DotnetClass c;
- if (!_nameClassMap.TryGetValue(schema.Fullname, out c))
- {
- return null;
- }
-
- return c;
+ return GetClass(schema.Fullname);
Review Comment:
## Dereferenced variable may be null
Variable [schema](1) may be null here because of [this](2) potential null argument.
[Show more details](https://github.com/apache/avro/security/code-scanning/2865)
##########
lang/csharp/src/apache/main/Reflect/Service/ArrayService.cs:
##########
@@ -0,0 +1,63 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Text;
+using Avro.Reflect.Array;
+
+namespace Avro.Reflect.Service
+{
+ /// <summary>
+ /// Additional functionality to serialize and deserialize arrays.
+ /// Works with array helpert
+ /// </summary>
+ public class ArrayService : IArrayService
+ {
+ private readonly IReflectCache _reflectCache;
+
+ /// <summary>
+ /// Public constructor
+ /// </summary>
+ /// <param name="reflectCache"></param>
+ public ArrayService(IReflectCache reflectCache)
+ {
+ _reflectCache = reflectCache;
+ }
+
+ /// <summary>
+ /// Find an array helper for an array schema node.
+ /// </summary>
+ /// <param name="schema">Schema</param>
+ /// <param name="enumerable">The array object. If it is null then Add(), Count() and Clear methods will throw exceptions.</param>
+ /// <returns></returns>
+ public IArrayHelper GetArrayHelper(ArraySchema schema, IEnumerable enumerable)
+ {
+ string s = GetHelperName(schema);
+
+ if (s != null && _reflectCache.TryGetArrayHelperType(s, out Type arrayHelperType))
+ {
+ return (IArrayHelper)Activator.CreateInstance(arrayHelperType, enumerable);
+ }
+
+ return (IArrayHelper)Activator.CreateInstance(typeof(ArrayHelper), enumerable);
+ }
+
+ internal string GetHelperName(ArraySchema ars)
+ {
+ // ArraySchema is unnamed schema and doesn't have a FulllName, use "helper" metadata.
+ // Metadata is json string, strip quotes
+
+ string s = null;
+ s = ars.GetProperty("helper");
+ if (s != null && s.Length > 2)
+ {
+ s = s.Substring(1, s.Length - 2);
+ }
+ else
+ {
+ s = null;
+ }
Review Comment:
## Missed ternary opportunity
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
[Show more details](https://github.com/apache/avro/security/code-scanning/2874)
##########
lang/csharp/src/apache/main/Reflect/ClassCache.cs:
##########
@@ -179,20 +190,15 @@
/// <returns></returns>
public DotnetClass GetClass(RecordSchema schema)
{
- DotnetClass c;
- if (!_nameClassMap.TryGetValue(schema.Fullname, out c))
- {
- return null;
- }
-
- return c;
+ return GetClass(schema.Fullname);
Review Comment:
## Call to obsolete method
Call to obsolete method [GetClass](1).
[Show more details](https://github.com/apache/avro/security/code-scanning/2859)
##########
lang/csharp/src/apache/main/Reflect/ReflectDefaultWriter.cs:
##########
@@ -188,9 +185,9 @@
return obj is string;
case Schema.Type.Error:
case Schema.Type.Record:
- return _classCache.GetClass(sc as RecordSchema).GetClassType() == obj.GetType();
+ return _reflectCache.GetClass(sc.Fullname).GetClassType() == obj.GetType();
Review Comment:
## Dereferenced variable may be null
Variable [obj](1) may be null here as suggested by [this](2) null check.
Variable [obj](1) may be null here as suggested by [this](3) null check.
[Show more details](https://github.com/apache/avro/security/code-scanning/2867)
##########
lang/csharp/src/apache/main/Reflect/ReflectDefaultWriter.cs:
##########
@@ -188,9 +185,9 @@
return obj is string;
case Schema.Type.Error:
case Schema.Type.Record:
- return _classCache.GetClass(sc as RecordSchema).GetClassType() == obj.GetType();
+ return _reflectCache.GetClass(sc.Fullname).GetClassType() == obj.GetType();
case Schema.Type.Enumeration:
- return EnumCache.GetEnumeration(sc as EnumSchema) == obj.GetType();
+ return _reflectCache.GetEnum(sc.Fullname) == obj.GetType();
Review Comment:
## Dereferenced variable may be null
Variable [obj](1) may be null here as suggested by [this](2) null check.
Variable [obj](1) may be null here as suggested by [this](3) null check.
[Show more details](https://github.com/apache/avro/security/code-scanning/2868)
##########
lang/csharp/src/apache/main/Reflect/ReflectDefaultReader.cs:
##########
@@ -383,7 +381,7 @@
var i = d.ReadEnum();
var symbol = writerSchema[i];
var es = readerSchema as EnumSchema;
- var enumType = EnumCache.GetEnumeration(es);
+ var enumType = _reflectCache.GetEnum(es.Fullname);
Review Comment:
## Dereferenced variable may be null
Variable [es](1) may be null here because of [this](2) assignment.
[Show more details](https://github.com/apache/avro/security/code-scanning/2866)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org