Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(#1095) Remove code from ctor #1098

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions pom.xml
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
<?xml version="1.0"?>
<!--
The MIT License (MIT)

Copyright (c) 2014-2019 Yegor Bugayenko

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included
in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE
Expand Down Expand Up @@ -532,4 +528,4 @@ SOFTWARE.
</build>
</profile>
</profiles>
</project>
</project>
Copy link
Contributor

Choose a reason for hiding this comment

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

@baudoliver7 I suggest reverting this part, if xcop doesn't complain than xml formating is fine

80 changes: 41 additions & 39 deletions src/main/java/org/takes/rq/multipart/RqMtBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.cactoos.Scalar;
import org.cactoos.scalar.Sticky;
import org.cactoos.scalar.Unchecked;
import org.cactoos.text.FormattedText;
import org.cactoos.text.Lowered;
Expand Down Expand Up @@ -102,19 +104,19 @@ public final class RqMtBase implements RqMultipart {
private static final String CRLF = "\r\n";

/**
* Map of params and values.
* Scalar of Map of params and values.
*/
private final Map<String, List<Request>> map;
private final Scalar<Map<String, List<Request>>> smap;

/**
* Internal buffer.
* Scalar of Internal buffer.
*/
private final ByteBuffer buffer;
private final Scalar<ByteBuffer> sbuffer;

/**
* InputStream based on request body.
* Scalar of InputStream based on request body.
*/
private final InputStream stream;
private final Scalar<InputStream> sstream;

/**
* Original request.
Expand All @@ -125,27 +127,24 @@ public final class RqMtBase implements RqMultipart {
* Ctor.
* @param req Original request
* @throws IOException If fails
* @todo #950:30m Remove code from this ctor, leaving only
* initialization. Currently this constructor access body
* of the request and triggers its evaluation. This breaks
* composition of multiple request, as it can be seen in
* {@link RqMtFake}. When this task is done, remove
* explicit lazy evaluation for RqMtFake.
* @checkstyle ExecutableStatementCountCheck (2 lines)
*/
public RqMtBase(final Request req) throws IOException {
this.origin = req;
this.stream = new RqLengthAware(req).body();
this.buffer = ByteBuffer.allocate(
// @checkstyle MagicNumberCheck (1 line)
Math.min(8192, this.stream.available())
);
this.map = this.requests(req);
this.sstream = new Sticky<>(() -> new RqLengthAware(req).body());
Copy link
Contributor

Choose a reason for hiding this comment

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

@baudoliver7 Is it possible to use RqOnce on the ctor parameter instead of three Stickies?
It seems also that some fields could be moved inside of request method, such as sbuffer

this.sbuffer = new Sticky<>(() -> {
return ByteBuffer.allocate(
// @checkstyle MagicNumberCheck (1 line)
Math.min(8192, RqMtBase.this.sstream.value().available())
);
});
this.smap = new Sticky<>(() -> RqMtBase.this.requests(req));
}

@Override
public Iterable<Request> part(final CharSequence name) {
final List<Request> values = this.map.getOrDefault(
final Map<String, List<Request>> map = new Unchecked<>(this.smap).value();
final List<Request> values = map.getOrDefault(
new UncheckedText(
new Lowered(name.toString())
).asString(),
Expand All @@ -157,7 +156,7 @@ public Iterable<Request> part(final CharSequence name) {
Collections.emptyList(),
new FormattedText(
"there are no parts by name \"%s\" among %d others: %s",
name, this.map.size(), this.map.keySet()
name, map.size(), map.keySet()
).toString()
);
} else {
Expand All @@ -174,7 +173,7 @@ public Iterable<Request> part(final CharSequence name) {

@Override
public Iterable<String> names() {
return this.map.keySet();
return new Unchecked<>(this.smap).value().keySet();
}

@Override
Expand Down Expand Up @@ -222,8 +221,10 @@ private Map<String, List<Request>> requests(
)
);
}
final ReadableByteChannel body = Channels.newChannel(this.stream);
if (body.read(this.buffer) < 0) {
final ByteBuffer buffer = new Unchecked<>(this.sbuffer).value();
final InputStream stream = new Unchecked<>(this.sstream).value();
final ReadableByteChannel body = Channels.newChannel(stream);
if (body.read(buffer) < 0) {
throw new HttpException(
HttpURLConnection.HTTP_BAD_REQUEST,
"failed to read the request body"
Expand All @@ -232,15 +233,15 @@ private Map<String, List<Request>> requests(
final byte[] boundary = String.format(
"%s--%s", RqMtBase.CRLF, matcher.group(1)
).getBytes(RqMtBase.ENCODING);
this.buffer.flip();
this.buffer.position(boundary.length - 2);
buffer.flip();
buffer.position(boundary.length - 2);
final Collection<Request> requests = new LinkedList<>();
while (this.buffer.hasRemaining()) {
final byte data = this.buffer.get();
while (buffer.hasRemaining()) {
final byte data = buffer.get();
if (data == '-') {
break;
}
this.buffer.position(this.buffer.position() + 1);
buffer.position(buffer.position() + 1);
requests.add(this.make(boundary, body));
}
return RqMtBase.asMap(requests);
Expand Down Expand Up @@ -292,30 +293,31 @@ private void copy(final WritableByteChannel target,
throws IOException {
int match = 0;
boolean cont = true;
final ByteBuffer buffer = new Unchecked<>(this.sbuffer).value();
while (cont) {
if (!this.buffer.hasRemaining()) {
this.buffer.clear();
if (!buffer.hasRemaining()) {
buffer.clear();
for (int idx = 0; idx < match; ++idx) {
this.buffer.put(boundary[idx]);
buffer.put(boundary[idx]);
}
match = 0;
if (body.read(this.buffer) == -1) {
if (body.read(buffer) == -1) {
break;
}
this.buffer.flip();
buffer.flip();
}
final ByteBuffer btarget = this.buffer.slice();
final int offset = this.buffer.position();
final ByteBuffer btarget = buffer.slice();
final int offset = buffer.position();
btarget.limit(0);
while (this.buffer.hasRemaining()) {
final byte data = this.buffer.get();
while (buffer.hasRemaining()) {
final byte data = buffer.get();
if (data == boundary[match]) {
++match;
} else if (data == boundary[0]) {
match = 1;
} else {
match = 0;
btarget.limit(this.buffer.position() - offset);
btarget.limit(buffer.position() - offset);
}
if (match == boundary.length) {
cont = false;
Expand Down Expand Up @@ -378,7 +380,7 @@ public void close() throws IOException {
super.close();
} finally {
for (final List<Request> requests
: RqMtBase.this.map.values()) {
: new Unchecked<>(RqMtBase.this.smap).value().values()) {
for (final Request request : requests) {
request.body().close();
}
Expand Down
21 changes: 9 additions & 12 deletions src/main/java/org/takes/rq/multipart/RqMtFake.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@
import org.cactoos.Scalar;
import org.cactoos.io.InputOf;
import org.cactoos.io.InputStreamOf;
import org.cactoos.scalar.IoChecked;
import org.cactoos.scalar.LengthOf;
import org.cactoos.scalar.Sticky;
import org.cactoos.scalar.Unchecked;
import org.takes.Body;
import org.takes.Request;
import org.takes.rq.RequestOf;
Expand Down Expand Up @@ -59,40 +57,39 @@ public final class RqMtFake implements RqMultipart {
/**
* Fake multipart request.
*/
private final Scalar<RqMultipart> fake;
private final RqMultipart fake;

/**
* Fake ctor.
* @param req Fake request header holder
* @param dispositions Fake request body parts
* @throws IOException If fails
*/
public RqMtFake(final Request req, final Request... dispositions) {
this.fake = new Sticky<>(
() -> new RqMtBase(
new RqMtFake.FakeMultipartRequest(req, dispositions)
)
public RqMtFake(final Request req, final Request... dispositions)
throws IOException {
this.fake = new RqMtBase(
new RqMtFake.FakeMultipartRequest(req, dispositions)
);
}

@Override
public Iterable<Request> part(final CharSequence name) {
return new Unchecked<>(this.fake).value().part(name);
return this.fake.part(name);
}

@Override
public Iterable<String> names() {
return new Unchecked<>(this.fake).value().names();
return this.fake.names();
}

@Override
public Iterable<String> head() throws IOException {
return new IoChecked<>(this.fake).value().head();
return this.fake.head();
}

@Override
public InputStream body() throws IOException {
return new IoChecked<>(this.fake).value().body();
return this.fake.body();
}

/**
Expand Down