Skip to content

Conversation

@suarezrominajulieta
Copy link

No description provided.

Copy link
Collaborator

@arcuri82 arcuri82 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suarezrominajulieta thx for your first PR! see comments


when {

format.isCsharp() -> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't add for isCsharp(), as it is deprecated, and no longer in use (although we haven't cleaned the code to remove its occurrences)

}

var gene = getGene("body", obj.schema, schemaHolder,currentSchema, referenceClassDef = null, options = options, messages = messages, examples = examples)
val deref = obj.schema.`$ref`?.let { ref -> val name = ref.substringAfterLast("/")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hard to read. use better indentation, eg:

val deref = obj.schema.`$ref`?.let { ref -> 
                            val name = ref.substringAfterLast("/")
                            SchemaUtils.getReferenceSchema(schemaHolder, currentSchema, ref, messages) 
                   } ?: obj.schema

also, is name variable used in this block?

.replace("\"", """)
.replace("'", "'")

private fun singularize(n: String): String =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this for? also, it would not be working for irregular terms, eg, foot vs feet.
also, if really needed, should go into org.evomaster.core.utils.StringUtils

else -> n
}.replaceFirstChar { it.uppercase() }

private fun unwrap(v: Any?): Any? =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you want to unwrap genes, need to use Gene.getLeafGene().
also if needed, generic utils on genes should be in GeneUtils

public fun cleanXmlValueString(v: String): String =
v.removeSurrounding("\"").let(::escapeXmlSafe)

private fun getPrintedValue(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getPrintedValueForXml?

)
)

private fun isPrimitiveGene(value: Any?): Boolean =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the definition of "primitive" here? and is it something specific for XML only? many other genes could have to be included here... eg Regex, Uri, URL, and so on.
Depending on how it is used, and its definition, might need to consider to add a Gene.isPrimitive


is Gene -> "<$name>${getPrintedValue(previousGenes, v, targetFormat)}</$name>"

else -> "<$name>${cleanXmlValueString(v.toString())}</$name>"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we actually considering cases in which v is not a Gene? how would that happen? leave comment in the code if indeed it is possible

.keys

if (duplicated.isNotEmpty()) {
throw IllegalStateException("Duplicate child elements not allowed in XML: $duplicated")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this type of checks on the state of this fields should be done at construction time as well. also depends if they are mutable. as this is info from the schema (which can have mistakes), we should not crash, but rather flags errors in RestActionBuilderV3


assertEquals("{foo,bar,nested{hello}}", actual)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we also need E2E tests (ask @jgaleotti to show/explain you how they work):

  • at least 1 white-box in core-tests/e2e-tests/spring/spring-rest-openapi-v3
  • 1 in black-box in core-tests/e2e-tests/spring/spring-rest-bb

for example, could have API with endpoint same name but supporting incoming payload in both JSON and XML. for XML, could check some attributes are properly handled/parsed

@suarezrominajulieta
Copy link
Author

thank you for the comments! I'll look into them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants