The default bug view has changed. See this FAQ.

Implement XML Schema Validation for Complex Types

RESOLVED FIXED

Status

()

Core
Web Services
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Doron Rosenberg (IBM), Assigned: Doron Rosenberg (IBM))

Tracking

({fixed1.8.0.4, fixed1.8.1})

Trunk
x86
Linux
fixed1.8.0.4, fixed1.8.1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

12 years ago
 
(Assignee)

Comment 1

12 years ago
Created attachment 194663 [details] [diff] [review]
work in progress, just in case something goes bad with my new box
(Assignee)

Comment 2

12 years ago
Created attachment 196037 [details] [diff] [review]
current progress

Current progress, includes the inherited simpletype patch and the attribute
qualification patch in 2 other bugs.

testsuite also added, currently the 100 complex type tests results are 100%
identical in this code and xerces-J
Attachment #194663 - Attachment is obsolete: true
(Assignee)

Comment 3

11 years ago
Created attachment 213780 [details] [diff] [review]
current progress - trunk

Latest progress, requires trunk due to a change to nsISchema that isn't on the branches.
Attachment #196037 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Blocks: 326556
(Assignee)

Comment 4

11 years ago
Created attachment 215933 [details] [diff] [review]
latest progress

Pretty much reviewable
Attachment #213780 - Attachment is obsolete: true
(Assignee)

Comment 5

11 years ago
Created attachment 216052 [details] [diff] [review]
first reviewable patch

I didn't diff my testsuite stuff here, just c++ changes.  I also fixed some minor datetime parsing bugs (wasn't testing length requirements).
Attachment #215933 - Attachment is obsolete: true
Attachment #216052 - Flags: review?(aaronr)

Comment 6

11 years ago
Comment on attachment 216052 [details] [diff] [review]
first reviewable patch


>Index: extensions/schema-validation/src/nsSchemaValidator.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/extensions/schema-validation/src/nsSchemaValidator.cpp,v
>retrieving revision 1.22
>diff -u -r1.22 nsSchemaValidator.cpp
>--- extensions/schema-validation/src/nsSchemaValidator.cpp	21 Mar 2006 16:39:35 -0000	1.22
>+++ extensions/schema-validation/src/nsSchemaValidator.cpp	23 Mar 2006 21:24:00 -0000


>+  /* 
>+   * We allow the schema validator to continue validating a structure
>+   * even if the nodevalue is invalid per its simpletype binding.  We remember
>+   * this by using mForceInvalid as an override for the final return.
>+   */
>+

Ummm, why do all this work if you are just going to make aResult PR_FALSE anyhow?  Just
for the rv?  Please comment why you do this.

>+  PRBool isValid = PR_FALSE;
>+  rv = ValidateAgainstType(aElement, type, &isValid);
>+
>+  *aResult = mForceInvalid ? PR_FALSE : isValid;
>+  return rv;
> }
> 
> NS_IMETHODIMP
>@@ -253,9 +308,38 @@
>         NS_ConvertUTF16toUTF8(nodeValue).get()));
> 
>       rv = ValidateSimpletype(nodeValue, simpleType, &isValid);


please use -p when you cvs diff.  Nice to know what function you are changing w/o
having to open the file and go to the line number.

Also, no sense assigning into rv if you aren't going to use it.

>+
>+      nsAutoString typeName, nodeName;
>+      rv = aType->GetName(typeName);
>+      NS_ENSURE_SUCCESS(rv, rv);
>+
>+      rv = aElement->GetLocalName(nodeName);
>+      NS_ENSURE_SUCCESS(rv, rv);
>+
>+      // go on validating, but remember we failed
>+      if (!isValid) {
>+        mForceInvalid = PR_TRUE;
>+        isValid = PR_TRUE;
>+      }
>+
>+      // set the property so that callers can check validity of nodes
>+      nsCOMPtr<nsIContent> content = do_QueryInterface(aElement);
>+      NS_ENSURE_STATE(content);
>+
>+      nsCOMPtr<nsIAtom> myAtom = do_GetAtom("xsdtype");
>+
>+      // We need to make aType live one cycle longer, so that getProperty
>+      // can access it.  ReleaseObject will take care of the releasing.
>+      NS_ADDREF(aType);
>+      rv = content->SetProperty(myAtom, aType, ReleaseObject);
>+      NS_ENSURE_SUCCESS(rv, rv);
>     }
>   } else if (typevalue == nsISchemaType::SCHEMA_TYPE_COMPLEX) {
>-    rv = NS_ERROR_NOT_IMPLEMENTED;
>+    nsCOMPtr<nsISchemaComplexType> complexType = do_QueryInterface(aType);
>+    if (complexType) {
>+      LOG(("  Type is a complex type!"));
>+      rv = ValidateComplextype(aElement, complexType, &isValid);
>+    }
>   } else {
>     rv = NS_ERROR_UNEXPECTED;
>   }
>@@ -278,7 +362,7 @@
>     // if we are a built-in type, we can get a nsISchemaType for it from
>     // nsISchemaCollection->GetType.
>     nsCOMPtr<nsISchemaLoader> schemaLoader =
>-      do_GetService("@mozilla.org/xmlextras/schemas/schemaloader;1", &rv);
>+      do_CreateInstance("@mozilla.org/xmlextras/schemas/schemaloader;1", &rv);
>     NS_ENSURE_SUCCESS(rv, rv);
>     mSchema = do_QueryInterface(schemaLoader);
>     NS_ENSURE_STATE(mSchema);
>@@ -1504,7 +1588,7 @@
>   // GDay looks like this: ---DD(Z|(+|-)hh:mm)
> 
>   PRUint32 strLength = aNodeValue.Length();
>-  //   ---DD               ---DDZ              ---DD(+|-)hh:mm
>+  //   ---DD            ---DDZ            ---DD(+|-)hh:mm
>   if (strLength != 5 && strLength != 6 && strLength != 11)
>     return PR_FALSE;
> 
>@@ -1628,7 +1712,7 @@
>   // GMonth looks like this: --MM(Z|(+|-)hh:mm)
> 
>   PRUint32 strLength = aNodeValue.Length();
>-  //   --MM              --MMZ             --MM(+|-)hh:mm
>+  //   --MM                --MMZ               --MM(+|-)hh:mm
>   if ((strLength != 4) && (strLength != 5) && (strLength != 10))
>     return PR_FALSE;
> 
>@@ -1642,7 +1726,7 @@
>   aNodeValue.EndReading(end);
>   nsAutoString nodeValue(aNodeValue);
> 
>-  // validate the --MM-- part
>+  // validate the --MM part
>   PRBool isValid = Substring(start.get(), start.get()+2).EqualsLiteral("--") &&
>                    IsValidSchemaGType(Substring(start.get()+2, start.get()+4),
>                                       1, 12, &monthInt);
>@@ -1931,7 +2015,8 @@
>       // back one up since we have the separator included
>       year = Substring(buffStart, --start);
> 
>-      isValid = IsValidSchemaGYear(year, aYearMonth ? &aYearMonth->gYear : nsnull);
>+      isValid = IsValidSchemaGYear(year,
>+                                   aYearMonth ? &aYearMonth->gYear : nsnull);
> 
>       if (isValid) {
>         nsAutoString month;
>@@ -1939,7 +2024,8 @@
>         start++;
>         month.Append(Substring(start, end));
> 
>-        isValid = IsValidSchemaGMonth(month, aYearMonth ? &aYearMonth->gMonth : nsnull);
>+        isValid = IsValidSchemaGMonth(month,
>+                                      aYearMonth ? &aYearMonth->gMonth : nsnull);
>       }
>       done = PR_TRUE;
>     } else {
>@@ -2054,7 +2140,8 @@
>       nsAutoString month;
>       month.AppendLiteral("--");
>       month.Append(Substring(buffStart, start.advance(2)));
>-      isValid = IsValidSchemaGMonth(month, aMonthDay ? &aMonthDay->gMonth : nsnull);
>+      isValid = IsValidSchemaGMonth(month,
>+                                    aMonthDay ? &aMonthDay->gMonth : nsnull);
> 
>       if (isValid) {
>         buffStart = start;
>@@ -2067,7 +2154,8 @@
>       nsAutoString day;
>       day.AppendLiteral("---");
>       day.Append(Substring(++buffStart, end));
>-      isValid = IsValidSchemaGDay(day, aMonthDay ? &aMonthDay->gDay : nsnull);
>+      isValid = IsValidSchemaGDay(day,
>+                                  aMonthDay ? &aMonthDay->gDay : nsnull);
>       done = PR_TRUE;
>     }
>   }
>@@ -2361,7 +2449,6 @@
>     char fulldate[100] = "";
>     nsCAutoString monthShorthand;
>     nsSchemaValidatorUtils::GetMonthShorthand(dateTime.date.month, monthShorthand);
>-
>     // 22-AUG-1993 10:59:12.82
>     sprintf(fulldate, "%d-%s-%u %d:%d:%d.%u",
>       dateTime.date.day,
>@@ -2398,9 +2485,8 @@
> 
>   nsAutoString dateString(aNodeValue); 
>   if (dateString.First() == '-') {
>-    /* nspr can't handle negative years it seems */
>     aResult->isNegative = PR_TRUE;
>- }
>+  }
> 
>  /*
>     http://www.w3.org/TR/xmlschema-2/#date
>@@ -2507,7 +2593,8 @@
>   PRBool isValid = PR_FALSE;
>   nsCOMPtr<nsISchemaDuration> duration;
> 
>-  isValid = nsSchemaValidatorUtils::ParseSchemaDuration(aNodeValue, getter_AddRefs(duration));
>+  isValid = nsSchemaValidatorUtils::ParseSchemaDuration(aNodeValue,
>+                                                        getter_AddRefs(duration));
> 
>   duration.swap(*aResult);
>   return isValid;
>@@ -2656,7 +2743,8 @@
>   }
> 
>   if (isValid && aEnumerationList && (aEnumerationList->Count() > 0)) {
>-    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue, *aEnumerationList);
>+    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue,
>+                                                        *aEnumerationList);
>   }
> 
>  *aResult = isValid;
>@@ -2738,7 +2826,8 @@
>   }
> 
>   if (isValid && aEnumerationList && (aEnumerationList->Count() > 0)) {
>-    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue, *aEnumerationList);
>+    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue,
>+                                                        *aEnumerationList);
>   }
> 
> #ifdef PR_LOGGING
>@@ -2878,7 +2967,8 @@
>   }
> 
>   if (isValid && aEnumerationList && (aEnumerationList->Count() > 0)) {
>-    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue, *aEnumerationList);
>+    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue,
>+                                                        *aEnumerationList);
>   }
> 
> #ifdef PR_LOGGING
>@@ -3009,7 +3099,8 @@
>             (!aMaxLength || length <= aMaxLength);
> 
>   if (isValid && aEnumerationList && (aEnumerationList->Count() > 0)) {
>-    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue, *aEnumerationList);
>+    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue,
>+                                                        *aEnumerationList);
>   }
> 
>   *aResult = isValid;
>@@ -3051,7 +3142,7 @@
>   if (isValid) {
>     length = strlen(decodedString);
> 
>-    if (aLengthDefined && (length != aLength)) {  
>+    if (aLengthDefined && (length != aLength)) {
>       isValid = PR_FALSE;
>       LOG(("  Not valid: Not the right length (%d)", length));
>     }
>@@ -3068,7 +3159,8 @@
>   }
> 
>   if (isValid && aEnumerationList && (aEnumerationList->Count() > 0)) {
>-    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue, *aEnumerationList);
>+    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue,
>+                                                        *aEnumerationList);
>   }
> 
> #ifdef PR_LOGGING
>@@ -3126,7 +3218,8 @@
>   }
> 
>   if (isValid && aEnumerationList && (aEnumerationList->Count() > 0)) {
>-    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue, *aEnumerationList);
>+    isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue,
>+                                                        *aEnumerationList);
>   }
> 
> #ifdef PR_LOGGING
>@@ -3195,7 +3288,8 @@
>     }
> 
>     if (isValid && aEnumerationList && (aEnumerationList->Count() > 0)) {
>-      isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue, *aEnumerationList);
>+      isValid = nsSchemaValidatorUtils::HandleEnumeration(aNodeValue,
>+                                                          *aEnumerationList);
>     }
>   }
> 
>@@ -3245,3 +3339,1053 @@
>   LOG(("  Base Type is %s (%d)", NS_ConvertUTF16toUTF8(typeName).get(),foo));
> }
> #endif
>+
>+
>+
>+/****************************************************
>+ * Complex Type Validation                          *
>+ ****************************************************/
>+
>+// http://w3c.org/TR/xmlschema-1/#Complex_Type_Definitions
>+nsresult
>+nsSchemaValidator::ValidateComplextype(nsIDOMNode* aNode,
>+                                       nsISchemaComplexType *aSchemaComplexType,
>+                                       PRBool *aResult)
>+{
>+  PRBool isValid = PR_FALSE;

nit: initialize *aResult to false here?  Otherwise in some error conditions
you are setting it to false, other times you don't do anything.

>+
>+  PRUint16 contentModel;
>+  nsresult rv = aSchemaComplexType->GetContentModel(&contentModel);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  switch(contentModel) {
>+    case nsISchemaComplexType::CONTENT_MODEL_EMPTY: {
>+      // element has no children
>+      rv = NS_ERROR_NOT_IMPLEMENTED;
>+      break;
>+    }
>+
>+    case nsISchemaComplexType::CONTENT_MODEL_SIMPLE: {
>+      LOG(("    complex type, simple content"));
>+      rv = ValidateComplexModelSimple(aNode, aSchemaComplexType, &isValid);
>+      break;
>+    }
>+
>+    case nsISchemaComplexType::CONTENT_MODEL_ELEMENT_ONLY: {
>+      LOG(("    xsd:element only"));
>+      rv = ValidateComplexModelElement(aNode, aSchemaComplexType, &isValid);
>+      break;
>+    }
>+
>+    case nsISchemaComplexType::CONTENT_MODEL_MIXED: {
>+      rv = NS_ERROR_NOT_IMPLEMENTED;
>+      break;
>+    }
>+  }

nit: if isValid is false, can't you just exit out right here?

>+
>+  // if we are valid, validate the attributes
>+  if (isValid) {
>+    nsCOMPtr<nsIDOMNamedNodeMap> attrMap;
>+    rv = aNode->GetAttributes(getter_AddRefs(attrMap));
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    PRUint32 attrCount = 0, nsAttrCount = 0, totalAttr = 0, foundAttrCount = 0;
>+

maybe name nsAttrCount to omitAttrCount or ignoreAttrCount or something similar.
When I see nsAttrCount, I'm not thinking namespace, but rather netscape.  So 
makes the code a little less understandable at a quick glance.

>+nsresult
>+nsSchemaValidator::ValidateComplexModelSimple(nsIDOMNode* aNode,
>+                                              nsISchemaComplexType *aSchemaComplexType,
>+                                              PRBool *aResult)
>+{
>+  PRBool isValid = PR_FALSE;
>+
>+  PRUint16 derivation;;
>+  nsresult rv = aSchemaComplexType->GetDerivation(&derivation);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  // two choices for complex model with simple content: derivation through
>+  // extension or through restriction
>+  switch(derivation) {
>+    case nsISchemaComplexType::DERIVATION_EXTENSION_SIMPLE: {
>+      LOG(("      -- deriviation by extension of a simple type"));
>+
>+      nsCOMPtr<nsISchemaSimpleType> simpleBaseType;
>+      rv = aSchemaComplexType->GetSimpleBaseType(getter_AddRefs(simpleBaseType));
>+      NS_ENSURE_SUCCESS(rv, rv);
>+
>+      nsAutoString nodeValue;
>+      nsCOMPtr<nsIDOM3Node> domNode3 = do_QueryInterface(aNode);
>+      domNode3->GetTextContent(nodeValue);
>+
>+      rv = ValidateSimpletype(nodeValue, simpleBaseType, &isValid);
>+      break;
>+    }
>+
>+    case nsISchemaComplexType::DERIVATION_RESTRICTION_SIMPLE: {
>+      LOG(("      -- deriviation by restriction of a simple type"));
>+
>+      nsCOMPtr<nsISchemaSimpleType> simpleBaseType;
>+      rv = aSchemaComplexType->GetSimpleBaseType(getter_AddRefs(simpleBaseType));
>+      NS_ENSURE_SUCCESS(rv, rv);
>+
>+      nsAutoString nodeValue;
>+      nsCOMPtr<nsIDOM3Node> domNode3 = do_QueryInterface(aNode);
>+      rv = domNode3->GetTextContent(nodeValue);
>+      NS_ENSURE_SUCCESS(rv, rv);
>+
>+      rv = ValidateSimpletype(nodeValue, simpleBaseType, &isValid);
>+      break;
>+    }
>+  }
>+

Ummm...aren't you doing the exact same thing in both cases?  Well, except for the LOG
statement.

>+
>+nsresult
>+nsSchemaValidator::ValidateComplexSequence(nsIDOMNode* aStartNode,
>+                                           nsISchemaModelGroup *aSchemaModelGroup,
>+                                           nsIDOMNode **aLeftOvers,
>+                                           PRBool *aNotFound, PRBool *aResult)
>+{
>+  if (!aStartNode || !aSchemaModelGroup)
>+    return NS_ERROR_UNEXPECTED;
>+
>+  PRBool isValid = PR_FALSE;
>+  PRBool notFound = PR_FALSE;
>+
>+  // get the model group details
>+  PRUint32 minOccurs;
>+  nsresult rv = aSchemaModelGroup->GetMinOccurs(&minOccurs);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  PRUint32 maxOccurs;
>+  rv = aSchemaModelGroup->GetMaxOccurs(&maxOccurs);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  PRUint32 particleCount;
>+  rv = aSchemaModelGroup->GetParticleCount(&particleCount);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  // xsd:sequence means that the order of the particles matters
>+  PRUint32 validatedNodes = 0;
>+  PRUint32 particleCounter = 0;
>+  PRUint16 nodeType;
>+  PRBool done = PR_FALSE;
>+  nsCOMPtr<nsISchemaParticle> particle;
>+  nsCOMPtr<nsIDOMNode> currentNode(aStartNode), leftOvers, tmpNode;
>+
>+  LOG(("====================== New Sequence ==========================="));
>+
>+  // while valid and not done
>+  // we are done when we hit a node that doesn't fit our schema
>+  while (!done && currentNode && (particleCounter < particleCount)) {
>+    // get node type
>+    currentNode->GetNodeType(&nodeType);
>+
>+    // if not an element node, skip
>+    if (nodeType != nsIDOMNode::ELEMENT_NODE) {
>+      currentNode->GetNextSibling(getter_AddRefs(tmpNode));
>+      currentNode = tmpNode;
>+      continue;
>+    }
>+
>+    // get the particle
>+    rv = aSchemaModelGroup->GetParticle(particleCounter,
>+                                        getter_AddRefs(particle));
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    rv = ValidateComplexParticle(currentNode, particle,
>+                                 getter_AddRefs(leftOvers), &notFound, &isValid);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    // if we found the node, increment the amount of validated nodes.
>+    if (!notFound)
>+      validatedNodes++;
>+
>+    if (isValid) {
>+      particleCounter++;
>+    } else {
>+      done = PR_TRUE;
>+    }
>+
>+    // valid and not found means it was optional, so ok
>+    if (isValid && notFound) {
>+      isValid = PR_TRUE;
>+      notFound = PR_FALSE;
>+    }

isValid is already true.  Why set it again?

I'm about 1/2 of the way through the review.  More tomorrow.
(Assignee)

Comment 7

11 years ago
(In reply to comment #6)
> (From update of attachment 216052 [details] [diff] [review] [edit])
> 
> >Index: extensions/schema-validation/src/nsSchemaValidator.cpp
> >===================================================================
> >RCS file: /cvsroot/mozilla/extensions/schema-validation/src/nsSchemaValidator.cpp,v
> >retrieving revision 1.22
> >diff -u -r1.22 nsSchemaValidator.cpp
> >--- extensions/schema-validation/src/nsSchemaValidator.cpp	21 Mar 2006 16:39:35 -0000	1.22
> >+++ extensions/schema-validation/src/nsSchemaValidator.cpp	23 Mar 2006 21:24:00 -0000
> 
> 
> >+  /* 
> >+   * We allow the schema validator to continue validating a structure
> >+   * even if the nodevalue is invalid per its simpletype binding.  We remember
> >+   * this by using mForceInvalid as an override for the final return.
> >+   */
> >+
> 
> Ummm, why do all this work if you are just going to make aResult PR_FALSE
> anyhow?  Just
> for the rv?  Please comment why you do this.
> 

Reason is, so that we continue marking nodes' schema types even if we encounter a node value that is invalid.  But yeah, needs to be commented better.

thanks for reviewing!

Comment 8

11 years ago
Comment on attachment 216052 [details] [diff] [review]
first reviewable patch


>+nsresult
>+nsSchemaValidator::ValidateComplexElement(nsIDOMNode* aNode,
>+                                          nsISchemaParticle *aSchemaParticle,
>+                                          PRBool *aResult)
>+{
>+  PRBool isValid = PR_FALSE;
>+
>+  nsCOMPtr<nsISchemaElement> schemaElement = do_QueryInterface(aSchemaParticle);
>+  if (!schemaElement)
>+    return NS_ERROR_UNEXPECTED;
>+

nit: according to nsCOMPtr docs, should use 'direct initialisation' rather than
'copy initialisation' :-)  All over your code.  I won't mention it at each place.


>+nsresult
>+nsSchemaValidator::ValidateComplexChoice(nsIDOMNode* aStartNode,
>+                                         nsISchemaModelGroup *aSchemaModelGroup,
>+                                         nsIDOMNode **aLeftOvers,
>+                                         PRBool *aNotFound, PRBool *aResult)
>+{
>+  // get the model group details
>+  PRUint32 minOccurs;
>+  nsresult rv = aSchemaModelGroup->GetMinOccurs(&minOccurs);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  PRUint32 maxOccurs;
>+  rv = aSchemaModelGroup->GetMaxOccurs(&maxOccurs);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  PRUint32 particleCount;
>+  rv = aSchemaModelGroup->GetParticleCount(&particleCount);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  nsCOMPtr<nsIDOMNodeList> nodeList;
>+  aStartNode->GetChildNodes(getter_AddRefs(nodeList));
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  if (!nodeList)
>+    return NS_ERROR_UNEXPECTED;
>+
>+  PRUint32 childNodesLength;
>+  rv = nodeList->GetLength(&childNodesLength);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  /*
>+    xsd:choice means one of the particles must validate.
>+  */
>+  PRBool isValid = PR_FALSE;
>+  PRBool notFound = PR_FALSE;
>+  nsCOMPtr<nsISchemaParticle> particle;
>+  nsCOMPtr<nsIDOMNode> currentNode(aStartNode), leftOvers, tmpNode;
>+  nsAutoString localName, particleName;
>+  PRUint32 particleCounter = 0;
>+
>+  LOG(("======================== New Choice ==============================="));
>+
>+  while (!isValid && currentNode && (particleCounter < particleCount)) {
>+    // get node type
>+    PRUint16 nodeType;
>+    currentNode->GetNodeType(&nodeType);
>+
>+    // if not an element node, skip
>+    if (nodeType != nsIDOMNode::ELEMENT_NODE) {
>+      currentNode->GetNextSibling(getter_AddRefs(tmpNode));
>+      currentNode = tmpNode;
>+      continue;
>+    }
>+
>+    // get the particle
>+    rv = aSchemaModelGroup->GetParticle(particleCounter, getter_AddRefs(particle));
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    particle->GetName(particleName);
>+    currentNode->GetLocalName(localName);
>+
>+    // if the particle has no name (so not an xsd:element) or the name patches
>+    // the node's localname, then we should try to validate.
>+    if (particleName.IsEmpty() || localName.Equals(particleName)) {
>+      rv = ValidateComplexParticle(currentNode, particle,
>+                                   getter_AddRefs(leftOvers), &notFound,
>+                                   &isValid);

'patches'?  Or do you mean 'matches'?

>+
>+      // if not valid and the names matched, we can bail early
>+      if (!isValid && localName.Equals(particleName)) {
>+        // The schema spec says that you can't have 2 particles with the same name,
>+        // so we know we don't have to continue looking for matching particles.
>+        LOG(("  Invalid: We found a matching particle, but it did not validate"));
>+        break;
>+      }
>+
>+      currentNode = leftOvers;
>+    }
>+
>+    particleCounter++;
>+  }
>+
>+  if (!isValid) {
>+    // None of the particles managed to validate.
>+    notFound = PR_TRUE;
>+  }
>+
>+  // make sure aLeftOvers points to null or an element node
>+  nsSchemaValidatorUtils::SetToNullOrElement(currentNode, aLeftOvers);
>+
>+  *aNotFound = notFound;
>+  *aResult = isValid;
>+
>+  return rv;
>+}
>+
>+nsresult
>+nsSchemaValidator::ValidateComplexAll(nsIDOMNode* aStartNode,
>+                                      nsISchemaModelGroup *aSchemaModelGroup,
>+                                      nsIDOMNode **aLeftOvers, PRBool *aNotFound,
>+                                      PRBool *aResult)
>+{
>+  if (!aStartNode || !aSchemaModelGroup)
>+    return NS_ERROR_UNEXPECTED;
>+
>+  // get the model group details
>+  PRUint32 minOccurs;
>+  nsresult rv = aSchemaModelGroup->GetMinOccurs(&minOccurs);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  PRUint32 maxOccurs;
>+  rv = aSchemaModelGroup->GetMaxOccurs(&maxOccurs);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  PRUint32 particleCount;
>+  rv = aSchemaModelGroup->GetParticleCount(&particleCount);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  // xsd:all means that the order of the particles does not matter, and
>+  // that particles can only occur 0 or 1 time
>+  PRBool isValid = PR_FALSE;
>+  PRBool notFound = PR_FALSE;
>+  PRBool done = PR_FALSE;
>+  nsCOMPtr<nsISchemaParticle> particle;
>+  nsCOMPtr<nsIDOMNode> currentNode(aStartNode), leftOvers, tmpNode;
>+  nsAutoString localName, particleName;
>+
>+  // since xsd:all does not care about order, we need to record how often each
>+  // particle was hit
>+  nsDataHashtable<nsISupportsHashKey,PRUint32> particleHits;
>+  if (!particleHits.Init())
>+    return NS_ERROR_OUT_OF_MEMORY;

You should move the isValid, etc. stuff above to below here.  No sense doing
that work if you are just going to jump out.

>+
>+  for (PRUint32 i = 0; i < particleCount; ++i) {
>+    rv = aSchemaModelGroup->GetParticle(i, getter_AddRefs(particle));
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    particleHits.Put(particle, 0);
>+  }
>+
>+  LOG(("====================== New All ==========================="));
>+
>+  PRUint32 validatedNodes = 0;
>+
>+  // we are done when we hit a node that doesn't fit our schema
>+  while (!done && currentNode) {
>+    // get node type
>+    PRUint16 nodeType;
>+    currentNode->GetNodeType(&nodeType);
>+    currentNode->GetLocalName(localName);
>+
>+    // if not an element node, skip
>+    if (nodeType != nsIDOMNode::ELEMENT_NODE) {
>+      currentNode->GetNextSibling(getter_AddRefs(tmpNode));
>+      currentNode = tmpNode;
>+      continue;
>+    }
>+
>+    LOG(("      - Validating element (%s)",
>+         NS_ConvertUTF16toUTF8(localName).get()));
>+
>+    // walk all the particles until we find one that validates
>+    PRUint32 particleNum = 0;
>+    PRBool foundParticle = PR_FALSE;
>+
>+    while (!foundParticle && particleNum < particleCount) {
>+      rv = aSchemaModelGroup->GetParticle(particleNum, getter_AddRefs(particle));
>+      NS_ENSURE_SUCCESS(rv, rv);
>+
>+      particle->GetName(particleName);
>+
>+      if (particleName.Equals(localName)) {
>+        // try to validate
>+        rv = ValidateComplexParticle(currentNode, particle,
>+                                     getter_AddRefs(leftOvers), &notFound,
>+                                     &foundParticle);
>+        NS_ENSURE_SUCCESS(rv, rv);
>+
>+        nsAutoString tmps;
>+        leftOvers->GetLocalName(tmps);
>+      }

Ummmm, why get local name here if you never use it?  tmps will go out of scope right after
you call GetLocalName.


>+
>+nsresult
>+nsSchemaValidator::ValidateSchemaAttribute(nsIDOMNode* aNode,
>+                                           nsISchemaAttribute *aAttr,
>+                                           const nsAString & aAttrName,
>+                                           PRUint32 *aFoundAttrCount,
>+                                           PRBool *aResult)
>+{
>+  PRUint16 use;
>+  nsresult rv = aAttr->GetUse(&use);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  nsAutoString fixedValue, attrValue;
>+  rv = aAttr->GetFixedValue(fixedValue);
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  nsCOMPtr<nsISchemaSimpleType> simpleType;
>+  rv = aAttr->GetType(getter_AddRefs(simpleType));
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  nsCOMPtr<nsISchemaComponent> schema(do_QueryInterface(simpleType, &rv));
>+  NS_ENSURE_SUCCESS(rv, rv);

You don't reference 'use' or 'simpleType' until much later. Need to do the test this
early?  Also, I don't see where you use 'schema' at all.


Most of these comments were just nits or simple things.  With them, r=me
Attachment #216052 - Flags: review?(aaronr) → review+
(Assignee)

Comment 9

11 years ago
fixed trunk/1.8 branch
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: xf-to-branch
(Assignee)

Comment 10

11 years ago
checked into the 1.8.0.3 branch
Keywords: fixed1.8.0.3

Updated

10 years ago
Whiteboard: xf-to-branch
You need to log in before you can comment on or make changes to this bug.