From: Szymon Jastrzebski Date: Wed, 19 Dec 2018 07:58:40 +0000 (+0100) Subject: [Bookmark] Refactoring module X-Git-Tag: submit/tizen/20190109.233706~4^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=dea6b2635202a8fc156d01df286cbd9cb40b7946;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git [Bookmark] Refactoring module [Verification] TCT 100% Change-Id: I9f6f9ac1236c0e5ea049a1820e3de30ed332cdd6 --- diff --git a/src/bookmark/bookmark_api.js b/src/bookmark/bookmark_api.js index 7ccf8a14..a1be1aa7 100755 --- a/src/bookmark/bookmark_api.js +++ b/src/bookmark/bookmark_api.js @@ -115,7 +115,6 @@ BookmarkManager.prototype.remove = function() { if (native_.isFailure(result)) { throw native_.getErrorObject(result); } - return; } @@ -156,7 +155,7 @@ BookmarkProvider.prototype.addToFolder = function() { title: args.bookmark.title, url: String(args.bookmark.url), parentId: args.parentId, - type: args.bookmark instanceof tizen.BookmarkFolder ? 1 : 0 + type: args.bookmark instanceof tizen.BookmarkFolder } ); @@ -187,7 +186,7 @@ BookmarkProvider.prototype.getFolder = function() { var ret = native_.callSync('Bookmark_get', { id: args.id, - shouldGetItems: 0 + shouldGetItems: false }); if (native_.isFailure(ret)) { @@ -223,7 +222,7 @@ BookmarkProvider.prototype.getFolderItems = function() { var ret = native_.callSync('Bookmark_get', { id: Number(args.id), - shouldGetItems: 1 + shouldGetItems: true }); if (native_.isFailure(ret)) { diff --git a/src/bookmark/bookmark_extension.cc b/src/bookmark/bookmark_extension.cc index 1b0160b3..9769d74c 100644 --- a/src/bookmark/bookmark_extension.cc +++ b/src/bookmark/bookmark_extension.cc @@ -19,9 +19,9 @@ #include "common/logger.h" namespace { -const char kBookmark[] = "tizen.bookmark"; -const char kBookmarkItem[] = "tizen.BookmarkItem"; -const char kBookmarkFolder[] = "tizen.BookmarkFolder"; +constexpr char kBookmark[] = "tizen.bookmark"; +constexpr char kBookmarkItem[] = "tizen.BookmarkItem"; +constexpr char kBookmarkFolder[] = "tizen.BookmarkFolder"; } // This will be generated from bookmark_api.js. @@ -35,18 +35,11 @@ BookmarkExtension::BookmarkExtension() { SetExtensionName(kBookmark); SetJavaScriptAPI(kSource_bookmark_api); - const char* entry_points[] = {kBookmarkItem, kBookmarkFolder, NULL}; + const char* entry_points[] = {kBookmarkItem, kBookmarkFolder, nullptr}; SetExtraJSEntryPoints(entry_points); - - if (bp_bookmark_adaptor_initialize()) { - LoggerE("Fail: Bookmark not supported"); - } } BookmarkExtension::~BookmarkExtension() { - if (bp_bookmark_adaptor_deinitialize()) { - LoggerE("Fail: Deinitialize Bookmark"); - } } common::Instance* BookmarkExtension::CreateInstance() { diff --git a/src/bookmark/bookmark_instance.cc b/src/bookmark/bookmark_instance.cc index 145d6b4e..e7e3f2b4 100644 --- a/src/bookmark/bookmark_instance.cc +++ b/src/bookmark/bookmark_instance.cc @@ -31,14 +31,15 @@ namespace extension { namespace bookmark { namespace { -const char kId[] = "id"; -const char kTitle[] = "title"; -const char kType[] = "type"; -const char kParentId[] = "parentId"; -const char kUrl[] = "url"; - -const std::string kPrivilegeBookmarkRead = "http://tizen.org/privilege/bookmark.read"; -const std::string kPrivilegeBookmarkWrite = "http://tizen.org/privilege/bookmark.write"; +constexpr char kId[] = "id"; +constexpr char kTitle[] = "title"; +constexpr char kType[] = "type"; +constexpr char kParentId[] = "parentId"; +constexpr char kUrl[] = "url"; +constexpr char kShouldGetItems[] = "shouldGetItems"; + +constexpr char kPrivilegeBookmarkRead[] = "http://tizen.org/privilege/bookmark.read"; +constexpr char kPrivilegeBookmarkWrite[] = "http://tizen.org/privilege/bookmark.write"; } // namespace BookmarkInstance::BookmarkInstance() { @@ -53,37 +54,41 @@ BookmarkInstance::BookmarkInstance() { REGISTER_SYNC("Bookmark_removeAll", BookmarkRemoveAll); REGISTER_SYNC("Bookmark_getRootId", BookmarkGetRootId); #undef REGISTER_SYNC + + if (bp_bookmark_adaptor_initialize()) { + LoggerE("bp_bookmark_adaptor_initialize failed."); + } } BookmarkInstance::~BookmarkInstance() { ScopeLogger(); + if (bp_bookmark_adaptor_deinitialize()) { + LoggerE("bp_bookmark_adaptor_deinitialize failed."); + } } bool BookmarkInstance::bookmark_foreach(Context& ctx, bp_bookmark_info_fmt& info) { ScopeLogger(); int ids_count = 0; - int* ids = NULL; - BookmarkObject item; + int* ids = nullptr; if (bp_bookmark_adaptor_get_ids_p(&ids, &ids_count, -1, 0, -1, -1, -1, -1, - BP_BOOKMARK_O_DATE_CREATED, 0) < 0) + BP_BOOKMARK_O_DATE_CREATED, 0) < 0) { return false; - - if (ids_count > 0) { - for (int i = 0; i < ids_count; i++) { - if (bp_bookmark_adaptor_get_easy_all(ids[i], &info) < 0) { - int errorcode = bp_bookmark_adaptor_get_errorcode(); - LoggerW("bp_bookmark_adaptor_get_easy_all for id %d returns error: %d", ids[i], errorcode); - continue; - } - item.id = ids[i]; - item.bookmark_info = info; - if ((ctx.shouldGetItems && item.bookmark_info.parent != ctx.id) || - (!ctx.shouldGetItems && item.id != ctx.id)) - continue; - ctx.folders.push_back(item); + } + ctx.folders.reserve(ids_count); + std::unique_ptr ids_ptr(ids, free); + for (int i = 0; i < ids_count; ++i) { + if (bp_bookmark_adaptor_get_easy_all(ids[i], &info) < 0) { + int errorcode = bp_bookmark_adaptor_get_errorcode(); + LoggerW("bp_bookmark_adaptor_get_easy_all for id %d returns error: %d", ids[i], errorcode); + continue; + } + if ((ctx.shouldGetItems && info.parent != ctx.id) || + (!ctx.shouldGetItems && ids[i] != ctx.id)) { + continue; } + ctx.folders.push_back({ids[i], info}); } - free(ids); return true; } @@ -91,8 +96,6 @@ PlatformResult BookmarkInstance::BookmarkUrlExists(const char* url, bool* exists ScopeLogger(); int ids_count = 0; int* ids = nullptr; - char* compare_url = nullptr; - int ntv_ret = bp_bookmark_adaptor_get_ids_p(&ids, // ids &ids_count, // count -1, // limit @@ -108,39 +111,31 @@ PlatformResult BookmarkInstance::BookmarkUrlExists(const char* url, bool* exists ErrorCode::UNKNOWN_ERR, "Failed to obtain bookmarks", ("bp_bookmark_adaptor_get_ids_p error: %d (%s)", ntv_ret, get_error_message(ntv_ret))); } - - PlatformResult result{ErrorCode::NO_ERROR}; - bool url_found = false; - for (int i = 0; (i < ids_count) && result && !url_found; ++i) { + std::unique_ptr ids_ptr(ids, free); + for (int i = 0; i < ids_count; ++i) { + char* compare_url = nullptr; ntv_ret = bp_bookmark_adaptor_get_url(ids[i], &compare_url); if (ntv_ret < 0) { - result = LogAndCreateResult( + PlatformResult result = LogAndCreateResult( ErrorCode::UNKNOWN_ERR, "Failed to obtain URL", ("bp_bookmark_adaptor_get_url error: %d (%s)", ntv_ret, get_error_message(ntv_ret))); - } else { - url_found = (0 == strcmp(url, compare_url)); - free(compare_url); - compare_url = nullptr; + return result; + } + std::unique_ptr bookmark_url(compare_url, free); + if (0 == strcmp(url, compare_url)) { + *exists = true; + return PlatformResult{ErrorCode::NO_ERROR}; } } - - if (result) { - *exists = url_found; - } - - free(ids); - - return result; + *exists = false; + return PlatformResult{ErrorCode::NO_ERROR}; } PlatformResult BookmarkInstance::BookmarkTitleExistsInParent(const char* title, int parent, bool* exists) { ScopeLogger(); int ids_count = 0; - int compare_parent = -1; int* ids = nullptr; - char* compare_title = nullptr; - int ntv_ret = bp_bookmark_adaptor_get_ids_p(&ids, // ids &ids_count, // count -1, // limit @@ -156,33 +151,33 @@ PlatformResult BookmarkInstance::BookmarkTitleExistsInParent(const char* title, ErrorCode::UNKNOWN_ERR, "Failed to obtain bookmarks", ("bp_bookmark_adaptor_get_ids_p error: %d (%s)", ntv_ret, get_error_message(ntv_ret))); } - - PlatformResult result{ErrorCode::NO_ERROR}; - bool title_found = false; - for (int i = 0; (i < ids_count) && result && !title_found; ++i) { - if ((ntv_ret = bp_bookmark_adaptor_get_parent_id(ids[i], &compare_parent)) < 0) { - result = LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Failed to obtain parent ID", - ("bp_bookmark_adaptor_get_parent_id error: %d (%s)", ntv_ret, - get_error_message(ntv_ret))); - } else if ((ntv_ret = bp_bookmark_adaptor_get_title(ids[i], &compare_title)) < 0) { - result = LogAndCreateResult( + std::unique_ptr ids_ptr(ids, free); + for (int i = 0; i < ids_count; ++i) { + int compare_parent = -1; + ntv_ret = bp_bookmark_adaptor_get_parent_id(ids[i], &compare_parent); + if (ntv_ret < 0) { + PlatformResult result = + LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Failed to obtain parent ID", + ("bp_bookmark_adaptor_get_parent_id error: %d (%s)", ntv_ret, + get_error_message(ntv_ret))); + return result; + } + char* compare_title = nullptr; + ntv_ret = bp_bookmark_adaptor_get_title(ids[i], &compare_title); + if (ntv_ret < 0) { + PlatformResult result = LogAndCreateResult( ErrorCode::UNKNOWN_ERR, "Failed to obtain title", ("bp_bookmark_adaptor_get_title error: %d (%s)", ntv_ret, get_error_message(ntv_ret))); - } else { - title_found = (parent == compare_parent) && (0 == strcmp(title, compare_title)); - free(compare_title); - compare_title = nullptr; - compare_parent = -1; + return result; + } + std::unique_ptr bookmark_url(compare_title, free); + if (parent == compare_parent && 0 == strcmp(title, compare_title)) { + *exists = true; + return PlatformResult{ErrorCode::NO_ERROR}; } } - - if (result) { - *exists = title_found; - } - - free(ids); - - return result; + *exists = false; + return PlatformResult{ErrorCode::NO_ERROR}; } void BookmarkInstance::BookmarkGet(const picojson::value& arg, picojson::object& o) { @@ -191,9 +186,7 @@ void BookmarkInstance::BookmarkGet(const picojson::value& arg, picojson::object& Context ctx = {0}; bp_bookmark_info_fmt info = {0}; - picojson::value::array arr; - - ctx.shouldGetItems = arg.get("shouldGetItems").get(); + ctx.shouldGetItems = arg.get(kShouldGetItems).get(); ctx.id = arg.get(kId).get(); if (!bookmark_foreach(ctx, info)) { @@ -201,34 +194,33 @@ void BookmarkInstance::BookmarkGet(const picojson::value& arg, picojson::object& return; } - std::vector::iterator it; - for (it = ctx.folders.begin(); it != ctx.folders.end(); ++it) { + picojson::value::array arr; + arr.reserve(ctx.folders.size()); + for (auto& item : ctx.folders) { picojson::object obj; - BookmarkObject entry = *it; - - obj[kTitle] = picojson::value(entry.bookmark_info.title); - obj[kId] = picojson::value(std::to_string(entry.id)); - obj[kType] = picojson::value(std::to_string(entry.bookmark_info.type)); - obj[kParentId] = picojson::value(std::to_string(entry.bookmark_info.parent)); - if (!entry.bookmark_info.type) obj[kUrl] = picojson::value(entry.bookmark_info.url); + obj.emplace(kTitle, picojson::value(item.bookmark_info.title)); + obj.emplace(kId, picojson::value(std::to_string(item.id))); + obj.emplace(kType, picojson::value(std::to_string(item.bookmark_info.type))); + obj.emplace(kParentId, picojson::value(std::to_string(item.bookmark_info.parent))); + if (!item.bookmark_info.type) { + obj.emplace(kUrl, picojson::value(item.bookmark_info.url)); + } - arr.push_back(picojson::value(obj)); + arr.push_back(std::move(picojson::value(std::move(obj)))); } - ReportSuccess(picojson::value(arr), o); + ReportSuccess(picojson::value(std::move(arr)), o); } void BookmarkInstance::BookmarkAdd(const picojson::value& arg, picojson::object& o) { ScopeLogger(); CHECK_PRIVILEGE_ACCESS(kPrivilegeBookmarkWrite, &o); - int saved_id = -1; - const auto& title = arg.get(kTitle).get(); const int parent = static_cast(arg.get(kParentId).get()); - const int type = static_cast(arg.get(kType).get()); + const bool is_folder = arg.get(kType).get(); const auto& url = arg.get(kUrl).get(); - if (0 == type) { // bookmark + if (!is_folder) { // bookmark bool exists = false; auto result = BookmarkUrlExists(url.c_str(), &exists); if (!result) { @@ -239,9 +231,7 @@ void BookmarkInstance::BookmarkAdd(const picojson::value& arg, picojson::object& &o); return; } - } - - if (1 == type) { // folder + } else { // folder bool exists = false; auto result = BookmarkTitleExistsInParent(title.c_str(), parent, &exists); if (!result) { @@ -254,41 +244,39 @@ void BookmarkInstance::BookmarkAdd(const picojson::value& arg, picojson::object& } } - int ntv_ret; - - ntv_ret = bp_bookmark_adaptor_create(&saved_id); + int saved_id = -1; + int ntv_ret = bp_bookmark_adaptor_create(&saved_id); if (ntv_ret < 0) { LogAndReportError(PlatformResult(ErrorCode::UNKNOWN_ERR, "Failed to create adaptor"), &o); return; } + std::unique_ptr bookmark( + &saved_id, [](int* bookmark_ptr) { bp_bookmark_adaptor_delete(*bookmark_ptr); }); ntv_ret = bp_bookmark_adaptor_set_title(saved_id, title.c_str()); if (ntv_ret < 0) { - bp_bookmark_adaptor_delete(saved_id); LogAndReportError(PlatformResult(ErrorCode::UNKNOWN_ERR, "Failed to set title"), &o); return; } ntv_ret = bp_bookmark_adaptor_set_parent_id(saved_id, parent); if (ntv_ret < 0) { - bp_bookmark_adaptor_delete(saved_id); LogAndReportError(PlatformResult(ErrorCode::UNKNOWN_ERR, "Failed to set parent id"), &o); return; } - ntv_ret = bp_bookmark_adaptor_set_type(saved_id, type); + ntv_ret = bp_bookmark_adaptor_set_type(saved_id, static_cast(is_folder)); if (ntv_ret < 0) { - bp_bookmark_adaptor_delete(saved_id); LogAndReportError(PlatformResult(ErrorCode::UNKNOWN_ERR, "Failed to set type"), &o); return; } ntv_ret = bp_bookmark_adaptor_set_url(saved_id, url.c_str()); if (ntv_ret < 0) { - bp_bookmark_adaptor_delete(saved_id); LogAndReportError(PlatformResult(ErrorCode::UNKNOWN_ERR, "Failed to set url"), &o); return; } + bookmark.release(); ReportSuccess(picojson::value(std::to_string(saved_id)), o); } diff --git a/src/bookmark/bookmark_instance.h b/src/bookmark/bookmark_instance.h index f4ee2ad1..ae468c73 100644 --- a/src/bookmark/bookmark_instance.h +++ b/src/bookmark/bookmark_instance.h @@ -34,7 +34,7 @@ struct BookmarkObject { struct Context { int id; - int shouldGetItems; + bool shouldGetItems; std::vector folders; };