Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
exceptionfactory closed pull request #8442: NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… URL: https://github.com/apache/nifi/pull/8442 -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
exceptionfactory commented on code in PR #8442:
URL: https://github.com/apache/nifi/pull/8442#discussion_r1500057587
##
nifi-api/src/main/java/org/apache/nifi/components/ValidationContext.java:
##
@@ -21,9 +21,13 @@
import org.apache.nifi.controller.ControllerService;
import org.apache.nifi.controller.ControllerServiceLookup;
import org.apache.nifi.expression.ExpressionLanguageCompiler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
Review Comment:
This introduces a dependency on `slf4j-api` in `nifi-api`, which is coming
through right now from `slf4j-simple`. As all of the logging statements are for
debugging, it seems like the most straightforward option is to remove the debug
logging.
##
nifi-api/src/test/java/org/apache/nifi/time/TestTimeFormat.java:
##
@@ -0,0 +1,278 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.nifi.time;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class TestTimeFormat {
Review Comment:
This should be renamed to `TestDurationFormat`.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
markap14 commented on PR #8442: URL: https://github.com/apache/nifi/pull/8442#issuecomment-1959863512 Thanks @dan-s1 for catching my oversight on the URL Validator issue. I restored the logic from UriUtils, but we don't want to have Utils types of classes in the API module. Because of that, I just moved the logic into the URLValidator class, so that it becomes part of the Validator itself. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
markap14 commented on code in PR #8442:
URL: https://github.com/apache/nifi/pull/8442#discussion_r1498393202
##
nifi-api/src/main/java/org/apache/nifi/processor/util/StandardValidators.java:
##
@@ -540,9 +540,9 @@ public ValidationResult validate(final String subject,
final String input, final
}
try {
+// Check that we can parse the value as a URL
final String evaluatedInput =
context.newPropertyValue(input).evaluateAttributeExpressions().getValue();
-final URI uri = UriUtils.create(evaluatedInput);
-uri.toURL();
+URI.create(evaluatedInput).toURL();
Review Comment:
Got it. I saw it was used in InvokeHTTP but didn't think it made sense for
validation. But I guess it does.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
markap14 commented on code in PR #8442:
URL: https://github.com/apache/nifi/pull/8442#discussion_r1498391143
##
nifi-api/src/main/java/org/apache/nifi/processor/util/StandardValidators.java:
##
@@ -843,7 +843,7 @@ public ValidationResult validate(final String subject,
final String input, final
final boolean validSyntax = pattern.matcher(lowerCase).matches();
final ValidationResult.Builder builder = new
ValidationResult.Builder();
if (validSyntax) {
-final long nanos = FormatUtils.getTimeDuration(lowerCase,
TimeUnit.NANOSECONDS);
+final long nanos = new TimeFormat().getTimeDuration(lowerCase,
TimeUnit.NANOSECONDS);
Review Comment:
Yeah, I intentionally didn't make the method static, because I was thinking
there were more methods, some of which might benefit from providing a timezone,
etc. But now that the refactoring is complete, the methods are really only
about durations. Probably makes sense to use static methods and name it
`DurationFormat` rather than `TimeFormat`.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
dan-s1 commented on code in PR #8442:
URL: https://github.com/apache/nifi/pull/8442#discussion_r1498371538
##
nifi-api/src/main/java/org/apache/nifi/processor/util/StandardValidators.java:
##
@@ -540,9 +540,9 @@ public ValidationResult validate(final String subject,
final String input, final
}
try {
+// Check that we can parse the value as a URL
final String evaluatedInput =
context.newPropertyValue(input).evaluateAttributeExpressions().getValue();
-final URI uri = UriUtils.create(evaluatedInput);
-uri.toURL();
+URI.create(evaluatedInput).toURL();
Review Comment:
Lines 544-545 which were removed were meant to fix the bug discovered in
NIFI-12513
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
markap14 commented on code in PR #8442:
URL: https://github.com/apache/nifi/pull/8442#discussion_r1498374579
##
nifi-api/src/main/java/org/apache/nifi/time/TimeFormat.java:
##
@@ -0,0 +1,251 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.nifi.time;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class TimeFormat {
+private static final String UNION = "|";
+
+
+// for Time Durations
+private static final String NANOS = join(UNION, "ns", "nano", "nanos",
"nanosecond", "nanoseconds");
+private static final String MILLIS = join(UNION, "ms", "milli", "millis",
"millisecond", "milliseconds");
+private static final String SECS = join(UNION, "s", "sec", "secs",
"second", "seconds");
+private static final String MINS = join(UNION, "m", "min", "mins",
"minute", "minutes");
+private static final String HOURS = join(UNION, "h", "hr", "hrs", "hour",
"hours");
+private static final String DAYS = join(UNION, "d", "day", "days");
+private static final String WEEKS = join(UNION, "w", "wk", "wks", "week",
"weeks");
+
+private static final String VALID_TIME_UNITS = join(UNION, NANOS, MILLIS,
SECS, MINS, HOURS, DAYS, WEEKS);
+public static final String TIME_DURATION_REGEX = "([\\d.]+)\\s*(" +
VALID_TIME_UNITS + ")";
+public static final Pattern TIME_DURATION_PATTERN =
Pattern.compile(TIME_DURATION_REGEX);
+private static final List TIME_UNIT_MULTIPLIERS =
Arrays.asList(1000L, 1000L, 1000L, 60L, 60L, 24L);
+
+
+/**
+ * Returns a time duration in the requested {@link TimeUnit} after parsing
the {@code String}
+ * input. If the resulting value is a decimal (i.e.
+ * {@code 25 hours -> TimeUnit.DAYS = 1.04}), the value is rounded.
+ * Use {@link #getPreciseTimeDuration(String, TimeUnit)} if fractional
values are desirable
+ *
+ * @param value the raw String input (i.e. "28 minutes")
+ * @param desiredUnit the requested output {@link TimeUnit}
+ * @return the whole number value of this duration in the requested units
+ * @see #getPreciseTimeDuration(String, TimeUnit)
+ */
+public long getTimeDuration(final String value, final TimeUnit
desiredUnit) {
+return Math.round(getPreciseTimeDuration(value, desiredUnit));
+}
+
+/**
+ * Returns the parsed and converted input in the requested units.
+ *
+ * If the value is {@code 0 <= x < 1} in the provided units, the units
will first be converted to a smaller unit to get a value >= 1 (i.e. 0.5 seconds
-> 500 milliseconds).
+ * This is because the underlying unit conversion cannot handle decimal
values.
+ *
+ * If the value is {@code x >= 1} but x is not a whole number, the units
will first be converted to a smaller unit to attempt to get a whole number
value (i.e. 1.5 seconds -> 1500 milliseconds).
+ *
+ * If the value is {@code x < 1000} and the units are {@code
TimeUnit.NANOSECONDS}, the result will be a whole number of nanoseconds,
rounded (i.e. 123.4 ns -> 123 ns).
+ *
+ * This method handles decimal values over {@code 1 ns}, but {@code < 1
ns} will return {@code 0} in any other unit.
+ *
+ * Examples:
+ *
+ * "10 seconds", {@code TimeUnit.MILLISECONDS} -> 10_000.0
+ * "0.010 s", {@code TimeUnit.MILLISECONDS} -> 10.0
+ * "0.010 s", {@code TimeUnit.SECONDS} -> 0.010
+ * "0.010 ns", {@code TimeUnit.NANOSECONDS} -> 1
+ * "0.010 ns", {@code TimeUnit.MICROSECONDS} -> 0
+ *
+ * @param value the {@code String} input
+ * @param desiredUnit the desired output {@link TimeUnit}
+ * @return the parsed and converted amount (without a unit)
+ */
+public double getPreciseTimeDuration(final String value, final TimeUnit
desiredUnit) {
+final Matcher matcher =
TIME_DURATION_PATTERN.matcher(value.toLowerCase());
+if (!matcher.matches()) {
+throw new IllegalArgumentException("Value '" + value + "' is not a
valid time duration");
+}
+
+final String duration = matcher.group(1);
+final String units
Re: [PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
dan-s1 commented on code in PR #8442:
URL: https://github.com/apache/nifi/pull/8442#discussion_r1498320874
##
nifi-api/src/main/java/org/apache/nifi/processor/util/StandardValidators.java:
##
@@ -843,7 +843,7 @@ public ValidationResult validate(final String subject,
final String input, final
final boolean validSyntax = pattern.matcher(lowerCase).matches();
final ValidationResult.Builder builder = new
ValidationResult.Builder();
if (validSyntax) {
-final long nanos = FormatUtils.getTimeDuration(lowerCase,
TimeUnit.NANOSECONDS);
+final long nanos = new TimeFormat().getTimeDuration(lowerCase,
TimeUnit.NANOSECONDS);
Review Comment:
`TimeFormat`, does not have any instance variables, Couldn't all the methods
be static hence the constructor for it should be private and no instantiation
should be allowed.
##
nifi-api/src/main/java/org/apache/nifi/time/TimeFormat.java:
##
@@ -0,0 +1,251 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.nifi.time;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class TimeFormat {
+private static final String UNION = "|";
+
+
+// for Time Durations
+private static final String NANOS = join(UNION, "ns", "nano", "nanos",
"nanosecond", "nanoseconds");
+private static final String MILLIS = join(UNION, "ms", "milli", "millis",
"millisecond", "milliseconds");
+private static final String SECS = join(UNION, "s", "sec", "secs",
"second", "seconds");
+private static final String MINS = join(UNION, "m", "min", "mins",
"minute", "minutes");
+private static final String HOURS = join(UNION, "h", "hr", "hrs", "hour",
"hours");
+private static final String DAYS = join(UNION, "d", "day", "days");
+private static final String WEEKS = join(UNION, "w", "wk", "wks", "week",
"weeks");
+
+private static final String VALID_TIME_UNITS = join(UNION, NANOS, MILLIS,
SECS, MINS, HOURS, DAYS, WEEKS);
+public static final String TIME_DURATION_REGEX = "([\\d.]+)\\s*(" +
VALID_TIME_UNITS + ")";
+public static final Pattern TIME_DURATION_PATTERN =
Pattern.compile(TIME_DURATION_REGEX);
+private static final List TIME_UNIT_MULTIPLIERS =
Arrays.asList(1000L, 1000L, 1000L, 60L, 60L, 24L);
+
+
+/**
+ * Returns a time duration in the requested {@link TimeUnit} after parsing
the {@code String}
+ * input. If the resulting value is a decimal (i.e.
+ * {@code 25 hours -> TimeUnit.DAYS = 1.04}), the value is rounded.
+ * Use {@link #getPreciseTimeDuration(String, TimeUnit)} if fractional
values are desirable
+ *
+ * @param value the raw String input (i.e. "28 minutes")
+ * @param desiredUnit the requested output {@link TimeUnit}
+ * @return the whole number value of this duration in the requested units
+ * @see #getPreciseTimeDuration(String, TimeUnit)
+ */
+public long getTimeDuration(final String value, final TimeUnit
desiredUnit) {
+return Math.round(getPreciseTimeDuration(value, desiredUnit));
+}
+
+/**
+ * Returns the parsed and converted input in the requested units.
+ *
+ * If the value is {@code 0 <= x < 1} in the provided units, the units
will first be converted to a smaller unit to get a value >= 1 (i.e. 0.5 seconds
-> 500 milliseconds).
+ * This is because the underlying unit conversion cannot handle decimal
values.
+ *
+ * If the value is {@code x >= 1} but x is not a whole number, the units
will first be converted to a smaller unit to attempt to get a whole number
value (i.e. 1.5 seconds -> 1500 milliseconds).
+ *
+ * If the value is {@code x < 1000} and the units are {@code
TimeUnit.NANOSECONDS}, the result will be a whole number of nanoseconds,
rounded (i.e. 123.4 ns -> 123 ns).
+ *
+ * This method handles decimal values over {@code 1 ns}, but {@code < 1
ns} will return {@code 0} in any other unit.
+ *
+ * Examples:
+ *
+ * "10 seconds", {@code TimeUnit.MILLISECONDS} -> 10_000.0
+ * "0.010 s", {@code
[PR] NIFI-12832: Eliminated unnecessary dependencies from nifi-mock; moved… [nifi]
markap14 opened a new pull request, #8442: URL: https://github.com/apache/nifi/pull/8442 … StandardValidators to nifi-api; broke apart FormatUtils into FormatUtils and TimeFormat classes, with TimeFormat existing in nifi-api # Summary [NIFI-0](https://issues.apache.org/jira/browse/NIFI-0) # Tracking Please complete the following tracking steps prior to pull request creation. ### Issue Tracking - [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created ### Pull Request Tracking - [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-0` - [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-0` ### Pull Request Formatting - [ ] Pull Request based on current revision of the `main` branch - [ ] Pull Request refers to a feature branch with one commit containing changes # Verification Please indicate the verification steps performed prior to pull request creation. ### Build - [ ] Build completed using `mvn clean install -P contrib-check` - [ ] JDK 21 ### Licensing - [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html) - [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files ### Documentation - [ ] Documentation formatting appears as expected in rendered files -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
