szaszm commented on a change in pull request #1123: URL: https://github.com/apache/nifi-minifi-cpp/pull/1123#discussion_r667032440
########## File path: libminifi/include/core/state/nodes/ConfigurationChecksums.h ########## @@ -0,0 +1,56 @@ +/** + * 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. + */ + +#pragma once + +#include <string> +#include <vector> + +#include "core/Resource.h" +#include "core/state/nodes/MetricsBase.h" +#include "utils/ChecksumCalculator.h" +#include "utils/gsl.h" + +namespace org { +namespace apache { +namespace nifi { +namespace minifi { +namespace state { +namespace response { + +class ConfigurationChecksums : public ResponseNode { + public: + ConfigurationChecksums() = default; + explicit ConfigurationChecksums(const std::string& name, const utils::Identifier& uuid = {}) : ResponseNode(name, uuid) {} + + void addChecksumCalculator(gsl::not_null<utils::ChecksumCalculator*> checksum_calculator); Review comment: Same here, I would take a reference and convert it to `gsl::not_null<utils::ChecksumCalculator*>` only when storing in the vector. `std::reference_wrapper` is a possible alternative storage type that represents the same concept but without null checking in the constructor. ########## File path: libminifi/include/utils/ChecksumCalculator.h ########## @@ -0,0 +1,57 @@ +/** + * 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. + */ + +#pragma once + +#include <string> +#include <utility> + +#include "utils/OptionalUtils.h" + +namespace org { +namespace apache { +namespace nifi { +namespace minifi { +namespace utils { + +class ChecksumCalculator { + public: + static constexpr const char* CHECKSUM_TYPE = "SHA256"; + static constexpr size_t LENGTH_OF_HASH_IN_BYTES = 32; + + void setFileLocation(const std::string& file_location); + std::string getFileName() const; + std::string getChecksum(); + void invalidateChecksum(); Review comment: Did you consider `stat`-ing the files on each call and recalculating the checksum based on the last modified timestamp? This would fix the case when the watched files were modified outside of minifi. An alternative faster, but way harder and more complex approach would be subscribing to filesystem events like `inotify` (Linux)/`FindFirstChangeNotification`(Windows)/`fsevents`(Mac OS X), and recalculating the hashes or invalidating the cache on change events. ########## File path: libminifi/include/core/FlowConfiguration.h ########## @@ -162,6 +165,8 @@ class FlowConfiguration : public CoreComponent { } } + gsl::not_null<utils::ChecksumCalculator*> getChecksumCalculator() { return gsl::make_not_null(&checksum_calculator_); } Review comment: Why not return a reference? `gsl::not_null<utils::ChecksumCalculator*>` is only useful in case of non-static data members in my experience, to support move semantics and similar mutations. -- 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]
