chibenwa commented on a change in pull request #765:
URL: https://github.com/apache/james-project/pull/765#discussion_r758080077
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/ProcessingContext.scala
##########
@@ -75,27 +73,8 @@ object JsonPath {
case "" => Nil
case "*" => List(WildcardPart)
case string if ArrayElementPart.parse(string).isDefined =>
ArrayElementPart.parse(string)
- case part: String =>
- val arrayElementPartPosition = part.indexOf('[')
- if (arrayElementPartPosition < 0) {
- asPlainPart(part)
- } else if (arrayElementPartPosition == 0) {
- asArrayElementPart(string)
- } else {
- asArrayElementInAnObject(string, part, arrayElementPartPosition)
- }
+ case part: String => List(PlainPart(part))
Review comment:
I think we could change the return type here to `Option` and further
simplify things here...
```
case string => ArrayElementPart.parse(string)
.orElse(Some(PlainPart(part)))
```
##########
File path:
server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/json/BackReferenceTest.scala
##########
@@ -46,26 +46,23 @@ class BackReferenceTest extends AnyWordSpec with Matchers {
}
"Array element parsing" should {
- "succeed when poositive" in {
- ArrayElementPart.parse("[1]") should equal(Some(ArrayElementPart(1)))
+ "succeed when positive" in {
+ ArrayElementPart.parse("1") should equal(Some(ArrayElementPart(1)))
}
"succeed when zero" in {
- ArrayElementPart.parse("[0]") should equal(Some(ArrayElementPart(0)))
+ ArrayElementPart.parse("0") should equal(Some(ArrayElementPart(0)))
+ }
+ "succeed when no bracket" in {
+ ArrayElementPart.parse("0") should equal(Some(ArrayElementPart(0)))
}
"fail when negative" in {
- ArrayElementPart.parse("[-1]") should equal(None)
+ ArrayElementPart.parse("-1") should equal(None)
}
"fail when not an int" in {
- ArrayElementPart.parse("[invalid]") should equal(None)
- }
- "fail when not closed" in {
- ArrayElementPart.parse("[0") should equal(None)
+ ArrayElementPart.parse("invalid") should equal(None)
}
- "fail when not open" in {
- ArrayElementPart.parse("0]") should equal(None)
- }
- "fail when no bracket" in {
- ArrayElementPart.parse("0") should equal(None)
+ "fail when has bracket" in {
+ ArrayElementPart.parse("[0]") should equal(None)
Review comment:
Test also:
`list/id/99999999999999999999999999999999999999999999999999999999999999999999999999999999`
##########
File path:
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/ProcessingContext.scala
##########
@@ -45,10 +45,8 @@ case class PlainPart(name: String) extends JsonPathPart {
object ArrayElementPart {
def parse(string: String): Option[ArrayElementPart] = {
- if (string.startsWith("[") && string.endsWith("]")) {
- val positionPart: String = string.substring(1, string.length - 1)
- Try(positionPart.toInt)
- .fold(_ => None, fromInt)
+ if (string forall Character.isDigit) {
+ fromInt(string.toInt)
Review comment:
We got read of some error handling here. Are we sure to handle all cases
already?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
